feat: add Drive file content update, delete, and MIME auto-detection - #569
Open
addhass wants to merge 7 commits into
Open
feat: add Drive file content update, delete, and MIME auto-detection#569addhass wants to merge 7 commits into
addhass wants to merge 7 commits into
Conversation
When mime_type is left as the default "text/plain", infer the correct MIME type from the file extension (.md → text/markdown, .json → application/json, .csv → text/csv, etc.). This ensures files created via MCP are stored with the correct MIME type in Google Drive without requiring callers to specify it explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a new tool that overwrites the content of existing text-based files (.md, .txt, .csv, .json, .html, .xml, .yaml) in Google Drive using the same MediaIoBaseUpload pattern as create_drive_file. Includes MIME type validation to prevent accidental corruption of binary files and native Google Docs/Sheets/Slides (with helpful error messages pointing to the correct tools). Placed in the drive.extended tier alongside update_drive_file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a dedicated delete tool that handles the full deletion lifecycle: - Default: moves file to trash (recoverable for 30 days) - If file is already trashed: automatically permanently deletes - permanent=True: skips trash, deletes immediately The smart auto-escalation prevents the confusing "no changes" response that occurs when using update_drive_file(trashed=True) on an already- trashed file. The docstring includes "delete", "remove", and "trash" keywords so LLM-based callers can discover it naturally. Placed in the drive.extended tier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdded two Drive actions and their implementations: Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DriveTools as gdrive/drive_tools.py
participant GoogleDrive as Google Drive API
Caller->>DriveTools: update_drive_file_content(file_id, content, [mime_type])
DriveTools->>DriveTools: validate/resolve mime_type (UPDATABLE_TEXT_MIME_TYPES)
DriveTools->>GoogleDrive: files.get(fileId) (fetch metadata/mimeType)
GoogleDrive-->>DriveTools: file metadata (mimeType, name)
DriveTools->>GoogleDrive: files.update(fileId, media=content, mimeType)
GoogleDrive-->>DriveTools: updated file response
DriveTools-->>Caller: summary (name, id, chars, link)
Caller->>DriveTools: delete_drive_file(file_id, permanent=False)
DriveTools->>GoogleDrive: files.delete(fileId, supportsAllDrives, permanent?)
GoogleDrive-->>DriveTools: deletion confirmation
DriveTools-->>Caller: confirmation message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 3
🧹 Nitpick comments (1)
gdrive/drive_tools.py (1)
1755-1759: Set explicit public tool metadata instead of relying on decorator defaults.Please give these tools explicit FastMCP
name/descriptionvalues so the exported IDs stay imperative camelCase, and keepcore/tool_tiers.yamlaligned with the public IDs.As per coding guidelines, "FastMCP tool names must be imperative, camelCase, and ≤3 words; descriptions must be a single sentence in present tense with parameter hints."
Also applies to: 1842-1845
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gdrive/drive_tools.py` around lines 1755 - 1759, The tool decorated around function update_drive_file_content lacks explicit FastMCP metadata; update the `@server.tool`() decorator to include name="updateDriveFileContent" and a single-sentence present-tense description with parameter hints (e.g., "Update a Drive file's content given fileId and newContent."), ensuring the name is imperative camelCase and ≤3 words; apply the same pattern to the other Drive tool with missing metadata in the nearby block (the other decorated Drive function) so exported IDs and core/tool_tiers.yaml remain aligned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gdrive/drive_tools.py`:
- Around line 1785-1787: The log in update_drive_file_content currently emits
the raw user_google_email; change the logger.info calls to avoid logging PII by
either removing the user email or replacing it with a redacted identifier (e.g.,
mask the email local-part or emit a stable hashed identifier), and apply the
same change to the other logger.info calls in this module that include
user_google_email; update the log message to include only non-PII context
(file_id, operation) and the redacted or hashed user token instead of the raw
email.
- Around line 1882-1905: The current logic escalates from trashing to deleting
when a retry sees already_trashed=True, which makes permanent=False
non-idempotent; change the branch so that files().delete(...) is called only
when permanent is True, and if permanent is False and already_trashed is True
return an explicit "already in trash" message (use the variables file_id and
file_name) instead of deleting; update the block around service.files().delete
and service.files().update so permanent controls deletion and non-permanent
retries simply return the already-in-trash response.
- Around line 1804-1828: Validate the mime_type override before using it by
ensuring mime_type (when provided) is in UPDATABLE_TEXT_MIME_TYPES and
reject/raise if it’s not (use the existing current_mime/mime_type/content_mime
logic), and when calling service.files().update include body={"mimeType":
content_mime} (in addition to media_body=media) so the file's stored MIME type
is persisted; update the code around the variables current_mime, mime_type,
content_mime, MediaIoBaseUpload media and the service.files().update call to
perform these checks and add the body param.
---
Nitpick comments:
In `@gdrive/drive_tools.py`:
- Around line 1755-1759: The tool decorated around function
update_drive_file_content lacks explicit FastMCP metadata; update the
`@server.tool`() decorator to include name="updateDriveFileContent" and a
single-sentence present-tense description with parameter hints (e.g., "Update a
Drive file's content given fileId and newContent."), ensuring the name is
imperative camelCase and ≤3 words; apply the same pattern to the other Drive
tool with missing metadata in the nearby block (the other decorated Drive
function) so exported IDs and core/tool_tiers.yaml remain aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f93fe4e0-6b2f-472b-8560-b8cf00d43409
📒 Files selected for processing (2)
core/tool_tiers.yamlgdrive/drive_tools.py
- Validate mime_type override in update_drive_file_content: reject non-text MIME types when caller explicitly provides one - Make delete_drive_file idempotent: when permanent=False and file is already trashed, return informational message instead of escalating to permanent deletion Skipped PII logging and @server.tool() metadata feedback — both would diverge from the existing patterns used by all other tools in this file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
gdrive/drive_tools.py (2)
1811-1834:⚠️ Potential issue | 🟠 MajorPersist MIME metadata when
mime_typeoverride is provided.Line 1811 allows a MIME override, but Line 1828-Line 1833 only sends
media_body. If callers passmime_type, includebody={"mimeType": content_mime}so metadata changes are persisted as documented.🩹 Proposed fix
- updated_file = await asyncio.to_thread( - service.files() - .update( - fileId=file_id, - media_body=media, - fields="id, name, modifiedTime, webViewLink", - supportsAllDrives=True, - ) - .execute - ) + update_kwargs: Dict[str, Any] = { + "fileId": file_id, + "media_body": media, + "fields": "id, name, mimeType, modifiedTime, webViewLink", + "supportsAllDrives": True, + } + if mime_type is not None: + update_kwargs["body"] = {"mimeType": content_mime} + + updated_file = await asyncio.to_thread( + service.files().update(**update_kwargs).execute + )For Google Drive API v3 `files.update`, when uploading with `media_body`, does changing file MIME type require `body={"mimeType": "..."}` to persist metadata, or is media upload MIME alone sufficient?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gdrive/drive_tools.py` around lines 1811 - 1834, The code currently only passes media_body to service.files().update (see MediaIoBaseUpload, content_mime, and the update call), so an explicit metadata change when a mime_type override is provided isn’t persisted; modify the files().update invocation to include body={"mimeType": content_mime} (only when mime_type is provided/overridden) alongside media_body and existing params (fileId, media_body, fields, supportsAllDrives) so the new MIME metadata is sent to Drive and persisted.
1785-1787:⚠️ Potential issue | 🟠 MajorRemove raw user email from new tool logs.
Line 1786 and Line 1876 log
user_google_emaildirectly. Please redact or omit it.🔒 Proposed fix
- logger.info( - f"[update_drive_file_content] Updating content of {file_id} for {user_google_email}" - ) + logger.info( + f"[update_drive_file_content] Updating content of file_id='{file_id}'" + ) - logger.info( - f"[delete_drive_file] Deleting {file_id} for {user_google_email} (permanent={permanent})" - ) + logger.info( + f"[delete_drive_file] Deleting file_id='{file_id}' (permanent={permanent})" + )Based on learnings, Applies to **/*.py : Never log or leak secrets, refresh tokens, or PII in exceptions or event streams.
Also applies to: 1875-1877
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gdrive/drive_tools.py` around lines 1785 - 1787, The log statements in update_drive_file_content (logger.info calls that include user_google_email) must not emit raw user email; remove or redact the PII by either omitting user_google_email or replacing it with a non-sensitive token (e.g., user_id or "<redacted>") and update both occurrences (the logger.info at the update_drive_file_content call and the similar logger at the other block around lines 1875-1877). Ensure the new message keeps context (e.g., "Updating content of {file_id} for user=<redacted>") and do not introduce any new PII in exception logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gdrive/drive_tools.py`:
- Around line 1755-1760: The `@server.tool`() decorators on the functions
update_drive_file_content and delete_drive_file need explicit name and
description arguments to conform to FastMCP guidelines: add name values in
imperative camelCase (e.g., name="updateDriveFile" for update_drive_file_content
and name="deleteDriveFile" for delete_drive_file) and supply a single-sentence,
present-tense description that includes parameter hints (e.g., "Update the
content of a Drive file given fileId and new content.") while leaving the Python
function names unchanged; also simplify each function's multi-sentence docstring
to a single-sentence present-tense description with parameter hints to match the
decorator description.
---
Duplicate comments:
In `@gdrive/drive_tools.py`:
- Around line 1811-1834: The code currently only passes media_body to
service.files().update (see MediaIoBaseUpload, content_mime, and the update
call), so an explicit metadata change when a mime_type override is provided
isn’t persisted; modify the files().update invocation to include
body={"mimeType": content_mime} (only when mime_type is provided/overridden)
alongside media_body and existing params (fileId, media_body, fields,
supportsAllDrives) so the new MIME metadata is sent to Drive and persisted.
- Around line 1785-1787: The log statements in update_drive_file_content
(logger.info calls that include user_google_email) must not emit raw user email;
remove or redact the PII by either omitting user_google_email or replacing it
with a non-sensitive token (e.g., user_id or "<redacted>") and update both
occurrences (the logger.info at the update_drive_file_content call and the
similar logger at the other block around lines 1875-1877). Ensure the new
message keeps context (e.g., "Updating content of {file_id} for
user=<redacted>") and do not introduce any new PII in exception logs.
When callers explicitly pass mime_type to update_drive_file_content,
include body={"mimeType": content_mime} in the files().update call so
the file's stored metadata MIME type is updated alongside the content.
The MediaIoBaseUpload mimetype only sets the upload Content-Type header;
the Drive API metadata field needs to be set separately.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
Thanks for the thorough reviews @coderabbitai. We've addressed the actionable items:
Two items we intentionally left as-is: PII in logs: Every existing tool in
Both of these are valid improvements to the project as a whole — just not something we should introduce as one-off changes in a feature PR. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gdrive/drive_tools.py`:
- Around line 1811-1818: The code allows an explicit empty string mime_type to
bypass validation; change the validation in the content_mime/mime_type block to
treat an empty string as invalid by raising UserInputError when mime_type == ""
(distinct from mime_type is None), keeping the existing behavior that None falls
back to current_mime; reference content_mime, mime_type, current_mime,
UPDATABLE_TEXT_MIME_TYPES and UserInputError when making the check so the error
is raised for empty-string overrides before checking membership in
UPDATABLE_TEXT_MIME_TYPES.
Contributor
|
The PR looks good to me. ✏️ Learnings added
✅ Actions performedComments resolved. Auto-approval is disabled; enable |
Treat mime_type="" as invalid input rather than silently falling back to the file's current MIME type. Normalizes with strip() and raises UserInputError for empty strings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gdrive/drive_tools.py (1)
613-631: Consider documenting the auto-detection override behavior.The auto-detection triggers when
mime_type == "text/plain", which includes cases where the caller explicitly passesmime_type="text/plain". If a caller explicitly wantstext/plainfor a.jsonfile, the auto-detection will override it toapplication/json.This is a reasonable design tradeoff, but the docstring (Line 603) should clarify that auto-detection activates for
"text/plain"regardless of whether it was explicitly passed or defaulted.📝 Suggested docstring clarification
- mime_type (str): The MIME type of the file. Defaults to 'text/plain'. + mime_type (str): The MIME type of the file. Defaults to 'text/plain'. + When 'text/plain', the MIME type is auto-detected from file extension + (.md, .csv, .json, .html, .xml, .yaml). Pass a different MIME type + explicitly to override auto-detection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gdrive/drive_tools.py` around lines 613 - 631, Update the create_drive_file docstring to explicitly state that the MIME auto-detection runs when mime_type == "text/plain" (not only when omitted), so an explicit mime_type="text/plain" will be overridden based on the file extension (e.g., .json -> application/json, .md -> text/markdown) by the _ext_mime_map logic; mention how callers can force literal text/plain (e.g., by passing a different sentinel or bypassing auto-detection) and include a brief list or reference to the common extensions handled by the auto-detection so callers understand which extensions will be remapped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@gdrive/drive_tools.py`:
- Around line 613-631: Update the create_drive_file docstring to explicitly
state that the MIME auto-detection runs when mime_type == "text/plain" (not only
when omitted), so an explicit mime_type="text/plain" will be overridden based on
the file extension (e.g., .json -> application/json, .md -> text/markdown) by
the _ext_mime_map logic; mention how callers can force literal text/plain (e.g.,
by passing a different sentinel or bypassing auto-detection) and include a brief
list or reference to the common extensions handled by the auto-detection so
callers understand which extensions will be remapped.
edisonb24
added a commit
to Construct-Digital/google_workspace_mcp
that referenced
this pull request
Jul 19, 2026
…uto-detect Adds update_drive_file_content (in-place files.update with media body, preserves file ID), delete_drive_file, and create_drive_file MIME auto-detection. Resolved conflicts against newer upstream main: - union imports (+UserInputError) - kept HEAD base64-capable create_drive_file validation + PR MIME detect - kept PR new tools + HEAD annotated get_drive_shareable_link decorator Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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
Three improvements to Google Drive file management:
Auto-detect MIME type in
create_drive_file: Whenmime_typeis left as the defaulttext/plain, infer the correct type from the file extension (.md→text/markdown,.json→application/json,.csv→text/csv, etc.). No behavior change whenmime_typeis explicitly provided.New
update_drive_file_contenttool: Overwrites the content of existing text-based files (.md,.txt,.csv,.json,.html,.xml,.yaml) using the sameMediaIoBaseUploadpattern ascreate_drive_file. Includes MIME type validation to prevent corruption of binary files and native Google Docs/Sheets/Slides (with helpful error messages pointing to the correct tools).New
delete_drive_filetool: Dedicated deletion tool that handles the full lifecycle — trash by default, auto-permanent-delete if already trashed,permanent=Trueto skip trash. Solves the confusing "no changes" response fromupdate_drive_file(trashed=True)on already-trashed files. Docstring includes "delete", "remove", and "trash" keywords for LLM discoverability.Both new tools are placed in the
drive.extendedtier and follow all existing patterns (decorator chain,resolve_drive_itemfor shortcut resolution,supportsAllDrives,handle_http_errors,require_google_service).Motivation
When using workspace-mcp via Claude Desktop (or any MCP client) to manage markdown files in Google Drive:
.mdfiles were created astext/plainunless the caller explicitly setmime_typeupdate_drive_fileonly handles metadataupdate_drive_file(trashed=True), which silently returned "no changes" if the file was already trashedThese gaps made it impossible to have full CRUD on uploaded text files.
Test plan
All tested manually via
workspace-mcp --cli:create_drive_filewith.mdextension → MIME type istext/markdown(wastext/plain)create_drive_filewith explicitmime_type→ uses provided value (unchanged)update_drive_file_contenton.mdfile → content updated, char count reportedupdate_drive_file_contenton Google Doc →UserInputErrorwith helpful messageupdate_drive_file_contenton non-existent file → 404 fromresolve_drive_itemdelete_drive_fileon live file → moved to trashdelete_drive_fileon already-trashed file → permanently deleted (auto-escalation)delete_drive_filewithpermanent=True→ permanently deleted immediatelydelete_drive_fileon non-existent file → 404 errorruff checkpassesruff formatpasses🤖 Generated with Claude Code
Summary by CodeRabbit