Skip to content

fix: keep requested false bools in Drive list JSON - #1246

Closed
SebTardif wants to merge 2 commits into
openclaw:mainfrom
SebTardif:fix/drive-list-false-bools
Closed

SebTardif wants to merge 2 commits into
openclaw:mainfrom
SebTardif:fix/drive-list-false-bools

Conversation

@SebTardif

@SebTardif SebTardif commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What Problem This Solves

drive ls, drive search, and sites list/search JSON dropped requested false booleans. drive.File marks those fields omitempty, and only drive get set ForceSendFields. A list mask such as files(id,trashed) only exposes the name files to the top-level scan, so hasThumbnail: false and trashed: false never reached the JSON.

Evidence

Before the fix, the same Drive JSON command failed because those fields were absent:

go test ./internal/cmd/ -count=1 -run 'TestDriveGet_JSONKeepsRequestedFalseBools|TestDriveLs_JSON|TestDriveList_JSON|TestDriveSearch_JSON'
--- FAIL: TestDriveLs_JSONKeepsRequestedFalseBools (0.00s)
    --- FAIL: TestDriveLs_JSONKeepsRequestedFalseBools/default_fields (0.02s)
        drive_fields_test.go:299: missing "hasThumbnail" in output: {
              "count": 1,
              "files": [
                {
                  "id": "f1",
                  "name": "photo.png"
                }
              ],
              "has_more": false,
              "nextPageToken": ""
            }
    --- FAIL: TestDriveLs_JSONKeepsRequestedFalseBools/files_mask (0.02s)
        drive_fields_test.go:299: missing "trashed" in output: {
              "count": 1,
              "files": [
                {
                  "id": "f1"
                }
              ],
              "has_more": false,
              "nextPageToken": ""
            }
FAIL
FAIL	github.com/openclaw/gogcli/internal/cmd	2.787s
FAIL

After the fix, that command completed and the existing get cases still kept requested false bools:

go test ./internal/cmd/ -count=1 -v -run 'TestDriveGet_JSONKeepsRequestedFalseBools|TestDriveLs_JSONKeepsRequestedFalseBools'
--- PASS: TestDriveLs_JSONKeepsRequestedFalseBools (0.00s)
    --- PASS: TestDriveLs_JSONKeepsRequestedFalseBools/default_fields (0.01s)
    --- PASS: TestDriveLs_JSONKeepsRequestedFalseBools/files_mask (0.01s)
--- PASS: TestDriveGet_JSONKeepsRequestedFalseBools (0.00s)
    --- PASS: TestDriveGet_JSONKeepsRequestedFalseBools/default_fields (0.01s)
    --- PASS: TestDriveGet_JSONKeepsRequestedFalseBools/explicit_fields (0.01s)
    --- PASS: TestDriveGet_JSONKeepsRequestedFalseBools/conditional_bools_stay_absent (0.01s)
    --- PASS: TestDriveGet_JSONKeepsRequestedFalseBools/true_value_and_nested_wildcard (0.01s)
PASS
ok  	github.com/openclaw/gogcli/internal/cmd	1.877s

The non-verbose form of the same filter also completed: ok github.com/openclaw/gogcli/internal/cmd 1.888s.

Real behavior proof

  • Behavior or issue addressed: gog drive ls --json dropped requested false bools. The default mask dropped hasThumbnail:false. --fields files(id,trashed) dropped trashed:false.
  • Real environment tested: Windows, Go 1.27.0. Built cmd/gog from this branch (0e831a4). GOG_GOOGLE_API_BASE_URL pointed at http://127.0.0.1:63193, a local Drive stand-in. No Google account was used.
  • Exact steps or command run after this patch: gog drive ls --json and gog drive ls --fields files(id,trashed) --json.
  • Evidence after fix: Default list JSON:
{
  "count": 1,
  "files": [
    {
      "hasThumbnail": false,
      "id": "f1",
      "name": "notes"
    }
  ],
  "has_more": false,
  "nextPageToken": ""
}

files(id,trashed) list JSON:

{
  "count": 1,
  "files": [
    {
      "id": "f1",
      "name": "notes",
      "trashed": false
    }
  ],
  "has_more": false,
  "nextPageToken": ""
}
  • Observed result after fix: The default list contains hasThumbnail:false. The explicit mask contains trashed:false. The stand-in also returned name, so that string is in both objects.
  • What was not tested: a live Google Drive account. Drive search and sites list were not run as separate commands. They use this same list writer.

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>
@clawsweeper

clawsweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Oct 22, 2026
@clawsweeper

clawsweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

Preserve 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.

  • Before: The file object contains id=f1 but omits trashed.
  • After: The file object includes "trashed": false.

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) A focused repair with shipped CLI proof, meaningful regression coverage, green exact-head checks, and the earlier finding resolved.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The supplied Windows output exercises the built gog CLI at this head against a local Drive stand-in and shows both repaired masks through the production SDK and shared writer; live Google access and separate sibling commands are outside the demonstrated scope.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Scripts lose explicitly requested false metadata when Drive list responses are serialized to JSON.
Reason: The repair makes list output consistent with the existing get behavior and preserves the prior fix's intentional exclusions.

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
Reviewed head: 0e831a4100dafea3fc4f9f1684954dbd4d290e0f

Before merge

None.

Findings

None.

Agent review details

How this fits together

Drive 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]
Loading

Technical review

Best 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

  • Requested boolean preservation keeps the original intent (bfac16e: Preserve requested top-level false booleans while retaining conditional and nested omission.)
  • List field-mask selection keeps the original intent (7b288cc: Allow explicit list field masks while retaining the default mask.)
  • Shared Drive and Sites list output keeps the original intent (1710b54: Extract shared listing flows; [codex] add Google Sites support #574 subsequently reused them for Sites.)
  • List aggregates and projection keeps the original intent (3dd0bf1: Expose count and has_more while preserving continuation tokens and results-only projection.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

  • Current-main omission: The default branch still serializes list File objects without invoking the boolean-preservation helper; GitHub confirms main remains at the pinned base. (internal/cmd/drive_listing.go:127, ec34fedc23dd)
  • Introduced repair: The pinned introduced diff applies the existing boolean helper in the shared JSON writer and supplies the matching request mask from all four callers. (internal/cmd/drive_listing.go:127, 0e831a4100da)
  • Imported SDK serialization contract: The target imports google.golang.org/api v0.306.0. Its File fields HasThumbnail and Trashed use omitempty, and File.MarshalJSON honors ForceSendFields; this imported contract directly explains the omission. (drive/v3/drive-gen.go:1910, 0d9c67e59dc9)
  • Earlier fix has a distinct command scope: fix(drive): retain requested false metadata fields #1223 repaired drive get and deliberately preserved conditional and nested omissions; this PR reuses those exclusions for lists. The maintainer's landing comment confirms that earlier scope. (internal/cmd/drive_fields.go:15, bfac16e00c98)
  • Updated shipped CLI proof: The complete current PR body records Windows CLI runs from this head against a local Drive endpoint, showing hasThumbnail:false for the default mask and trashed:false for files(id,trashed). The contributor explicitly identifies the stand-in and the untested live-account scope. (0e831a4100da)
  • Previous finding resolved: The only code change since the earlier reviewed head adds an Unreleased changelog entry with the PR reference and contributor thanks, satisfying root AGENTS.md. (CHANGELOG.md:5, 0e831a4100da)

Likely related people:

  • Peter Steinberger: Raw commit 1710b54 adds internal/cmd/drive_listing.go:91 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 1710b549bce4; files: internal/cmd/drive_listing.go)
  • postoso: Raw commit bfac16e adds internal/cmd/drive_fields.go:28 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: bfac16e00c98; files: internal/cmd/drive_fields.go)
  • Shadow: Raw commit 806d1b0 adds internal/cmd/sites.go:63 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 806d1b01088a; files: internal/cmd/sites.go)

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: Missing false metadata affects scripts consuming Drive and Sites list JSON, with limited scope and no demonstrated urgent regression.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/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.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-22T02:10:43.853Z sha dd907b5 :: needs real behavior proof before merge. :: [P3] Add the required Unreleased changelog entry

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>
@SebTardif

Copy link
Copy Markdown
Contributor Author

@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.

@clawsweeper

clawsweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Oct 22, 2026
steipete added a commit that referenced this pull request Oct 23, 2026
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>
@steipete

Copy link
Copy Markdown
Collaborator

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 make ci gate, and independent Codex review passed. Exact-head CI: https://github.com/openclaw/gogcli/actions/runs/39888593907. Landed as 60ae623da5b5955b5e3127d8bb9d5f63d23f8397. Closing this PR because the fix is now on main.

@steipete steipete closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants