Skip to content

feat(notifications): deliver the maintainer recap digest to Discord (#2245) - #4317

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
davion-knight:feat/gittensory-recap-discord-delivery
Jul 8, 2026
Merged

feat(notifications): deliver the maintainer recap digest to Discord (#2245)#4317
JSONbored merged 1 commit into
JSONbored:mainfrom
davion-knight:feat/gittensory-recap-discord-delivery

Conversation

@davion-knight

Copy link
Copy Markdown
Contributor

Adds deliverRecapToDiscord(env, report) — the Discord channel of the #1963 maintainer recap digest (#2245), sibling of the Slack delivery.

What it does

  • Posts a RecapReport to the operator's global DISCORD_WEBHOOK_URL as an embed (window totals: reviewed / merged / closed / gate false-positives / overrides / reversals + the summary lines).
  • A recap spans every repo, so — unlike notifyActionToDiscord's per-repo routing — it targets the single operator-level webhook, reusing the existing isValidDiscordWebhook validation + best-effort postWebhook send pattern already in notify-discord.ts.
  • Best-effort: an unset or invalid webhook is a silent no-op; a send failure is swallowed — a Discord outage never breaks the (best-effort) recap job.
  • The RecapReport is already public-safe (the builder sanitizes every free-text field), so no re-scrub is needed here.

Tests

test/unit/notify-discord.test.ts: sends the embed when configured (asserts the payload + public-safe: no economic/identity term leaks), no-ops when the webhook is unset, no-ops when it fails validation (non-https), and never throws on a send rejection — all four branches covered.

Closes #2245

@davion-knight
davion-knight requested a review from JSONbored as a code owner July 8, 2026 20:19
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@f498e70). Learn more about missing BASE report.
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4317   +/-   ##
=======================================
  Coverage        ?   93.81%           
=======================================
  Files           ?      398           
  Lines           ?    36784           
  Branches        ?    13439           
=======================================
  Hits            ?    34508           
  Misses          ?     1622           
  Partials        ?      654           
Files with missing lines Coverage Δ
src/services/notify-discord.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 8, 2026
@loopover-orb

loopover-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-08 21:29:46 UTC

2 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/services/notify-discord.ts (matched src/services/**).

Review summary
Adds deliverRecapToDiscord, a straightforward sibling of the existing notifyActionToDiscord that posts a RecapReport as a Discord embed to the single global DISCORD_WEBHOOK_URL, reusing the existing isValidDiscordWebhook validation and postWebhook/best-effort send pattern. The implementation is correct against the code shown — envString/isValidDiscordWebhook/postWebhook are all used exactly as their existing call sites use them, all fields referenced on RecapReport exist in the test fixture, and the four tests genuinely exercise both arms of the `!url || !isValidDiscordWebhook(url)` guard plus the success and swallowed-failure paths. The one open question is a design one, not a diff-local bug: the existing FocusManifestReviewRecapConfig doc (focus-manifest.ts) states Discord recap delivery should reuse the SAME per-repo resolveDiscordWebhook routing as notifyActionToDiscord, but this function only ever posts to the flat global DISCORD_WEBHOOK_URL — worth a contributor confirmation since I can't see the caller (review-recap.ts/buildMaintainerRecap) to verify per-repo enabled gating and channel routing happen upstream as intended.

Nits — 6 non-blocking
  • src/services/notify-discord.ts:154 — deliverRecapToDiscord never calls recordAuditEvent (via the existing auditExternalNotification helper) on send/deny/error, unlike notifyActionToDiscord right above it in the same file; that's an observability gap for an operator trying to confirm whether the weekly recap actually landed.
  • packages/gittensory-engine/src/focus-manifest.ts's FocusManifestReviewRecapConfig doc says Discord recap delivery 'reuses the SAME per-repo webhook resolution as the per-event notifier (resolveDiscordWebhook)', but this PR posts only to the single global DISCORD_WEBHOOK_URL — confirm this is an intentional scope decision (report.repos already filtered to opted-in repos upstream) and not a stale doc comment, since I can't see review-recap.ts to verify.
  • The magic numbers flagged (1800-char slice, 0x0969da color, 120-char error truncation) all mirror the existing conventions already used a few lines up in notifyActionToDiscord, so this is stylistic only, not a real gap.
  • If the per-repo resolveDiscordWebhook routing in the FocusManifestReviewRecapConfig doc is still the intended design, either update that doc comment to reflect the global-only decision or wire deliverRecapToDiscord to route per-repo like notifyActionToDiscord does.
  • Consider adding a recordAuditEvent call (success/denied/error) for the recap send, mirroring auditExternalNotification, so recap delivery failures are queryable the same way per-action notification failures are today.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2245
Related work ⚠️ 1 scoped overlap Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 89 registered-repo PR(s), 51 merged, 1 issue(s).
Contributor context ✅ Confirmed Gittensor contributor davion-knight; Gittensor profile; 89 PR(s), 1 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Addressed
The PR adds deliverRecapToDiscord(env, report) in notify-discord.ts that posts the RecapReport as a Discord embed via the existing webhook validation/best-effort pattern, no-ops with proper reasons when the webhook is unset or fails isValidDiscordWebhook, and swallows send errors without throwing, exactly as specified.

Review context
  • Author: davion-knight
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 89 PR(s), 1 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (issue #2240, issue #2244)
Contributor next steps
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

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

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 8, 2026
…SONbored#2245)

Add deliverRecapToDiscord(env, report): post a multi-repo maintainer RecapReport
to the operator's global DISCORD_WEBHOOK_URL as an embed, reusing notify-discord.ts's
isValidDiscordWebhook validation + best-effort postWebhook send pattern.

A maintainer recap is ONE operator-level digest spanning many repos (report.repos),
so — unlike the per-repo ReviewRecap sender sendReviewRecapToDiscord / notifyActionToDiscord,
which route per-repo via resolveDiscordWebhook — there is no single repo to route by and it
posts to the flat global webhook. Best-effort and observable, mirroring sendReviewRecapToDiscord:
an unset/invalid webhook or a send failure is recorded to the audit ledger
(maintainer_recap_notification.discord) and returned as { sent, reason } but never thrown, so a
Discord outage never breaks the recap job. The RecapReport is already public-safe (the builder
sanitizes every free-text field).

Closes JSONbored#2245
@JSONbored
JSONbored merged commit 46a19f8 into JSONbored:main Jul 8, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

feat(notifications): Discord delivery for the maintainer recap digest (reuse discord-notify)

2 participants