feat(drive): drive_search + drive_read — first additive service module#4
Merged
Conversation
- gdrive.py on the shared auth core: full-text search (escaped fullText query, trashed excluded), read-as-text (Docs -> text/plain, Sheets -> text/csv, Slides -> text/plain via export; text/* + JSON/XML/YAML via alt=media); binary files return metadata + a readable refusal, never mojibake - auth.request grows raw=True for non-JSON bodies (exports/media) — 401 retry and error surfacing apply uniformly - tools drive_search / drive_read (read-only, truncation with a truncated flag) - default OAuth scopes already include drive.readonly; manifest/README updated Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #4 | feat(drive): drive_search + drive_read — first additive service module
VERDICT: WARN
CI Status
- test: in_progress
Diff Review
auth.py:raw=Truepath added cleanly — backward-compatible (defaults False), error handling unchanged, returnsresp.textfor exports/mediagdrive.py(new):search()escapes user input correctly for Drive's fullText syntax;read()handles Google-native export, raw text fetch, and binary refusal with sensible mime-type dispatch__init__.py: two new@toolwrappers follow the existing_run/_credspattern consistently- Docs + manifest updated to reflect the new Drive tools
Observations
- LOW: clawpatch structural review unavailable (502 — checkout cache couldn't resolve git ref); reviewed from diff alone
- LOW:
gdrive.search()passesmax_resultsdirectly tomin(int(max_results), 50)— a negative value would produce an invalid pageSize, but the tool wrapper caps at 50 and the default is 20, so this is unlikely in practice - LOW:
gdrive.read()truncation on negativemax_charsreturns empty content withtruncated=True— harmless but odd; considermax(1, int(max_chars))if you want a floor
— Quinn, QA Engineer
|
Submitted COMMENT review on #4. |
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.
Makes good on the "built to grow" promise: Drive lands as one new module + two tool wrappers on the unchanged auth core.
What
gdrive.py:search()— free text escaped intofullText contains '…' and trashed = false(covers title + content; no orderBy — Drive rejects sorting on fullText).read()— Google-native files export as text (Docs → text/plain, Sheets → CSV, Slides → text),text/*/JSON/XML/YAML fetchalt=media, binary files return metadata + a readable refusal instead of mojibake. Content truncates atmax_charswith atruncatedflag.auth.request(raw=True): exports/media aren't JSON; the raw path keeps the 401-retry + Google-error surfacing uniform.drive_search(query, max)·drive_read(file_id, max_chars)— read-only, matching the pull-mode posture.drive.readonly(feat: one-click OAuth connect — consent flow, token persistence, Connect button #2), andwww.googleapis.comwas already in the manifest's network capability.Test
43 passed (5 new), ruff clean, host-free.
🤖 Generated with Claude Code