Skip to content

fix(notifications): set destructiveHint false on subscription tools#2845

Open
syf2211 wants to merge 1 commit into
github:mainfrom
syf2211:fix/2841-notification-subscription-destructive-hint
Open

fix(notifications): set destructiveHint false on subscription tools#2845
syf2211 wants to merge 1 commit into
github:mainfrom
syf2211:fix/2841-notification-subscription-destructive-hint

Conversation

@syf2211

@syf2211 syf2211 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Explicitly mark manage_notification_subscription and manage_repository_notification_subscription as non-destructive MCP tools.

Why

Fixes #2841

Under MCP schema 2025-06-18, an omitted destructiveHint defaults to true when readOnlyHint is false. These tools only update the caller's own notification subscription preferences (watch/ignore/delete subscription record) and do not modify thread or repository content, so conformant clients were over-warning on consent UIs.

What changed

  • Set DestructiveHint: jsonschema.Ptr(false) on both notification subscription tools in pkg/github/notifications.go
  • Add regression assertions in pkg/github/notifications_test.go
  • Update tool snapshots for both tools

MCP impact

  • Tool schema or behavior changed
  • Annotation-only change: clients now receive destructiveHint: false instead of inheriting the schema default of true.

Prompts tested (tool changes only)

  • N/A (annotation-only; existing handler tests cover behavior)

Security / limits

  • No security or limits impact

Tool renaming

  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with UPDATE_TOOLSNAPS=true go test ./pkg/github/ -run 'Test_ManageNotificationSubscription|Test_ManageRepositoryNotificationSubscription' -count=1

Docs

  • Not needed

Under MCP schema 2025-06-18, an omitted destructiveHint defaults to true.
manage_notification_subscription and manage_repository_notification_subscription
only update the caller's own subscription preferences, so mark them explicitly
non-destructive to avoid over-warning consent UIs.

Fixes github#2841
@syf2211 syf2211 requested a review from a team as a code owner July 9, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Two notification-subscription tools omit destructiveHint, defaulting to destructive under MCP 2025-06-18

1 participant