Skip to content

feat(mcp): add gog mcp subcommand with gog_exec tool#637

Open
auroracapital wants to merge 5 commits into
openclaw:mainfrom
auroracapital:feat/mcp-subcommand
Open

feat(mcp): add gog mcp subcommand with gog_exec tool#637
auroracapital wants to merge 5 commits into
openclaw:mainfrom
auroracapital:feat/mcp-subcommand

Conversation

@auroracapital
Copy link
Copy Markdown

Summary

  • Adds gog mcp subcommand that runs an MCP (Model Context Protocol) server over stdio
  • Exposes a single gog_exec tool that takes {"args": ["..."]} and runs gog <args> as a subprocess, returning {stdout, stderr, exit_code}
  • Adds github.com/mark3labs/mcp-go dependency (v0.54.0)

Implementation details

  • Self-path resolved via os.Executable() so the MCP server always delegates to the same binary
  • 60-second exec timeout; stdout/stderr each capped at 100 KB with truncation marker
  • Registered in CLI struct in root.go following existing kong pattern

Testing

  • gog mcp --help prints usage correctly
  • Smoke-tested with JSON-RPC tools/list request — gog_exec appears in tool list
  • Existing unit tests pass (go test ./internal/cmd/... -run TestVersion)
  • gofmt clean

Notes

  • No individual gmail/calendar/drive wrappers — the generic gog_exec tool gives agents full access via documented gog subcommands
  • gog auth is not currently configured on this machine; gog_exec calls will return auth errors at runtime, which is expected

🤖 Generated with Claude Code

Runs an MCP server over stdio exposing a single `gog_exec` tool that
delegates to the same gog binary as a subprocess, giving AI agents full
access to all gog subcommands. 60s timeout, 100KB stdout/stderr cap.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@socket-security
Copy link
Copy Markdown

socket-security Bot commented May 22, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedgolang/​github.com/​mark3labs/​mcp-go@​v0.54.073100100100100

View full report

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 22, 2026

Codex review: needs real behavior proof before merge.

Latest ClawSweeper review: 2026-05-23 07:22 UTC / May 23, 2026, 3:22 AM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR adds a root-level gog mcp stdio server with a generic subprocess-backed gog_exec tool, adds mcp-go, and changes Gmail list/search query defaults.

Reproducibility: yes. source-level: the PR diff shows MCP child invocations are built from tool args plus safety flags only, while current main root flags carry account, config-home, output, auth, and automation context. I did not run the PR because this review is read-only.

PR rating
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Summary: The PR is not quality-ready because proof is absent and the patch has security-sensitive scope, context inheritance, compatibility, and automation blockers.

Rank-up moves:

  • Get maintainer approval that core gog mcp is in scope despite the current spec non-goal.
  • Preserve the intended root execution context and safety policy for every gog_exec child invocation.
  • Restore unrelated Go/Gmail behavior changes or split them into separate PRs, then add redacted terminal JSON-RPC proof.
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.

Real behavior proof
Needs real behavior proof before merge: No after-fix real behavior proof is present; redacted terminal JSON-RPC output for tools/list and a gog_exec call would be sufficient, with private data such as tokens, account details, phone numbers, IPs, and non-public endpoints redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • Merging this would intentionally reverse the documented MCP non-goal and add a new core server/dependency without a maintainer-visible scope decision.
  • gog_exec child calls currently inherit only safety flags, so root options such as --home, --account, --client, JSON/plain output, no-input, and untrusted-content wrapping can be lost unless every tool call repeats them.
  • The Go directive change can affect CI/release automation because workflows use go-version-file: go.mod.
  • The Gmail no-query default changes existing CLI validation behavior in a PR whose stated feature is MCP.
  • The external contributor has not provided real MCP behavior proof; tests and smoke-test claims alone do not satisfy the proof gate.
  • The new mcp-go direct dependency and generic authenticated command execution surface need explicit security review before landing.

Maintainer options:

  1. Decide whether MCP belongs in core (recommended)
    Pause merge until maintainers explicitly decide whether gog mcp should override the current spec non-goal or remain an external wrapper/plugin path.
  2. Harden the MCP execution contract
    If MCP is approved, require child calls to preserve the intended root context and safety policy, with tests covering account/home/output/no-input and safety flag behavior.
  3. Split unrelated churn
    Restore the Go directive and move the Gmail no-query default into a separate reviewed PR with its own docs/tests if maintainers want that behavior.

Next step before merge
Maintainers need to decide MCP scope and security policy before repair automation is appropriate, and the contributor still needs to provide real behavior proof.

Security
Needs attention: The diff adds an agent-facing subprocess execution surface over authenticated Google Workspace commands and still needs trust-model approval plus a root-context fix.

Review findings

  • [P1] Preserve root context for MCP child executions — internal/cmd/mcp.go:78-79
  • [P2] Keep the module Go version aligned with main — go.mod:3-5
  • [P2] Split the Gmail default behavior out of this PR — internal/cmd/gmail_search.go:15-16
Review details

Best possible solution:

If maintainers want MCP in core, land it as a focused, approved PR that preserves root execution context and safety policy, documents the trust model, keeps Go/Gmail changes separate, adds focused tests, and includes redacted JSON-RPC terminal proof.

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

Yes, source-level: the PR diff shows MCP child invocations are built from tool args plus safety flags only, while current main root flags carry account, config-home, output, auth, and automation context. I did not run the PR because this review is read-only.

Is this the best way to solve the issue?

No, not as proposed: a core MCP server needs a maintainer scope decision first, and the implementation should be narrowed to preserve the effective execution context instead of mixing unrelated Gmail and Go directive changes.

Label justifications:

  • P2: This is a non-trivial feature PR with limited blast radius but real scope, compatibility, proof, and security review blockers.
  • merge-risk: 🚨 compatibility: The PR changes the root CLI surface, documented product scope, module Go directive, and Gmail no-query behavior.
  • merge-risk: 🚨 security-boundary: The PR exposes an agent-facing subprocess tool over authenticated Google Workspace commands and currently does not fully preserve root execution context.
  • merge-risk: 🚨 automation: The PR changes go.mod in a repo whose CI and release workflows select Go through go-version-file: go.mod.
  • rating: 🧂 unranked krab: Current PR rating is 🧂 unranked krab because proof is 🧂 unranked krab, patch quality is 🧂 unranked krab, and The PR is not quality-ready because proof is absent and the patch has security-sensitive scope, context inheritance, compatibility, and automation blockers.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; redacted terminal JSON-RPC output for tools/list and a gog_exec call would be sufficient, with private data such as tokens, account details, phone numbers, IPs, and non-public endpoints redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Full review comments:

  • [P1] Preserve root context for MCP child executions — internal/cmd/mcp.go:78-79
    gog_exec appends only safety flags to the tool-supplied args, so a server launched as gog --home ... --account ... --json --no-input mcp will run child commands without that configured home/account/output/automation context unless every tool call repeats it. Carry the effective root execution context into child calls, or validate/document that MCP clients must supply every root option themselves.
    Confidence: 0.86
  • [P2] Keep the module Go version aligned with main — go.mod:3-5
    This changes the module directive from go 1.26.2 to go 1.25.9 while CI uses go-version-file: go.mod and the spec says Go 1.26. Leave the directive unchanged unless the toolchain change is an intentional compatibility decision with separate review.
    Confidence: 0.88
  • [P2] Split the Gmail default behavior out of this PR — internal/cmd/gmail_search.go:15-16
    This MCP PR also changes empty Gmail search/list invocations from a validation error into a live in:inbox query, with the same behavior added to gmail messages list. That is a separate user-visible CLI change and should be reverted here or moved to its own reviewed PR with docs/tests.
    Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.88

Security concerns:

  • [high] MCP child commands can lose configured auth/config context — internal/cmd/mcp.go:78
    The child process receives tool args and safety flags only, so root options such as --home, --account, --client, --json, --no-input, and untrusted wrapping are not preserved from the server invocation; that can make agent deployments use the wrong credential/config context or fail differently than configured.
    Confidence: 0.86
  • [medium] New command-execution surface needs explicit trust review — internal/cmd/mcp.go:67
    The PR intentionally exposes generic subprocess access to all gog subcommands and adds a new direct MCP dependency; this is security-sensitive even though the latest head now propagates safety flags.
    Confidence: 0.82

What I checked:

  • Current spec lists MCP server support as out of scope: Current main documents Running an MCP server (this is a CLI) under non-goals, so adding gog mcp needs explicit product direction. (docs/spec.md:33, 96d50e5510ce)
  • Vision requires discussion for this kind of change: VISION.md says new API surfaces, hard-to-validate features, behavior changes that can break scripts, new dependencies, and long-running/background machinery should be discussed first. (VISION.md:23, 96d50e5510ce)
  • PR child execution preserves only safety flags: The latest PR appends args plus safetySuffix when spawning the child binary; root context such as --home, --account, --client, output mode, and no-input behavior is not carried unless each tool call repeats it. (internal/cmd/mcp.go:78, e7427cec582f)
  • Current root flags include auth, config, output, and automation context: Current main binds root flags and applies config home, output mode, client, access token, and safety checks before handlers run; MCP subprocess calls need a deliberate answer for which of those settings are inherited. (internal/cmd/root.go:30, 96d50e5510ce)
  • Agent guidance relies on root-level account and output flags: The bundled gog skill tells agents to pick accounts explicitly and prefer --json --wrap-untrusted, so an MCP bridge that drops parent root context can fail or return different output than the launched server implies. (.agents/skills/gog/SKILL.md:21, 96d50e5510ce)
  • PR changes the module Go directive used by automation: Current main declares go 1.26.2, while the PR changes it to go 1.25.9 plus toolchain go1.26.2; CI reads go-version-file: go.mod, so this is automation-relevant churn. (go.mod:3, 96d50e5510ce)

Likely related people:

  • Peter Steinberger: Blame points to the v0.19.0 release commit for the current root command enforcement, project scope docs, Go directive, and Gmail search/list query behavior. (role: current area author; confidence: high; commits: b25a3c029b37, f26af3adba6d, 6e0af2c46329; files: internal/cmd/root.go, docs/spec.md, VISION.md)
  • Drew Burchfield: Recent safety-profile hardening commit is directly relevant to whether an agent-facing MCP subprocess bridge preserves the intended safety boundary. (role: adjacent safety hardening contributor; confidence: medium; commits: 46900109e029, f26af3adba6d; files: internal/cmd/safety_profile.go, internal/cmd/safety_profile_default.go, internal/safetyprofile/profile.go)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 96d50e5510ce.

@clawsweeper clawsweeper Bot added 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. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels May 22, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 22, 2026

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

Allows 'gog gmail list' and 'gog gmail messages list' to run without a
positional query argument — defaults to in:inbox so the bare command
shows the user's inbox. Also adds a -q/--query flag as an alias for
the positional argument for ergonomic scripting.

Applies to both GmailSearchCmd (gmail list) and GmailMessagesSearchCmd
(gmail messages list) which shared the same UX pattern.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@auroracapital
Copy link
Copy Markdown
Author

Pushed follow-up commit c6964c4 — makes gog gmail list and gog gmail messages list work without a positional query (defaults to in:inbox) and adds a -q/--query flag as an alias for the positional.

Before: gog gmail list errored with expected "<query> ...". After: shows the inbox.

Existing behavior preserved when the positional or flag is provided. Same fix applied to gmail messages list which shared the UX. go test ./... green, go vet clean, gofmt clean.

Use utf8.Valid when trimming output so truncated slices never end with an
orphan lead byte. Derive exec CommandContext from the MCP handler context
so client cancellation ends subprocess work before the hard timeout.
Prepend --gmail-no-send, --enable-commands, and --disable-commands from the
parent invocation so MCP-spawned gog runs honor the same restrictions as
the parent process.
Kong resolves duplicate global flags last-wins; user-supplied MCP args
must not be able to follow and override --enable-commands, --disable-commands,
or --gmail-no-send enforced by the parent MCP process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants