Repository navigation
Two notification-subscription tools omit destructiveHint, defaulting to destructive under MCP 2025-06-18 #2841
Description
Describe the bug
Under MCP schema 2025-06-18, an omitted destructiveHint defaults to true.
Two tools currently emit readOnlyHint: false but omit destructiveHint, so a spec-conformant client will render them as destructive even though they appear to operate only on the caller's own notification-subscription state:
-
manage_notification_subscriptioninpkg/github/notifications.go:413- Updates the caller's own thread subscription state (
watch,ignore, or remove the subscription preference record at:460and:465) - This does not modify thread or repository content
- Updates the caller's own thread subscription state (
-
manage_repository_notification_subscriptioninpkg/github/notifications.go:509- Updates the caller's own repository subscription state (
subscribed,ignored, or remove the subscription preference record at:564and:569) - This does not modify repository content
- Updates the caller's own repository subscription state (
If that reading is right, both tools are non-read-only but not destructive, and should set destructiveHint: false explicitly.
I'm filing this as a question rather than asserting a bug in case I'm misreading the intended risk model.
I scoped this deliberately to these two tools. I did not include other write tools because their current hints are more arguable from source review.
Affected version
GitHub MCP Server v1.5.0
Source reviewed at commit 40db5e3
Affected code:
pkg/github/notifications.go:413pkg/github/notifications.go:509
Steps to reproduce
- Open
pkg/github/notifications.go - Inspect
manage_notification_subscriptionandmanage_repository_notification_subscription - Note that both emit
readOnlyHint: false - Note that neither sets
destructiveHint - Under MCP schema 2025-06-18, a conformant client therefore interprets both as
destructiveHint: true
Expected behavior
These per-user subscription toggles should surface as non-destructive writes.
Actual behavior
They default to destructive under schema 2025-06-18, so consent UIs over-warn on operations that appear to affect only the caller's own subscription metadata.
Logs
N/A. Static annotation review only.
If this matches the intended semantics, I'm happy to open a small PR setting destructiveHint: false explicitly on both. From the repo comments, it looks like snapshots can be regenerated with:
UPDATE_TOOLSNAPS=true go test ./...
Activity
juneja-varun commentedon Aug 14, 2026
This reading looks right to me — I'll send a small PR setting destructiveHint: false explicitly on both tools, plus regenerating the toolsnaps.
MaazAhmed47 commentedon Aug 15, 2026
Thanks for taking this forward. One point worth resolving before merge is the competing interpretation in #2936: both tools include a delete action, while the schema describes destructiveHint: false as performing only additive updates.
My original reasoning treated removal of the caller’s notification-subscription preference as non-destructive because it does not delete thread or repository content. However, because annotations apply to the entire tool, the literal “only additive updates” wording may support true.
I’d appreciate a maintainer ruling on which interpretation matches the intended annotation model.
alirezainanlosalehi7-lgtm commentedon Aug 15, 2026
juneja-varun commentedon Aug 16, 2026
@MaazAhmed47 good catch, and on reflection I think #2936 has the right read of this, not my PR.
My reasoning for
falsewas about real-world blast radius — removing your own subscription preference doesn't touch thread or repository content, so it felt low-risk. But that's not what the spec actually tests. Per the2026-07-28draft,destructiveHint: falsemeans the tool performs only additive updates, and annotations apply to the whole tool, not per-action.manage_notification_subscription'sdeleteaction genuinely callsclient.Activity.DeleteThreadSubscription— that's not additive by any literal reading, regardless of how narrow its effect is. Same applies tomanage_repository_notification_subscription's delete action.I'll close #3074 in favor of #2936 — no point carrying a duplicate with the less-defensible interpretation once a more careful reading is already up.