Repository navigation
Conversation
Drive get already forces requested false bools into JSON. List, search, and sites dropped them because omitempty ran with no ForceSendFields. Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. What this changesPreserve requested false booleans in JSON from Drive and Sites list/search commands. Example: Run gog drive ls --fields 'files(id,trashed)' --json for file f1 with trashed=false.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review This PR fixes a current-main omission and remains worth merging; the previous changelog finding is resolved, and no actionable defects remain. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherDrive and Sites commands request file metadata through the Google SDK, then pass decoded File objects through a shared writer that produces CLI JSON. flowchart LR
A[Drive and Sites commands] --> B[List field mask]
B --> C[Drive API client]
C --> D[SDK File objects]
D --> E[Shared list writer]
E --> F[CLI JSON]
Technical reviewBest possible solution: Land the shared-writer repair while retaining existing request masks, output envelopes, and conditional-field exclusions. Do we have a high-confidence way to reproduce the issue? Current-main serialization and the pinned SDK establish the omission, supported by supplied failing-before and passing-after command-path tests; this review ran no target code. Is this the best way to solve the issue? Reusing the existing helper in the shared writer fixes the demonstrated list masks for all callers without duplicating serialization policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against ec34fedc23dd. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
Reviewed October 21, 2026, 10:24 PM ET / October 22, 2026, 02:24 UTC (Revision 2). |
List JSON now keeps requested false fields. Record that under 0.46.1. Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
|
@clawsweeper re-review The body now has shipped gog drive ls JSON from this branch. The default list contains hasThumbnail false, and files(id,trashed) contains trashed false. CHANGELOG.md Unreleased credits this pull. |
|
🦞👀 Re-review progress:
|
Preserve Gmail search results and reduce retry work: back off on recognized 403 rate limits, retain response bodies and read errors, cancel remaining detail fetches after a failure, and keep quoted operators out of label filters. Include nonempty Cc/Bcc in thread text and isolate reply/forward header direction without altering plain text or quoted HTML. Preserve requested false booleans in Drive and Sites list JSON for grouped fields, slash paths, and wildcards while leaving conditional fields omitted. Reproduces and incorporates #1242, #1246, #1247, #1248, and #1250; addresses #1241. Focused regression tests and independent Codex review pass. Co-authored-by: Serghei <xyrolle@gmail.com> Co-authored-by: Sebastien Tardif <SebTardif@ncf.ca> Co-authored-by: Kit <kit@alexpriest.com> Co-authored-by: Malo Bourgon <mbourgon@gmail.com>
|
Thanks for this fix. Incorporated into #1253 with contributor credit and a changelog acknowledgment: preserving requested false booleans in Drive and Sites list JSON, including grouped fields, slash paths, and wildcard selectors. The regression was reproduced before the change. Focused regression tests, the full remote |
What Problem This Solves
drive ls,drive search, and sites list/search JSON dropped requested false booleans.drive.Filemarks those fields omitempty, and onlydrive getset ForceSendFields. A list mask such asfiles(id,trashed)only exposes the namefilesto the top-level scan, sohasThumbnail: falseandtrashed: falsenever reached the JSON.Evidence
Before the fix, the same Drive JSON command failed because those fields were absent:
After the fix, that command completed and the existing get cases still kept requested false bools:
The non-verbose form of the same filter also completed:
ok github.com/openclaw/gogcli/internal/cmd 1.888s.Real behavior proof
gog drive ls --jsondropped requested false bools. The default mask droppedhasThumbnail:false.--fields files(id,trashed)droppedtrashed:false.cmd/gogfrom this branch (0e831a4).GOG_GOOGLE_API_BASE_URLpointed athttp://127.0.0.1:63193, a local Drive stand-in. No Google account was used.gog drive ls --jsonandgog drive ls --fields files(id,trashed) --json.files(id,trashed)list JSON:hasThumbnail:false. The explicit mask containstrashed:false. The stand-in also returnedname, so that string is in both objects.