feat(notifications): event detection (changes-requested)

[kiannidev]

Summary

• Adds GitHubReviewPayload and an optional review field on GitHubWebhookPayload in src/types.ts so pull_request_review webhooks are typed end-to-end.
• Introduces detectNotificationEvents in src/notifications/events.ts to emit a single pull_request_changes_requested event for the PR author, with self-notification and bot suppression.
• Records detected events from processGitHubWebhook via audit metadata (notification.event_detected) for downstream notify-evaluate/deliver jobs.
• Adds unit coverage in test/unit/notifications-events.test.ts and a queue processor test in test/unit/queue.test.ts for detection, suppression rules, fallback metadata, and one audit record per changes-requested review.

Closes #566

Scope

• This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
• This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
• I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally; global coverage stays at or above 97% for lines, statements, functions, and branches (aim for 98%+ branch coverage locally so CI variance does not fail near the threshold)
• npm run test:workers
• npm run build:mcp
• npm run test:mcp-pack
• npm run ui:openapi:check
• npm run ui:lint
• npm run ui:typecheck
• npm run ui:build
• npm run audit --audit-level=moderate
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

• npm run validate was run locally (covers typecheck + test:coverage); branch coverage met the 97% threshold.
• Remaining CI checks (actionlint, test:workers, build:mcp, test:mcp-pack, ui:*, audit) are left for CI to run.

Safety

• No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
• Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
• Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (not applicable — webhook event detection only; no auth/session changes)
• API/OpenAPI/MCP behavior is updated and tested where needed. (no public API/OpenAPI/MCP surface change; audit metadata is internal)
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (not applicable — backend only)
• Visible UI changes include screenshots or a short recording. (not applicable — no UI changes)
• Public docs/changelogs are updated where needed. (not applicable)

Notes

• Parent epic: feat(notifications): event-to-subscription-to-delivery service #535
• Includes the typed review payload prerequisite needed for changes-requested detection (see feat(types): add review field to GitHubWebhookPayload #536 / roadmap(pillar 2): surfaces — browser-extension rebuild & notifications #527).
• Detection records audit metadata only; delivery channels (notify-evaluate / notify-deliver) are tracked in feat(notifications): queue jobs (notify-evaluate/deliver) #568 and follow-on notification issues.

[ghost]

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal	Result	Evidence	Action
Gate result	⚠️ Skipped	#609 is no longer open.	No action.

---

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[ghost]

reviewbot · advisory review

Reviewed 5 changed file(s) — two independent AI reviewers.

Suggested action: ✅ Safe to merge — the reviewer found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR introduces detection of "changes_requested" review events, adds type definitions, records audit events, and includes comprehensive tests. The implementation is clean and integrates smoothly with existing processing flow.

Suggestions

• Consider adding deduplication handling for the new notification events to avoid duplicate audit records.
• Verify that the nowIso utility is exported from ../utils/json and returns the expected ISO timestamp format.
• Update any audit event schema documentation to include the new notification.event_detected event type.

Worth double-checking

• Potential duplicate audit events if the same webhook is processed multiple times.
• Ensure the new GitHubReviewPayload type does not conflict with existing type imports elsewhere.

Reviewer B · nemotron-3-120b-a12b — review unavailable

[ghost]

gittensory · advisory review

Reviewed 5 changed file(s) — two independent AI reviewers.

Suggested action: ✅ Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR introduces a new notification detection for pull request changes‑requested events, adds the necessary type definitions, records audit events, and includes comprehensive unit tests. The implementation is clean, well‑tested, and integrates smoothly with existing processing logic.

Suggestions

• Consider exporting NotificationEventType for potential reuse in other modules.
• Add a comment documenting the dedupKey format for future maintainers.
• Verify that nowIso is correctly exported from utils/json and has appropriate timezone handling.

Worth double-checking

• Ensure that adding the optional review field to GitHubWebhookPayload does not conflict with any existing strict type checks elsewhere in the codebase.
• Confirm that the audit_event table schema can store the new metadata fields without truncation.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR adds a new feature to detect GitHub pull request review events with state 'changes_requested' and record corresponding audit notifications. Changes include: a new detection function in src/notifications/events.ts, integration into the webhook processor in src/queue/processors.ts, type definitions in src/types.ts, and comprehensive unit tests. The implementation is well-typed, follows existing patterns, includes proper edge-case handling (bot users, self-reviews, missing data), and has strong test coverage.

Suggestions

• Consider adding a JSDoc comment to detectNotificationEvents explaining the return type (array) and dedupKey strategy for future maintainers.
• In the audit event metadata, ensure the 'deeplink' field is included (it currently is via spread, but verify it's present in the object being spread).
• The test file uses trailing commas in objects (e.g., { ...basePayload, } ) which is fine but inconsistent with the codebase style if it avoids them; however, this is minor.
• For the dedupKey, using the timestamp (submittedAt or detectedAt) ensures uniqueness per event occurrence, but note that if a review is edited within the same second, it might generate a new dedupKey (which may be intended).
• The PR title includes '(changes-requested)' which is unusual; consider simplifying to 'feat(notifications): add changes-requested review event detection' for clarity.

Worth double-checking

• The dedupKey relies on timestamps; if system clocks are skewed or the GitHub deliveredAt time is inconsistent, deduplication might behave unexpectedly (low risk as timestamps are from the webhook payload).
• The audit event recording loops over detected events; while the detection function currently returns 0-1 events, if this changes in the future, it could lead to multiple audit records per webhook (acceptable if intended).
• Ensure the new audit event type 'notification.event_detected' is permitted in any event filtering or retention policies (if applicable).
• Verify that the HTML URL fallback (to PR URL) works correctly when review.html_url is missing but pullRequest.html_url is present (covered in tests).