feat(gmail): add list_gmail_drafts and delete_gmail_draft tools#781
Open
bcsmith528 wants to merge 1 commit into
Open
feat(gmail): add list_gmail_drafts and delete_gmail_draft tools#781bcsmith528 wants to merge 1 commit into
bcsmith528 wants to merge 1 commit into
Conversation
Surfaces two endpoints from the Gmail API that were not previously exposed: - list_gmail_drafts: wraps users.drafts.list with optional Gmail-search query filtering and pagination. Resolves each draft to its underlying message metadata (subject, from, to, cc, date, snippet, message/thread IDs) so callers can identify drafts without a second round-trip. - delete_gmail_draft: wraps users.drafts.delete, permanently removing a draft container and its underlying message. Both tools live at the gmail.compose permission level (same scope as the existing draft_gmail_message), so they require no new scopes when granted at the "drafts" service level or above. Added to the extended tier in tool_tiers.yaml and the README tools table. Includes tests covering: formatted output for non-empty drafts, the empty- account case, query and page_token propagation to the underlying API call, and that delete invokes drafts.delete with the correct draft_id and userId.
Contributor
📝 WalkthroughWalkthroughThis PR adds two new Gmail MCP tools for draft management. ChangesGmail Draft Management Tools
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gmail/gmail_tools.py`:
- Around line 2501-2506: The page_size parameter currently allows unbounded
values which can trigger massive drafts.get fan-out; add a hard upper bound
(e.g., 100) to the page_size Field and/or validate/clamp it before making calls
to drafts.get to prevent long-running listing paths. Specifically, update the
page_size Field definition (the Annotated Field at page_size) to include an
upper limit (use le or max in the Field) and ensure the code path that
iterates/calls drafts.get (the draft listing logic referenced around lines
2535-2543) enforces the same cap or returns a 400 error when page_size exceeds
the limit; also update the description to document the maximum accepted value.
- Around line 2514-2530: Replace the multi-paragraph docstring for the
drafts-listing tool (the docstring that begins "Lists drafts in the user's Gmail
account...") with a single present-tense sentence that describes the tool's
purpose, and remove parameter explanations from the docstring; instead ensure
parameter hints appear in the function signature/type annotations or the tool's
argument metadata. Apply the same change to the other multi-paragraph docstring
in gmail_tools.py around the later drafts-related tool (the block currently
spanning lines ~2631-2643), making each tool description one present-tense
sentence and moving parameter details into argument metadata/type hints. Ensure
formatting remains a single-line docstring per tool.
- Line 2492: Rename the new tool identifiers that use snake_case to imperative
camelCase (≤3 words): change the function named list_gmail_drafts to
listGmailDrafts and likewise rename the other snake_case tool introduced around
line 2620 to an imperative camelCase name (e.g., listGmailMessages or similar);
update every reference, import/export, and any registration or tool mapping that
uses the old names (including decorator usages, exported lists, and tests) so
the code and MCP surface remain consistent with the tool naming rule.
- Around line 2597-2599: The except HttpError handler that appends per-draft
error text should not swallow Gmail API failures; replace the current block that
does "lines.append(f\"... (failed to fetch metadata: {e})\")" with logic that
maps or raises a ToolExecutionError for HttpError (e.g., use the project's
Google API error-mapping helper if available, otherwise raise ToolExecutionError
with the HttpError details and context about the draft_id). Ensure you reference
the HttpError caught in this block and raise ToolExecutionError instead of
returning partial success text so centralized error handling can detect
rate-limit/quota and other API failures.
In `@tests/gmail/test_list_and_delete_drafts.py`:
- Around line 44-142: Tests in this file currently use chained Mock objects
instead of the repo-standard httpretty + canned fixtures approach; replace the
Mock-based Gmail API stubs in tests referencing list_gmail_drafts,
delete_gmail_draft (and helper _draft_metadata) with httpretty HTTP mocks that
load the canned JSON fixtures for the Gmail endpoints (/gmail/v1/users/me/drafts
list, get and delete) and return the same payloads used now; ensure the
httpretty responses assert/accept query params (q, pageToken, userId) and return
nextPageToken or empty bodies as appropriate so the existing assertions (e.g.,
checking "matching 'subject:invoice'", page_token='token-next', and delete
confirmation) continue to pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7562891-5633-44e8-95bf-7a65c666d9f3
📒 Files selected for processing (4)
README.mdcore/tool_tiers.yamlgmail/gmail_tools.pytests/gmail/test_list_and_delete_drafts.py
| ) | ||
| @handle_http_errors("list_gmail_drafts", is_read_only=True, service_type="gmail") | ||
| @require_google_service("gmail", GMAIL_COMPOSE_SCOPE) | ||
| async def list_gmail_drafts( |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Rename new tool identifiers to imperative camelCase.
Line 2492 and Line 2620 introduce snake_case names, which violate the tool naming rule and make the MCP surface inconsistent with the required schema convention.
As per coding guidelines **/*.py: "Tool names must be imperative, camelCase, and ≤3 words".
Also applies to: 2620-2620
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gmail/gmail_tools.py` at line 2492, Rename the new tool identifiers that use
snake_case to imperative camelCase (≤3 words): change the function named
list_gmail_drafts to listGmailDrafts and likewise rename the other snake_case
tool introduced around line 2620 to an imperative camelCase name (e.g.,
listGmailMessages or similar); update every reference, import/export, and any
registration or tool mapping that uses the old names (including decorator
usages, exported lists, and tests) so the code and MCP surface remain consistent
with the tool naming rule.
Comment on lines
+2501
to
+2506
| page_size: Annotated[ | ||
| int, | ||
| Field( | ||
| description="Maximum number of drafts to return. Defaults to 25.", | ||
| ), | ||
| ] = 25, |
Contributor
There was a problem hiding this comment.
Add a hard upper bound for page_size to prevent slow draft listing paths.
Line 2501 accepts unbounded values, which can trigger very large drafts.get fan-out and long request times.
Proposed guard
async def list_gmail_drafts(
@@
page_size: Annotated[
int,
Field(
description="Maximum number of drafts to return. Defaults to 25.",
),
] = 25,
@@
) -> str:
@@
+ if page_size < 1 or page_size > 100:
+ raise UserInputError("page_size must be between 1 and 100.")
+
list_kwargs = {"userId": "me", "maxResults": page_size}Also applies to: 2535-2543
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gmail/gmail_tools.py` around lines 2501 - 2506, The page_size parameter
currently allows unbounded values which can trigger massive drafts.get fan-out;
add a hard upper bound (e.g., 100) to the page_size Field and/or validate/clamp
it before making calls to drafts.get to prevent long-running listing paths.
Specifically, update the page_size Field definition (the Annotated Field at
page_size) to include an upper limit (use le or max in the Field) and ensure the
code path that iterates/calls drafts.get (the draft listing logic referenced
around lines 2535-2543) enforces the same cap or returns a 400 error when
page_size exceeds the limit; also update the description to document the maximum
accepted value.
Comment on lines
+2514
to
+2530
| """ | ||
| Lists drafts in the user's Gmail account, optionally filtered by a Gmail search query. | ||
|
|
||
| For each draft, returns the Draft ID along with the underlying message's Subject, | ||
| From, To, and a short snippet so you can identify which draft is which without | ||
| needing to fetch each one separately. | ||
|
|
||
| Args: | ||
| user_google_email (str): The user's Google email address. Required. | ||
| query (Optional[str]): Optional Gmail search query to filter the drafts list. | ||
| page_size (int): Maximum number of drafts to return. Defaults to 25. | ||
| page_token (Optional[str]): Pagination token from a previous response. | ||
|
|
||
| Returns: | ||
| str: A formatted list of drafts with their Draft IDs and key headers, or a | ||
| message indicating no drafts were found. | ||
| """ |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Tool descriptions should be reduced to a single present-tense sentence.
Line 2514 and Line 2631 currently use multi-paragraph docstrings; these should be concise one-sentence tool descriptions with parameter hints in argument metadata.
As per coding guidelines **/*.py: "Tool descriptions must be a single sentence in present tense with parameter hints included".
Also applies to: 2631-2643
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gmail/gmail_tools.py` around lines 2514 - 2530, Replace the multi-paragraph
docstring for the drafts-listing tool (the docstring that begins "Lists drafts
in the user's Gmail account...") with a single present-tense sentence that
describes the tool's purpose, and remove parameter explanations from the
docstring; instead ensure parameter hints appear in the function signature/type
annotations or the tool's argument metadata. Apply the same change to the other
multi-paragraph docstring in gmail_tools.py around the later drafts-related tool
(the block currently spanning lines ~2631-2643), making each tool description
one present-tense sentence and moving parameter details into argument
metadata/type hints. Ensure formatting remains a single-line docstring per tool.
Comment on lines
+2597
to
+2599
| except HttpError as e: | ||
| lines.append(f"{idx}. Draft ID: {draft_id} (failed to fetch metadata: {e})") | ||
| lines.append("") |
Contributor
There was a problem hiding this comment.
Do not swallow Gmail API failures inside per-draft metadata fetch.
Line 2597 converts HttpError into text output, which bypasses centralized tool error mapping and can return partial-success responses during API failure conditions.
Proposed handling adjustment
- except HttpError as e:
- lines.append(f"{idx}. Draft ID: {draft_id} (failed to fetch metadata: {e})")
- lines.append("")
+ except HttpError:
+ raise
+ except Exception as e:
+ lines.append(f"{idx}. Draft ID: {draft_id} (failed to fetch metadata: {e})")
+ lines.append("")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gmail/gmail_tools.py` around lines 2597 - 2599, The except HttpError handler
that appends per-draft error text should not swallow Gmail API failures; replace
the current block that does "lines.append(f\"... (failed to fetch metadata:
{e})\")" with logic that maps or raises a ToolExecutionError for HttpError
(e.g., use the project's Google API error-mapping helper if available, otherwise
raise ToolExecutionError with the HttpError details and context about the
draft_id). Ensure you reference the HttpError caught in this block and raise
ToolExecutionError instead of returning partial success text so centralized
error handling can detect rate-limit/quota and other API failures.
Comment on lines
+44
to
+142
| mock_service = Mock() | ||
| mock_service.users().drafts().list().execute.return_value = { | ||
| "drafts": [ | ||
| {"id": "r123", "message": {"id": "msg_r123", "threadId": "t1"}}, | ||
| {"id": "r456", "message": {"id": "msg_r456", "threadId": "t2"}}, | ||
| ] | ||
| } | ||
| mock_service.users().drafts().get().execute.side_effect = [ | ||
| _draft_metadata("r123", "First draft", "alice@example.com", snippet="Hi Alice"), | ||
| _draft_metadata("r456", "Second draft", "bob@example.com", snippet="Hi Bob"), | ||
| ] | ||
|
|
||
| result = await _unwrap(list_gmail_drafts)( | ||
| service=mock_service, | ||
| user_google_email="user@example.com", | ||
| ) | ||
|
|
||
| assert "Found 2 draft(s)" in result | ||
| assert "Draft ID: r123" in result | ||
| assert "First draft" in result | ||
| assert "alice@example.com" in result | ||
| assert "Draft ID: r456" in result | ||
| assert "Second draft" in result | ||
| assert "bob@example.com" in result | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_list_gmail_drafts_empty_account(): | ||
| mock_service = Mock() | ||
| mock_service.users().drafts().list().execute.return_value = {} | ||
|
|
||
| result = await _unwrap(list_gmail_drafts)( | ||
| service=mock_service, | ||
| user_google_email="user@example.com", | ||
| ) | ||
|
|
||
| assert result == "No drafts found." | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_list_gmail_drafts_passes_query_to_api(): | ||
| mock_service = Mock() | ||
| mock_service.users().drafts().list().execute.return_value = {} | ||
|
|
||
| result = await _unwrap(list_gmail_drafts)( | ||
| service=mock_service, | ||
| user_google_email="user@example.com", | ||
| query="subject:invoice", | ||
| ) | ||
|
|
||
| assert "matching 'subject:invoice'" in result | ||
| list_kwargs = ( | ||
| mock_service.users.return_value.drafts.return_value.list.call_args.kwargs | ||
| ) | ||
| assert list_kwargs["q"] == "subject:invoice" | ||
| assert list_kwargs["userId"] == "me" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_list_gmail_drafts_propagates_page_token(): | ||
| mock_service = Mock() | ||
| mock_service.users().drafts().list().execute.return_value = { | ||
| "drafts": [{"id": "r1", "message": {"id": "msg_r1", "threadId": "t1"}}], | ||
| "nextPageToken": "token-next", | ||
| } | ||
| mock_service.users().drafts().get().execute.return_value = _draft_metadata( | ||
| "r1", "Subject", "to@example.com" | ||
| ) | ||
|
|
||
| result = await _unwrap(list_gmail_drafts)( | ||
| service=mock_service, | ||
| user_google_email="user@example.com", | ||
| page_token="token-current", | ||
| ) | ||
|
|
||
| list_kwargs = ( | ||
| mock_service.users.return_value.drafts.return_value.list.call_args.kwargs | ||
| ) | ||
| assert list_kwargs["pageToken"] == "token-current" | ||
| assert "page_token='token-next'" in result | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_delete_gmail_draft_calls_api_and_confirms(): | ||
| mock_service = Mock() | ||
| mock_service.users().drafts().delete().execute.return_value = None | ||
|
|
||
| result = await _unwrap(delete_gmail_draft)( | ||
| service=mock_service, | ||
| user_google_email="user@example.com", | ||
| draft_id="r_abc123", | ||
| ) | ||
|
|
||
| assert "Draft r_abc123 deleted successfully." == result | ||
| delete_kwargs = ( | ||
| mock_service.users.return_value.drafts.return_value.delete.call_args.kwargs | ||
| ) | ||
| assert delete_kwargs["id"] == "r_abc123" | ||
| assert delete_kwargs["userId"] == "me" |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Use httpretty + canned fixtures for Google API mocking in this module.
Current direct Mock chains don't follow the repository’s required test mocking strategy for Google APIs.
As per coding guidelines tests/**/*.py: "Mock Google APIs with httpretty and canned fixtures; do not hit live services in CI".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/gmail/test_list_and_delete_drafts.py` around lines 44 - 142, Tests in
this file currently use chained Mock objects instead of the repo-standard
httpretty + canned fixtures approach; replace the Mock-based Gmail API stubs in
tests referencing list_gmail_drafts, delete_gmail_draft (and helper
_draft_metadata) with httpretty HTTP mocks that load the canned JSON fixtures
for the Gmail endpoints (/gmail/v1/users/me/drafts list, get and delete) and
return the same payloads used now; ensure the httpretty responses assert/accept
query params (q, pageToken, userId) and return nextPageToken or empty bodies as
appropriate so the existing assertions (e.g., checking "matching
'subject:invoice'", page_token='token-next', and delete confirmation) continue
to pass.
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 two tools that surface Gmail API endpoints not currently exposed by this server:
list_gmail_drafts— wrapsusers.drafts.listwith optional Gmail-search query filtering and pagination. For each draft, resolves the underlying message metadata (Subject, From, To, Cc, Date, snippet, message/thread IDs) so callers can identify drafts without an extra round-trip.delete_gmail_draft— wrapsusers.drafts.delete, permanently removing the draft container and its underlying message.Both tools live at the
gmail.composepermission level (same scope as the existingdraft_gmail_message), so they require no new scopes when granted at thedraftsservice level or above. Both are registered in theextendedtier incore/tool_tiers.yamland added to the README tools table.Motivation
Today the only draft-related tool is
draft_gmail_message. Once a draft is created there's no way through MCP to enumerate drafts or remove them, which means anything that creates a draft (e.g. an agent preparing emails for human review) accumulates clutter that the user has to clean up by hand in the Gmail web UI. Adding these two endpoints closes the gap and keeps the draft lifecycle self-contained within the server's existing scope grant.Test plan
tests/gmail/test_list_and_delete_drafts.pycovering:No drafts found.)queryparameter tousers.drafts.listpage_tokenand emission ofnextPageTokeninstructionsdelete_gmail_draftinvokesusers.drafts.deletewith the correctidanduserIdtests/gmail/suite passes (119 tests) locallyruff checkclean on touched filesNotes
list_gmail_draftsmakes onedrafts.listcall plus onedrafts.getper returned draft. For typical drafts folders this is small; users with very large drafts folders should rely onqueryfiltering orpage_sizeto keep the round-trip count bounded. A future improvement could batch thedrafts.getcalls through Gmail's HTTP batch endpoint.delete_gmail_draftis annotateddestructiveHint=True, idempotentHint=True. Idempotent because deleting a non-existent draft returns a 404 thathandle_http_errorssurfaces predictably.Summary by CodeRabbit
New Features
Documentation