fix(gmail): avoid duplicate sanitized content - #992
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
Contributor
|
Codex review: found issues before merge. Reviewed August 14, 2026, 3:24 PM ET / 19:24 UTC. ClawSweeper reviewWhat this changesThe PR removes duplicate top-level sanitized headers and body from Merge readinessKeep this collaborator-authored PR open for a maintainer compatibility decision. It fixes the reported duplication, but it silently removes existing top-level JSON fields from the default sanitized Gmail response. Priority: P1 Review scores
Verification
How this fits togetherGmail read commands fetch a message, optionally sanitize unsafe content, and serialize the result for CLI JSON consumers. The JSON formatter then optionally unwraps a single primary result for automation users who request flowchart LR
A[Gmail message] --> B[Gmail read command]
B --> C[Content sanitization]
C --> D[JSON envelope selection]
D --> E[Automation output]
Decision needed
Why: Both choices are technically feasible, but selecting a breaking default JSON contract requires maintainer ownership of the upgrade policy. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve the established default sanitized envelope and make the compact sanitized message available only through the already-documented Do we have a high-confidence way to reproduce the issue? Yes—current main constructs top-level sanitized Is this the best way to solve the issue? No. The change fixes duplication, but changing the default schema is not the narrowest safe path; preserve it and make Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 35f2bb286662. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
chrischall
added a commit
to chrischall/gogcli-mcp
that referenced
this pull request
Aug 14, 2026
…nitized get (#272) Raises `MIN_GOG_VERSION` **0.35.0 → 0.37.0** (and `fly-gog-runner/Dockerfile`'s pin with it) and takes up what 0.36.0 / 0.37.0 added. ## New tools **Gmail — draft-side reply/forward** (gog 0.36.0, openclaw/gogcli#977) | Tool | | |---|---| | `gog_gmail_drafts_reply` | Save a reply as a draft — inherited recipients, subject and quote; never sends | | `gog_gmail_drafts_reply_all` | Same, to every participant | | `gog_gmail_drafts_forward` | Save a forward as a draft; `to` is optional, so it can be staged with no recipients at all | They take the same flag set as the send commands and share gog's composition path, so `replySchema` / `appendReplyFlags` are reused verbatim rather than re-declared. Staging a reply previously meant `gog_gmail_drafts_create` + `replyToThreadId`, which threads the draft but inherits neither the original's recipients nor its quoted body — both had to be rebuilt by hand, and a missed Cc is invisible until the draft goes out. **Sheets — Connected Sheets reads** (gog 0.37.0, openclaw/gogcli#938) `gog_sheets_datasource_list` / `_describe`, and `gog_sheets_datasource_table_list` / `_describe` / `_read`. Read-only by construction (gog exposes no create/update/refresh/delete here). A data-source table has no id of its own in the Sheets API — its definition lives on its top-left cell — so extracts are addressed by a sheet-qualified A1 anchor like `Extracts!B3`. ## Behaviour these depend on - **`gog_gmail_get` gains `sanitizeContent`.** The flag predates 0.37.0 but emitted the headers and body **twice** in JSON (openclaw/gogcli#992), so the flag meant to shrink the payload enlarged it. Verified against a live 0.37.0 build: the sanitized message now arrives under a single `message` key. - **`gog_calendar_events` gains `days`**, and it and `gog_calendar_search` now describe the window rules gog 0.36.0 enforces (openclaw/gogcli#981). `--days` is a window *length* anchored at `--from`; it used to discard `--from` silently and answer for today, at exit 0, in a well-formed table. Fixed presets no longer combine with `from`/`to`/`days`, and `days` no longer combines with `to`. The old descriptions invited exactly the combinations that now fail, and the base calendar test asserted an arg array gog refuses to run. - **The auth tools gain `extraScopes`** (plus `--force-consent` on the interactive one, since Google re-prompts for a *new* scope only when consent is forced). Nothing else can request `bigquery.readonly`, which Google demands whenever a Sheets response *contains* BigQuery Connected Sheets data — without it the new sheets tools cannot be authorized through the wrapper at all. Documented in `docs/auth-scopes.md`. ## Not adopted: `gmail search --count` gog 0.36.0 (openclaw/gogcli#985) upstreamed this wrapper's match-count probe, down to the page size and the exact/lower-bound split. The local probe stays, and the stale comment claiming gog *cannot* supply the count is corrected to say why: - it is spent only on a result set already known to be truncated, where `--count` is decided before the search runs and would cost every search an extra Gmail request; - it is best-effort, where gog returns the probe's error from the whole command — a failed count would turn a search that *did* succeed into an error. ## Verification Beyond the unit tests (the mocked suites only assert arg arrays), every new command was run against the real gog v0.37.0 binary with fake ids, which parses flags before any API call: - `gmail drafts reply` / `reply-all` / `forward` — full flag sets parse; failures are the API's 400 on the fake id (and a send-as validation for `--from`), not unknown flags. - `sheets datasource list` / `describe` / `table list` / `table describe` / `table read` — all reach the API and 404 on the fake spreadsheet. - `calendar events --from 2026-09-25 --days 5` returns the anchored window; `--today --from …` and `--days … --to …` are both rejected with the messages the tool descriptions now quote. - `gmail get --sanitize-content --json` on a real message: top-level keys are exactly `["message"]`. `npm test` (all workspaces + fly-gog-runner), `npm run typecheck` and `npm run build` are green; the 100% coverage gate holds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EjH6C4jVKavpHiFwBW574N Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary
messageresultmessageenvelope while allowing--results-onlyto unwrap itReporter credit is recorded in the changelog. Thanks @ronny-rentner.
Verification
go test ./internal/cmd -run '^(TestGmailGetCmd_SanitizeContent_JSONUsesSafeEnvelope|TestGmailGetCmd_JSON_ResultsOnlyPreservesCompleteAttachmentResult|TestGmailThreadGet_SanitizeContent_JSONUsesSafeEnvelope)$' -count=1make cimessage; no URL or raw payload--results-only: direct sanitized message; no wrapper, URL, or raw payloadCloses #986