Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026

Summary

The dismiss_notification tool incorrectly rejected HTTP 204 (No Content) responses from the GitHub API as failures, despite 204 being the documented success status for marking notifications as done.

Why

When users marked notifications as done, the operation succeeded but reported: failed to mark notification as done: unexpected status 204:

What changed

  • Added http.StatusNoContent (204) to accepted success status codes in DismissNotification handler
  • Added test coverage for 204 response on "mark as done" operation

MCP impact

  • No tool or API changes — Internal bug fix only; tool behavior now matches GitHub API contract.

Prompts tested (tool changes only)

N/A — No user-facing tool changes

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 ./script/test

Docs

  • Not needed
Original prompt

This section details on the original issue you should resolve

<issue_title>dismiss_notification reports failure on successful 204 response</issue_title>
<issue_description>When using the dismiss_notification tool to mark notifications as done, the operation succeeds (HTTP 204 No Content) but the tool reports it as a failure:

failed to mark notification as done: unexpected status 204:

HTTP 204 is the expected success response for this endpoint per the GitHub API documentation.

Expected behavior: The tool should return success when receiving a 204 response.

Actual behavior: The tool returns an error message indicating failure despite the operation completing successfully.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Add http.StatusNoContent (204) to the list of accepted success status codes
- Add test case for 204 response when marking notification as done
- Retain existing test for 200 response for backwards compatibility

Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix dismiss_notification tool reporting failure on 204 response Fix dismiss_notification rejecting valid HTTP 204 response Jan 16, 2026
Copilot AI requested a review from SamMorrowDrums January 16, 2026 10:03
@SamMorrowDrums SamMorrowDrums marked this pull request as ready for review January 16, 2026 10:13
@SamMorrowDrums SamMorrowDrums requested a review from a team as a code owner January 16, 2026 10:13
Copilot AI review requested due to automatic review settings January 16, 2026 10:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug in the dismiss_notification tool where HTTP 204 (No Content) responses from GitHub's API were incorrectly treated as failures. The GitHub API returns 204 as the documented success status when marking notifications as done, so this change aligns the code with the actual API contract.

Changes:

  • Added http.StatusNoContent (204) to the accepted success status codes in the DismissNotification handler
  • Added test case for 204 response when marking notifications as done
  • Renamed existing test case to clarify it tests 200 response scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/github/notifications.go Added HTTP 204 status code to success condition check for dismiss notification operation
pkg/github/notifications_test.go Added test case for 204 response and clarified existing test naming to distinguish 200 vs 204 scenarios

@SamMorrowDrums SamMorrowDrums merged commit ccf5191 into main Jan 16, 2026
22 checks passed
@SamMorrowDrums SamMorrowDrums deleted the copilot/fix-dismiss-notification-error branch January 16, 2026 10:16
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.

dismiss_notification reports failure on successful 204 response

3 participants