Bugfix: Adding missing nextPageToken for pagination in drive tools - #513
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds optional Changes
Sequence DiagramsequenceDiagram
actor Client
participant Tools as search_drive_files / list_drive_items
participant Builder as build_drive_list_params
participant API as Google Drive API
Client->>Tools: Call with (query, page_size, page_token, ...)
Tools->>Builder: build_drive_list_params(..., page_token)
Builder-->>Tools: params (may include pageToken)
Tools->>API: Execute files.list with params
API-->>Tools: items + nextPageToken?
Tools-->>Client: return formatted items (include nextPageToken if present)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/gdrive/test_drive_tools.py (1)
16-36:⚠️ Potential issue | 🟠 MajorDuplicate imports and incomplete function definition need cleanup.
This segment appears to have merge artifacts or copy-paste errors:
- Lines 16-17 and line 24 duplicate the same imports from
gdrive.drive_tools- Lines 19-22 define an incomplete
_unwrapfunction (noreturnstatement, missing__wrapped__handling)- Lines 27-36 define the complete, correct
_unwrapfunction- The import on line 23 comes after code, causing the E402 linter failure
The second
_unwrapdefinition (lines 27-36) shadows the first, so tests will run, but the dead code should be removed.🔧 Proposed fix to consolidate imports and remove duplicate code
-from gdrive.drive_tools import list_drive_items, search_drive_files - - -def _unwrap(fn): - """Unwrap a decorator chain to the original async function.""" - if hasattr(fn, "fn"): - fn = fn.fn # FunctionTool wrapper (other server versions) -from gdrive.drive_helpers import build_drive_list_params from gdrive.drive_tools import list_drive_items, search_drive_files +from gdrive.drive_helpers import build_drive_list_params def _unwrap(tool): """Unwrap a FunctionTool + decorator chain to the original async function. Handles both older FastMCP (FunctionTool with .fn) and newer FastMCP (server.tool() returns the function directly). """ fn = tool.fn if hasattr(tool, "fn") else tool while hasattr(fn, "__wrapped__"): fn = fn.__wrapped__ return fn🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/gdrive/test_drive_tools.py` around lines 16 - 36, Remove the merge artifacts: consolidate imports so that build_drive_list_params, list_drive_items, and search_drive_files are imported once at the top (no imports after code to avoid E402), delete the incomplete duplicate _unwrap definition (the one missing return and __wrapped__ handling), and keep the complete _unwrap implementation (the version that handles tool.fn and unwraps __wrapped__); ensure only the final _unwrap symbol remains and that imports appear before any function definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/gdrive/test_drive_tools.py`:
- Around line 16-36: Remove the merge artifacts: consolidate imports so that
build_drive_list_params, list_drive_items, and search_drive_files are imported
once at the top (no imports after code to avoid E402), delete the incomplete
duplicate _unwrap definition (the one missing return and __wrapped__ handling),
and keep the complete _unwrap implementation (the version that handles tool.fn
and unwraps __wrapped__); ensure only the final _unwrap symbol remains and that
imports appear before any function definitions.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
gdrive/drive_helpers.pygdrive/drive_tools.pytests/gdrive/test_drive_tools.py
🚧 Files skipped from review as they are similar to previous changes (1)
- gdrive/drive_tools.py
…ace_mcp into drive_files_pagination_fix
Closes: #512
Description
Bug fix for #512
In GDrive Tools, nextPageToken is mentioned here:
google_workspace_mcp/gdrive/drive_helpers.py
However, it's not implemented.
Right now, only the first page can be returned.
This is true for both list_drive_items and search_drive_files.In GDrive Tools, nextPageToken is mentioned at:
Testing
Checklist
Additional Notes
Add any other context about the pull request here.
Summary by CodeRabbit
New Features
Tests