Skip to content

Mark page pulls destructive in MCP #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
keithah wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Mark page pulls destructive in MCP #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0efee25
Mark page pulls destructive in MCP
Sep 10, 2026
d5dee02
Document page pull MCP confirmation
Sep 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion internal/cmd/mcp/mcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,15 @@ func commandTool(c *cobra.Command, path []string, flags *pflag.FlagSet) *sdk.Too
// unless --no-increment is passed.
annotations := &sdk.ToolAnnotations{}
switch c.Name() {
case "list", "view", "get", "show", "status", "preview", "pull", "url", "search", "download":
case "list", "view", "get", "show", "status", "preview", "url", "search", "download":
annotations.ReadOnlyHint = true
case "pull":
if len(path) != 2 || path[0] != "pages" {
annotations.ReadOnlyHint = true
} else {
value := true
annotations.DestructiveHint = &value
}
Comment on lines +157 to +163

@greptile-apps greptile-apps Bot Sep 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Agent guidance remains stale

This change marks pages_pull as destructive, but the canonical agent guidance in skills/gumroad/SKILL.md still says pull-style MCP tools are read-only. The repository requires command behavior changes to be reflected in the skill documentation, so this requirement must be satisfied before merging to prevent agents from relying on incorrect annotation guidance.

Context Used: CLAUDE.md (source)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/cmd/mcp/mcp.go
Line: 157-163

Comment:
**Agent guidance remains stale**

This change marks `pages_pull` as destructive, but the canonical agent guidance in `skills/gumroad/SKILL.md` still says pull-style MCP tools are read-only. The repository requires command behavior changes to be reflected in the skill documentation, so this requirement must be satisfied before merging to prevent agents from relying on incorrect annotation guidance.

**Context Used:** CLAUDE.md ([source](https://github.com/antiwork/gumroad-cli/blob/main/CLAUDE.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@keithah keithah Sep 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d5dee02. The canonical MCP guidance now distinguishes non-page pull commands from pages_pull, which carries destructiveHint because it writes a local HTML file. Focused MCP and embedded-skill tests passed.

default:
value := true
annotations.DestructiveHint = &value
Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/mcp/mcp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ func TestEnumerationAndMetadata(t *testing.T) {
t.Errorf("empty description for %s", name)
}
}
for _, name := range []string{"products_list", "products_view", "pages_pull"} {
for _, name := range []string{"products_list", "products_view"} {
if tools[name] == nil || !tools[name].Annotations.ReadOnlyHint {
t.Errorf("missing read-only hint: %s", name)
}
}
for _, name := range []string{"sales_refund", "products_delete", "files_abort", "licenses_verify", "pages_push", "emails_send", "products_create"} {
for _, name := range []string{"sales_refund", "products_delete", "files_abort", "licenses_verify", "pages_pull", "pages_push", "emails_send", "products_create"} {
a := tools[name].Annotations
if a.DestructiveHint == nil || !*a.DestructiveHint {
t.Errorf("missing destructive hint: %s", name)
Expand Down
2 changes: 1 addition & 1 deletion skills/gumroad/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Tools follow CLI leaf command paths with underscores, including hyphens converte

Every call uses a fresh command with `--json --no-input --quiet` and automatically passes `--yes` where available. Mutations run immediately, without interactive confirmation; require approval in the MCP client before sending a mutating call. Use `dry-run: true` when supported to preview requests. Auth, admin, completion, skill, help, MCP itself, and hidden/deprecated commands are excluded. The server uses only the seller token, never the admin token.

Only connect trusted clients: tools have the same local file access as the CLI, including uploads and downloads. File paths are on the machine running the server. Stdin-based content input is unavailable; provide file paths or explicit flags instead. HTTP transport is not supported. Annotations are conservative: list/view/get/preview/pull/download-style commands are marked read-only; every other tool (including `licenses_verify`, which increments uses unless `no-increment` is true, `pages_push` and `emails_send`) carries `destructiveHint` so clients ask before running it. They describe the command category, not a security boundary (downloads still write local files).
Only connect trusted clients: tools have the same local file access as the CLI, including uploads and downloads. File paths are on the machine running the server. Stdin-based content input is unavailable; provide file paths or explicit flags instead. HTTP transport is not supported. Annotations are conservative: list/view/get/preview/download-style commands and non-page `pull` commands are marked read-only; `pages_pull` carries `destructiveHint` because it writes a local HTML file. Every other tool (including `licenses_verify`, which increments uses unless `no-increment` is true, `pages_push` and `emails_send`) also carries `destructiveHint` so clients ask before running it. They describe the command category, not a security boundary (downloads still write local files).

## Response shapes

Expand Down