feat(docs): Add comprehensive Google Docs editing capabilities #155
Conversation
|
Love it, I know #109 was looking at something similar and have wanted to get more granular with editing. Will give it a run through tomorrow! |
taylorwilsdon
requested review from
Copilot and
taylorwilsdon
and removed request for
Copilot
|
Will push up to branch with the personal files added to gitignore, in general looks like a good contribution thank you @cfdude! Few notes:
I'll take a run at fixes and test. |
|
I like the idea but this isn't working well at all - you've inspired me with the idea, will see what we can get cooking editing_demo.mp4 |
|
Hey Taylor, sorry about that, was a late night last night. I'm fixing all the bugs and testing.. should have an updated commit in a few minutes. |
I've gotten a little further along but still finding some issues with tables specifically - that's a tough spec for the LLM to wrap its head around when making the request, it seems. Would love to see your update and will share my progress shortly as well! |
|
@taylorwilsdon these commits fix the prior problems and also improve with helper functions. The insert image by URL or drive is working well now too. |
|
all thoroughly tested this time as well. |
|
Awesome, will give it a try! I've got my own version working really well with sophisticated table operations - |
|
What I'm going to do since my branch is based off your original but so divergent now is split it off to cover the table logic stuff which is still not possible with your branch, I found myself on a little side quest while testing - then I'll pull this up to main and we can get the rest of the features pulled in. All your commits are still in so it'll be a coauthor on the new PR. Thanks for the contributions! I love the idea of this being the most capable/sophisticated control of this type of stuff down to the tiniest nuance, but I also do wonder if we eventually need to make certain "classes" of tools (ie degree of sophistication) optional because small models may start to struggle with this much tool usage context. |
|
Merged in main and fixed stuff, kept all your adds. Will take a second pass at quality review, still needs some attention but we will get this in. As we get more granular we need to figure out a way to gate certain features on startup because this is getting to be too large a prompt for small models. |
|
Can you define tool tiers for these and pull in main/resolve conflicts? Love to get it released in the next |
|
@taylorwilsdon I fixed the merge conflicts and added the tool tiers |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the MCP server by adding comprehensive error handling for Google Drive image insertion and migrating all API request construction to centralized helper functions, improving maintainability and user experience.
- Added new Google Drive permission checking tools with detailed error messaging
- Enhanced image insertion with permission validation and better error handling
- Migrated inline API request construction to reusable helper functions
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Added package data configuration for core module |
| main.py | Added import for new drive file permissions module |
| gdrive/drive_helpers.py | New helper functions for Drive permissions and URL handling |
| gdrive/drive_file_permissions.py | New tools for checking Drive file permissions and sharing status |
| gdocs/docs_tools.py | Enhanced image insertion tools with Drive integration and permission validation |
| gdocs/docs_helpers.py | Added helper function for segmented text insertion |
| core/utils.py | Improved error handling to avoid suggesting re-auth for non-auth errors |
| core/tool_tiers.yaml | Added new image insertion tools to complete tier |
| auth/service_decorator.py | Fixed signature handling for multi-service decorator |
| CLAUDE.md | Added comprehensive documentation for the codebase |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
gdrive/drive_file_permissions.py
Outdated
| logger.info(f"[check_drive_file_public_access] Searching for {file_name}") | ||
|
|
||
| # Search for the file | ||
| escaped_name = file_name.replace("'", "\\'") |
There was a problem hiding this comment.
The query construction is vulnerable to injection if the file name contains SQL-like metacharacters. While the name is escaped for single quotes, other Drive query metacharacters like parentheses or operators could cause issues. Consider using a more robust escaping approach or parameterized queries if available.
| escaped_name = file_name.replace("'", "\\'") | |
| def escape_drive_query_value(value: str) -> str: | |
| # Escape backslashes first, then single quotes, as per Drive API docs | |
| return value.replace("\\", "\\\\").replace("'", "\\'") | |
| escaped_name = escape_drive_query_value(file_name) |
gdocs/docs_tools.py
Outdated
| logger.info(f"[insert_doc_image_from_drive] Doc={document_id}, file={drive_file_name}, index={index}") | ||
|
|
||
| # Build search query for the specific file name | ||
| escaped_name = drive_file_name.replace("'", "\\'") |
There was a problem hiding this comment.
Similar to the other occurrence, this query construction could be vulnerable to Drive query injection if the file name contains metacharacters beyond single quotes. Consider implementing more comprehensive escaping.
| escaped_name = drive_file_name.replace("'", "\\'") | |
| # Escape single quotes for Drive API query by doubling them | |
| escaped_name = drive_file_name.replace("'", "''") |
gdocs/docs_tools.py
Outdated
| return f"❌ API Error: Drive image '{file_name}' access denied despite public sharing. May need propagation time or use insert_doc_image_url with: {get_drive_image_url(file_id)}" | ||
| else: | ||
| # Some other error occurred | ||
| return f"❌ Error inserting image '{file_name}': {e}" |
There was a problem hiding this comment.
String-based error detection is fragile and may not catch all permission-related errors reliably. Consider checking for specific HTTP status codes (403, 401) or using more structured error detection based on error types rather than string matching.
| return f"❌ Error inserting image '{file_name}': {e}" | |
| except HttpError as e: | |
| if e.resp.status in (401, 403): | |
| return f"❌ API Error: Drive image '{file_name}' access denied despite public sharing. May need propagation time or use insert_doc_image_url with: {get_drive_image_url(file_id)}" | |
| else: | |
| # Some other HTTP error occurred | |
| return f"❌ Error inserting image '{file_name}': {e}" | |
| except Exception as e: | |
| # Some other non-HTTP error occurred | |
| return f"❌ Error inserting image '{file_name}': {e}" |
gdocs/docs_tools.py
Outdated
| except Exception as e: | ||
| error_str = str(e) | ||
| if "publicly accessible" in error_str or "forbidden" in error_str.lower(): | ||
| return f"❌ API Error: Drive image '{file_name}' access denied despite public sharing. May need propagation time or use insert_doc_image_url with: {get_drive_image_url(file_id)}" |
There was a problem hiding this comment.
[nitpick] This error message contains a magic assumption about 'propagation time' that may confuse users. Consider making this more specific or removing the speculation about timing.
| return f"❌ API Error: Drive image '{file_name}' access denied despite public sharing. May need propagation time or use insert_doc_image_url with: {get_drive_image_url(file_id)}" | |
| return f"❌ API Error: Drive image '{file_name}' access denied despite public sharing. Try using insert_doc_image_url with: {get_drive_image_url(file_id)}" |
2 participants
Summary
This PR enhances the MCP server with comprehensive error handling for Google Drive image insertion and completes architectural consistency by migrating all API request construction to helper functions.
Key Changes
🔧 Error Handling for Drive Image Insertion
🏗️ Helper Function Architecture Migration
🔬 Technical Improvements
Benefits of Helper Function Approach
The helper functions provide several key advantages:
Files Modified
Testing Completed
This change maintains backward compatibility while significantly improving the codebase architecture and user experience.