Fix high-severity SonarQube issues in notifications.go (S1192, S3776)

[eashansinha]

Summary

Resolves the 8 high-severity SonarQube findings in pkg/github/notifications.go (6× S1192 duplicated string literals, 2× S3776 cognitive complexity) without any behavioral change. All existing tests, go vet, gofmt, and golangci-lint (v2.1) pass.

S1192 — package-level constants for repeated literals

Each of these literals appeared 4–6 times in the file; SonarQube raises one issue per duplicated literal. Extracted to package-level consts:

const ( ownerParam = "owner"; actionParam = "action"; notificationIDParam = "notificationID" )
const (
    errFailedToGetClient       = "failed to get GitHub client: %w"
    errFailedToReadBody        = "failed to read response body: %w"
    errFailedToMarshalResponse = "failed to marshal response: %w"
)

The param-name constants replace the literals in mcp.WithString(...) / RequiredParam / OptionalParam call sites; the error formats replace the fmt.Errorf literals.

S3776 — reduce cognitive complexity

Only ListNotifications (~20) and MarkAllNotificationsRead (~16) exceeded the threshold of 15. Extracted four helpers so each handler body now sits at ~11:

• buildNotificationListOptions(filter, since, before, pagination) — moves option assembly + RFC3339 since/before parsing out of the handler.
• listNotificationsByScope(ctx, client, owner, repo, opts) — collapses the repo-vs-all branch.
• parseLastReadAt(lastReadAt) — defaults to time.Now() when empty, else parses RFC3339.
• notificationResponseError(resp, message) — reads the body and builds the error result, removing the repeated nested if err := io.ReadAll(...) block from the four handlers that used it.

Net effect on ListNotifications handler:

opts, err := buildNotificationListOptions(filter, since, before, paginationParams)
if err != nil { return mcp.NewToolResultError(err.Error()), nil }

notifications, resp, err := listNotificationsByScope(ctx, client, owner, repo, opts)
...
if resp.StatusCode != http.StatusOK {
    return notificationResponseError(resp, "failed to get notifications")
}

Error strings and tool-result messages are byte-for-byte identical to before (e.g. parseLastReadAt's error is still wrapped with the same invalid lastReadAt time format... text), so behavior and the existing test assertions are unchanged.

Notes / tradeoffs

• The other handlers (Dismiss, ManageNotificationSubscription, ManageRepositoryNotificationSubscription, GetNotificationDetails) were already under the complexity threshold; only the constant swaps and the shared notificationResponseError helper touch them, keeping the diff focused.
• gocognit reports higher numbers because it folds handler closures into the enclosing function with a nesting penalty; SonarQube analyzes each closure as its own function (nesting 0), which is why only the two handlers above were flagged.

Verification

• go build ./...
• go test ./pkg/github/ — pass
• gofmt -l . (clean) + go vet ./...
• golangci-lint run ./pkg/github/ (v2.1.6) — 0 issues

Link to Devin session: https://app.devin.ai/sessions/a3f594efd7014724a96b1236c9d54964
Requested by: @eashansinha

---

Devin Review

Status	Commit
🟢 Reviewed	d982358

[devin-ai-integration]

Original prompt from Eashan

Fix the following high severity SonarQube issues in @COG-GTM/github-mcp-server :
Repository: COG-GTM/github-mcp-server File: pkg/github/notifications.go Issues (8 total):

Fix these issues by:
For S1192 (duplicated strings): Define constants at the package level for repeated string literals
For S3776 (cognitive complexity): Refactor complex methods by extracting helper functions
Run tests after fixes to ensure nothing breaks.

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[staging-devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Debug

Playground

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.