search_drive_files: surface lastModifyingUser, createdTime, and anyon…#791
search_drive_files: surface lastModifyingUser, createdTime, and anyon…#791cosmic-fire-eng wants to merge 1 commit into
Conversation
…e-permission role (#1) Expand the Drive API fields parameter and response formatter so that search_drive_files returns the metadata needed for public link audits: createdTime, lastModifyingUser (displayName + email), and the role on any "anyone" permission (reader/commenter/writer). No OAuth scope changes required. Created By (via owners) is intentionally omitted for Shared Drive files — see TODO comment for the revisions API workaround. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR expands detailed-mode search functionality for Google Drive files. The API now requests creation time, last-modifying user, and file permissions data, which are then formatted and displayed with created timestamp, last editor attribution, and public link sharing indicators. ChangesDetailed Drive Metadata Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@gdrive/drive_helpers.py`:
- Around line 209-215: The shared build_drive_list_params currently always
requests createdTime, lastModifyingUser and permissions which bloats responses;
add a boolean parameter (e.g., include_creator_and_perms) to
build_drive_list_params and use it to conditionally append "createdTime,
lastModifyingUser(...), permissions(...)" to the fields string, then update
callers: call build_drive_list_params(include_creator_and_perms=True) from
search_drive_files (which needs those fields) and call it with False from
list_drive_items so list_drive_items only fetches size, driveId, modifiedTime,
webViewLink; keep the existing field fragments and symbol names
(build_drive_list_params, search_drive_files, list_drive_items) so changes are
minimal and clearly gated by the new flag.
🪄 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: 78c5cc0a-1eb2-428d-b692-cc8e665e9916
📒 Files selected for processing (3)
gdrive/drive_helpers.pygdrive/drive_tools.pytests/gdrive/test_drive_tools.py
| if detailed: | ||
| fields = "nextPageToken, files(id, name, mimeType, webViewLink, iconLink, modifiedTime, size, driveId)" | ||
| fields = ( | ||
| "nextPageToken, files(id, name, mimeType, webViewLink, iconLink," | ||
| " modifiedTime, createdTime, size, driveId," | ||
| " lastModifyingUser(displayName, emailAddress)," | ||
| " permissions(id, type, role))" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Call sites of build_drive_list_params =="
rg -n "build_drive_list_params\\(" gdrive/drive_tools.py
echo
echo "== search_drive_files uses newly requested fields in output =="
rg -n -C2 "Created:|Last Edited By:|Anyone with link|createdTime|lastModifyingUser|permissions" gdrive/drive_tools.py
echo
echo "== list_drive_items formatted output fields =="
rg -n -C3 "def list_drive_items|Size:|Drive ID:|Modified:|Link:" gdrive/drive_tools.pyRepository: taylorwilsdon/google_workspace_mcp
Length of output: 17629
🏁 Script executed:
# Check the build_drive_list_params function implementation and its signature
head -n 220 gdrive/drive_helpers.py | tail -n 25Repository: taylorwilsdon/google_workspace_mcp
Length of output: 1288
🏁 Script executed:
# See context around line 216 call
sed -n '210,225p' gdrive/drive_tools.pyRepository: taylorwilsdon/google_workspace_mcp
Length of output: 612
🏁 Script executed:
# See context around line 692 call
sed -n '685,700p' gdrive/drive_tools.pyRepository: taylorwilsdon/google_workspace_mcp
Length of output: 677
🏁 Script executed:
# Check if list_drive_items uses any of the disputed fields in output
sed -n '613,730p' gdrive/drive_tools.py | grep -n "createdTime\|lastModifyingUser\|permissions" || echo "No matches found in list_drive_items output"Repository: taylorwilsdon/google_workspace_mcp
Length of output: 289
Gate specialized fields in build_drive_list_params by caller to prevent unnecessary response bloat.
search_drive_files uses createdTime, lastModifyingUser, and permissions in its output (lines 239, 244, 260), but list_drive_items does not use any of these fields—it only outputs size, driveId, modifiedTime, and webViewLink (lines 715–720). Both functions call the shared build_drive_list_params with the same parameters, causing list_drive_items to fetch and deserialize unnecessary fields with every API call. Add a parameter (e.g., include_creator_and_perms) or extend the field specification to let each caller request only what it needs. This aligns with the coding guideline: "Response payloads must be concise."
🤖 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 `@gdrive/drive_helpers.py` around lines 209 - 215, The shared
build_drive_list_params currently always requests createdTime, lastModifyingUser
and permissions which bloats responses; add a boolean parameter (e.g.,
include_creator_and_perms) to build_drive_list_params and use it to
conditionally append "createdTime, lastModifyingUser(...), permissions(...)" to
the fields string, then update callers: call
build_drive_list_params(include_creator_and_perms=True) from search_drive_files
(which needs those fields) and call it with False from list_drive_items so
list_drive_items only fetches size, driveId, modifiedTime, webViewLink; keep the
existing field fragments and symbol names (build_drive_list_params,
search_drive_files, list_drive_items) so changes are minimal and clearly gated
by the new flag.
|
This seems to go hand in hand with #783 - let me see if we can't combine the two |
2 participants
…e-permission role (#1)
Expand the Drive API fields parameter and response formatter so that search_drive_files returns the metadata needed for public link audits: createdTime, lastModifyingUser (displayName + email), and the role on any "anyone" permission (reader/commenter/writer).
No OAuth scope changes required. Created By (via owners) is intentionally omitted for Shared Drive files — see TODO comment for the revisions API workaround.
Description
Brief description of the changes in this PR.
Type of Change
Testing
Checklist
Additional Notes
Add any other context about the pull request here.
To enable this setting:
Summary by CodeRabbit
New Features
Tests