Add "Mark notification as done" functionality

[rfearing]

Add MarkNotificationAsDone to notifications tooling

Note: Target branch is notifications-tooling #225

Feature added

• `mark_notification_done: Mark a specific notification thread as done

Technical implementation

• I largely copied mark_notification_read
• For the param threadId, WithNumber ran into some issues, so I kept the param as a string. I'm open to suggestions.

[Copilot]

Pull Request Overview

This PR adds the "Mark Notification as Done" functionality to the notifications tooling. The changes include:

• Adding a new tool registration in pkg/github/server.go.
• Implementing the MarkNotificationDone function in pkg/github/notifications.go to mark a specific notification thread as done.

Reviewed Changes

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

File	Description
pkg/github/server.go	Registers the new tool for marking a notification as done.
pkg/github/notifications.go	Implements MarkNotificationDone with parameter parsing and API call.

Comments suppressed due to low confidence (1)

pkg/github/notifications.go:260

• [nitpick] Consider renaming the parameter 'getclient' to 'getClient' for consistency with similar functions and to improve readability.

func MarkNotificationDone(getclient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {

[juruen]

@rfearing as having too many tools might become an issue, maybe it'd be worth exploring the possibility of merging some of these tools into one.

It seems that marking notifications might be a good candidate to having just one and using the parameters to decide what to do exactly.

And the same goes for the getters.

In general, we should probably explore providing higher level APIs instead of having a 1:1 mapping with the underlying API.

We are already doing that in some tools with a positive result.

In any case, these are just my two cents.

/cc @toby @SamMorrowDrums @williammartin

[sridharavinash]

Nice work! 💖

[SamMorrowDrums]

Surprised this was merged without addressing @juruen 's comment.

Probably it makes more sense for the two mark notification tools to be one with an enum for what state to change it to.

Can consider as a follow-up though.

[SamMorrowDrums]

Oh, sorry I didn't realise this was merged into a branch!

[rfearing]

@SamMorrowDrums, @juruen I targeted @sridharavinash's branch and that PR has not yet been merged

@sridharavinash I'm happy to invest some time into looking into @juruen's comment but in your branch.

[SamMorrowDrums]

@rfearing thank you, really appreciate this too - I was just worried it landed into main without a final pass. There's a bit of a rebase involved to get it into main too :-D

Bu also happy to take a look at the next branch, I just missed it wasn't a merge to main!