feat(gmail): add --inline and --text content modes to gmail attachment

[chrischall]

Problem

gog gmail attachment <messageId> <attachmentId> downloads the bytes, writes them to a local file, and returns only that path. When the caller is a remote MCP client (Claude), the path is unreadable — every attachment download is a dead end.

Changes

• --inline — returns the content base64-encoded in a contentBase64 JSON field for attachments up to 3 MiB (maxInlineAttachmentBytes). Oversized attachments keep the existing path-only behavior plus an explanatory reason, so callers get a clear signal instead of silently unusable output.
• --text — returns extracted text in a text field:
  • PDFs via github.com/ledongthuc/pdf (pure Go, panic-guarded); a note flags scanned/image-only PDFs with no text layer.
  • HTML: tags/scripts/styles stripped (gmailcontent.StripHTMLTags).
  • Plain text (and JSON/XML/YAML): returned verbatim.
  • Unsupported types (e.g. images) return a reason pointing at --inline/--out.
  • Mutually exclusive with --inline (usage error).
• Both modes resolve filename/mimeType from the message payload, with a single-attachment fallback — Gmail attachment IDs are not stable across API responses, so an exact ID match can miss — and content sniffing (%PDF- header, http.DetectContentType) when metadata is unavailable.
• The file is still written as before; default output (no new flags) is byte-for-byte unchanged ({path, cached, bytes}), so existing local workflows are unaffected.
• gog gmail get was checked for the same gap: bodies are already inlined (BestBodyText); only attachments resolve to IDs, which these flags now cover.

Tests

TDD throughout (12 new command-level tests against the httptest Gmail API): small-attachment inline round-trip, oversize fallback with reason, default-output unchanged, flag mutual exclusion, PDF extraction (hand-assembled minimal PDF fixture), image-only PDF note, HTML tag-stripping, plain-text verbatim, unsupported-type reason, oversize text reason, unstable-attachment-ID metadata fallback, no-metadata content sniffing.

make lint test passes. Live-verified against a real Gmail account: default/--text/--inline on a real PDF attachment (text extracted; base64 round-trips to the exact original bytes) — both via the CLI and end-to-end through the gogcli-mcp stdio server.

🤖 Generated with Claude Code

---

Review response (5e4cd61)

• [P1] --text --plain multiline output fixed: plain output is now a stable one-record-per-line TSV — any value containing tabs/newlines (extracted text) is emitted as a single Go-quoted field. Regression test round-trips multiline text through strconv.Unquote and asserts every row stays a 2-field record.
• PDF parsing bounded: extraction of attacker-controlled PDFs now runs under a 10 s timeout (pdfExtractTimeout), on top of the existing 3 MiB input cap and panic recovery, so a pathological PDF cannot hang the command or a long-lived MCP server. Added an adversarial corrupt-PDF fixture test (clear reason, no panic) and unit tests for the timeout path.
• Trust-boundary decision: acknowledged as the maintainer's call. --text is cleanly separable (gmail_attachment_text.go + its tests) — happy to split it into a follow-up PR and land only --inline here if that's the preferred boundary.

Live proof (redacted)

Real Gmail account, binary built from this branch, run against a real PDF receipt attachment (59,855 bytes). IDs, filename, and all personal/transaction content redacted; structure and byte counts are verbatim.

CLI

$ gog gmail attachment <messageId> <attachmentId> --json   # default — unchanged
{
  "bytes": 59855,
  "cached": false,
  "path": "~/[redacted]/[redacted].pdf"
}

$ gog gmail attachment <messageId> <attachmentId> --json --text
{
 "bytes": 59855,
 "cached": true,
 "filename": "[redacted].pdf",
 "mimeType": "application/pdf",
 "path": "~/[redacted]/[redacted].pdf",
 "text": "Thanks for your purchase!\nQuestions? Visit \nhttps://support.github.com/contact\n.\n[…rest of extracted receipt text redacted…]"
}

$ gog gmail attachment <messageId> <attachmentId> --json --inline
{
 "bytes": 59855,
 "cached": true,
 "contentBase64": "JVBERi0xLjMKJf////8KMSAwIG9iago8PCAvQ3JlYXRvciA8…[79718 base64 chars total — decodes to the exact 59855 original bytes, %PDF header verified]",
 "filename": "[redacted].pdf",
 "mimeType": "application/pdf",
 "path": "~/[redacted]/[redacted].pdf"
}

$ gog gmail attachment <messageId> <attachmentId> --plain --text   # multiline text stays ONE quoted TSV field
path	~/[redacted]/[redacted].pdf
cached	true
bytes	59855
filename	[redacted].pdf
mimeType	application/pdf
text	"Thanks for your purchase!\nQuestions? Visit \nhttps://support.github.com/contact\n.\n[…redacted…]"

MCP (gogcli-mcp gog_gmail_attachment over stdio JSON-RPC, GOG_PATH → this branch's binary)

DEFAULT: { "isError": false, "fields": ["bytes", "cached", "path"], "bytes": 59855 }
TEXT:    { "isError": false, "fields": ["bytes", "cached", "filename", "mimeType", "path", "text"],
           "mimeType": "application/pdf", "bytes": 59855,
           "textPreview": "Thanks for your purchase!\nQuestions? Visit \nhttps://support." }
INLINE:  { "isError": false, "fields": ["bytes", "cached", "contentBase64", "filename", "mimeType", "path"],
           "mimeType": "application/pdf", "bytes": 59855, "contentBase64Len": 79718 }

---

Review response (1d59b5b) — killable PDF extraction boundary

Addresses the remaining P1 ("the timeout only stops waiting; it cannot stop the goroutine performing the parse"):

• --text now runs PDF parsing in a separate killable process: the parent re-execs the gog binary as a hidden gmail pdf-extract child (PDF bytes on stdin → text on stdout) under exec.CommandContext, so the 10 s timeout SIGKILLs the parser outright — no orphaned CPU/memory work in a long-lived MCP server, even for repeated hostile attachments.
• Resource bounds on both sides: the child re-checks the 3 MiB input cap; the parent caps extracted-text output at 4 MiB (compressed content streams can inflate past the input cap) and fails the child on overflow.
• Tests exercise the real subprocess boundary (the test binary dispatches to the child logic via a TestMain hook): extraction through the child process, a deliberately hung child killed on timeout (TestExtractPDFTextIsolated_KillsChildOnTimeout completes in ~0.1 s with a 100 ms timeout — the child does not linger), corrupt-PDF stderr propagation, direct child-command IO, and oversized-input rejection.
• Live re-verified: --text on the same real Gmail PDF extracts through the subprocess path; gog gmail pdf-extract < corrupt.pdf exits 1 with not a PDF file: missing %EOF.

If the maintainer still prefers landing --inline alone, --text remains cleanly separable — say the word and I'll split it.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 6:36 PM ET / 22:36 UTC.

Summary
The PR adds --inline base64 output and --text extraction for Gmail attachments, with metadata fallback, bounded subprocess-isolated PDF parsing, tests, documentation, and a new PDF dependency.

Reproducibility: not applicable. This PR adds new attachment content-return modes rather than repairing behavior already promised by the current command contract.

Review metrics: 3 noteworthy metrics.

• Patch surface: 819 added, 15 removed across 12 files. The change is substantially broader than a flag-only transport addition because text extraction introduces parser, subprocess, dependency, and test-harness code.
• Parser dependencies: 1 new third-party PDF module. The new dependency processes attacker-controlled Gmail content and therefore needs explicit ownership beyond ordinary CI validation.
• Behavior coverage: 12 command-level scenarios plus subprocess unit coverage. The test set covers default compatibility, content modes, metadata fallback, malformed input, resource bounds, and process termination.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

• [P1] Merging --text makes the core binary responsible for parsing attacker-controlled email documents through a new third-party dependency, even though the implementation now isolates and bounds that work.
• [P2] PDF parsing remains availability-sensitive across supported operating systems and long-lived MCP use; the subprocess timeout and size caps reduce this risk but do not decide whether core should permanently own it.

Maintainer options:

1. Split parser from transport (recommended)
   Merge --inline separately and defer --text until maintainers explicitly approve the document-parser boundary.
2. Own the bounded parser
   Accept the dependency and subprocess design after a maintainer explicitly confirms that PDF extraction belongs in the core CLI.
3. Pause the feature
   Close or defer the PR if neither a split nor permanent parser ownership is desirable for the core project.

Next step before merge

• [P2] A maintainer should choose between narrowing the PR to --inline and explicitly accepting PDF/text extraction as a supported core trust boundary; no remaining narrow mechanical defect is suitable for an automated repair lane.

Maintainer decision needed

• Question: Should core gog own PDF and text extraction for attacker-controlled Gmail attachments, or should this PR be narrowed to the parser-free --inline mode?
• Rationale: The implementation is now bounded, killable, tested, and proven, so the remaining blocker is not a mechanical defect; it is whether the repository wants a third-party document parser and hidden subprocess within its permanent core trust boundary.
• Likely owner: steipete — This is a repository-wide product and trust-boundary decision rather than a remaining line-level Gmail defect.
• Options:
  • Land inline only (recommended): Split out --text and merge the parser-free base64 transport mode that directly solves remote filesystem isolation.
  • Accept core extraction: Keep both modes and explicitly own the pinned PDF parser, subprocess protocol, resource limits, and future parser security updates in core.

Security
Cleared: No concrete exploitable defect remains after the added size caps, panic recovery, killable subprocess timeout, bounded output, and pinned dependency; the unresolved concern is explicit ownership of the expanded trust boundary.

Review details

Best possible solution:

Land the narrow --inline transport capability now and split --text into follow-up work unless maintainers explicitly accept the PDF parser, dependency, subprocess command, and long-term security maintenance boundary in core.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR adds new attachment content-return modes rather than repairing behavior already promised by the current command contract.

Is this the best way to solve the issue?

Unclear: --inline is the narrowest maintainable solution to remote clients lacking filesystem access, while in-core PDF extraction is appropriate only if maintainers explicitly accept its broader trust and dependency boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e4239f8c57a5.

Label changes

Label changes:

• 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. Sufficient (live_output): Redacted live CLI and MCP output demonstrates the changed behavior after the fix, including unchanged defaults, PDF text extraction, exact base64 round-tripping, and multiline plain-output parsing.
• remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
• remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

• P2: This is a useful, well-proven Gmail improvement with limited direct blast radius, but it requires a core trust-boundary decision before merge.
• merge-risk: 🚨 security-boundary: The PR adds a third-party parser for attacker-controlled email attachments to the core binary and exposes its isolated child command internally.
• merge-risk: 🚨 availability: Malformed or adversarial PDFs invoke parser work whose CPU, memory, timeout, and termination behavior matters for long-lived MCP processes and supported platforms.
• 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. Sufficient (live_output): Redacted live CLI and MCP output demonstrates the changed behavior after the fix, including unchanged defaults, PDF text extraction, exact base64 round-tripping, and multiline plain-output parsing.
• proof: sufficient: Contributor real behavior proof is sufficient. Redacted live CLI and MCP output demonstrates the changed behavior after the fix, including unchanged defaults, PDF text extraction, exact base64 round-tripping, and multiline plain-output parsing.

Evidence reviewed

What I checked:

• Current behavior remains missing on the reviewed base: At base SHA e4239f8, GmailAttachmentCmd has no inline or text flags and returns only path, cached status, and byte count, so this PR is not redundant with current main. (internal/cmd/gmail_attachment.go:21, e4239f8c57a5)
• Parser isolation and resource bounds: The head revision runs PDF extraction in a separately killable process, rechecks the 3 MiB input limit, caps extracted output at 4 MiB, propagates parser errors, and retains timeout enforcement. (internal/cmd/gmail_attachment_text.go:1, 1d59b5bf2f4a)
• Regression coverage: Command and unit tests cover inline round-trips, unchanged default output, text formats, unstable attachment metadata, corrupt PDFs, output caps, and actual subprocess termination on timeout. (internal/cmd/gmail_attachment_text_unit_test.go:1, 1d59b5bf2f4a)
• Real behavior proof: The PR body contains redacted live CLI and MCP transcripts showing unchanged default output, extracted PDF text, exact base64 byte round-tripping, and parseable multiline plain output after the fixes. (1d59b5bf2f4a)
• Re-review continuity: The current head matches the last completed review SHA; the earlier plain-output and non-terminating-parser findings were addressed, and no previous finding remains open. (1d59b5bf2f4a)
• Attachment-area history: Repository release history credits salmonumbrella with the original Gmail thread attachment workflow and later attachment path/cache hardening, while zerone0x is credited for recent attachment destination behavior. (CHANGELOG.md:1, e4239f8c57a5)

Likely related people:

• salmonumbrella: Repository history credits this contributor with the original Gmail thread attachment workflow and later attachment path and cache hardening. (role: attachment feature owner; confidence: high; files: internal/cmd/gmail_attachment.go, internal/cmd/gmail.go)
• zerone0x: Release history credits this contributor with recent gmail attachment --out directory and filename behavior in the same command path. (role: recent adjacent contributor; confidence: medium; files: internal/cmd/gmail_attachment.go)
• steipete: Signed release history and repository stewardship make this the strongest available routing candidate for deciding whether a document parser belongs in the core binary. (role: release integrator and likely product decision owner; confidence: medium; files: go.mod, internal/cmd/gmail.go, internal/cmd/gmail_attachment.go)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (4 earlier review cycles)

• reviewed 2026-07-13T21:33:55.542Z sha e0dd518 :: needs real behavior proof before merge. :: [P2] Preserve parseable plain output for extracted text
• reviewed 2026-07-13T21:52:45.258Z sha e0dd518 :: needs real behavior proof before merge. :: [P2] Preserve parseable plain output for extracted text
• reviewed 2026-07-13T22:08:57.415Z sha 5e4cd61 :: found issues before merge. :: [P1] Make the PDF timeout terminate parser work
• reviewed 2026-07-13T22:29:15.230Z sha 1d59b5b :: needs maintainer review before merge. :: none

[chrischall]

@clawsweeper re-review — pushed 5e4cd61 fixing the --text --plain TSV contract (Go-quoted single field + regression test) and bounding PDF extraction with a 10s timeout + adversarial corrupt-PDF fixture; the PR body now includes redacted live CLI and MCP transcripts for default/--inline/--text and the fixed --plain output.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review published an exact durable review comment and queued an executing target-wide serialized router scan.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29288661816
• Updated: 2026-07-13T22:11:12.276Z

[chrischall]

@clawsweeper re-review — 1d59b5b replaces the goroutine timeout with a killable subprocess boundary: PDF parsing re-execs gog as a hidden gmail pdf-extract child under exec.CommandContext, so timeout = SIGKILL (no lingering parser work in long-lived MCP servers). Child re-checks the 3 MiB input cap, parent caps extracted text at 4 MiB. Tests spawn the real subprocess including a hung-child kill test; PR body has details and live re-verification.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review published an exact durable review comment and queued an executing target-wide serialized router scan.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29290197893
• Updated: 2026-07-13T22:38:37.870Z