Skip to content

feat(review): deliver the maintainer recap digest to Slack (#2246) - #4217

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
andriypolanski:feat/recap-slack-delivery-2246
Jul 8, 2026
Merged

feat(review): deliver the maintainer recap digest to Slack (#2246)#4217
JSONbored merged 1 commit into
JSONbored:mainfrom
andriypolanski:feat/recap-slack-delivery-2246

Conversation

@andriypolanski

Copy link
Copy Markdown
Contributor

Summary

  • Add deliverRecapToSlack(env, recap) in src/services/review-recap.ts — the Slack sibling of the
    existing sendReviewRecapToDiscord. Posts the review recap as a Block Kit mrkdwn section to
    SLACK_WEBHOOK_URL, reusing isValidSlackWebhook + escapeSlackMrkdwnText from notify-discord.ts
    (now exported) instead of re-implementing webhook validation/escaping a second time.
  • Best-effort, never-throws contract, matching sendReviewRecapToDiscord: denies with missing_webhook /
    invalid_webhook when unset/invalid, records completed / denied / error audit events under
    review_recap_notification.slack, and degrades to a recorded error result on a thrown fetch or a
    non-2xx webhook response instead of throwing.
  • Adds 6 unit tests to test/unit/review-recap.test.ts covering every branch: configured+sent, mrkdwn
    escaping (&, <, >), missing webhook, invalid webhook, thrown fetch, and non-2xx response.

This is the Slack-channel prerequisite named in #2252 ("lands after the builder/formatter/delivery
bounties") — that end-to-end fan-out test cannot be written until both channels' delivery functions
exist. No fan-out orchestrator is added here; wiring both channels together is out of scope for this PR.

Closes #2246

Scope

Changed files

File Change
src/services/notify-discord.ts Export isValidSlackWebhook and escapeSlackMrkdwnText
src/services/review-recap.ts Add deliverRecapToSlack
test/unit/review-recap.test.ts deliverRecapToSlack branch coverage tests

Codecov

Target codecov/patch 99% (branch-counted, src/** only). Verified locally with a scoped coverage
run on the two changed src/** files:

COVERAGE_NO_THRESHOLDS=1 npx vitest run test/unit/review-recap.test.ts test/unit/notify-discord.test.ts \
  --coverage --coverage.include="src/services/review-recap.ts" --coverage.include="src/services/notify-discord.ts"
# Statements 100% · Branches 100% · Functions 100% · Lines 100%

Validation

git diff --check
npx vitest run test/unit/review-recap.test.ts test/unit/notify-discord.test.ts
npm run typecheck
npm run selfhost:env-reference:check
npm run test:ci
npm audit --audit-level=moderate

@andriypolanski
andriypolanski requested a review from JSONbored as a code owner July 8, 2026 14:29
@andriypolanski
andriypolanski marked this pull request as draft July 8, 2026 14:30
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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 17:23:38 UTC

3 files · 1 AI reviewer · no blockers · readiness 100/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/**), src/services/review-recap.ts (matched src/services/**).

Review summary
This PR adds `deliverRecapToSlack` in src/services/review-recap.ts as a straightforward sibling of the existing `sendReviewRecapToDiscord`, reusing `isValidSlackWebhook`/`escapeSlackMrkdwnText` (now exported from notify-discord.ts) instead of re-implementing webhook validation/escaping. The control flow (missing/invalid webhook → denied audit event, thrown fetch/non-2xx → error audit event, success → completed audit event) exactly mirrors the existing Discord and notifyActionToSlack patterns, and the 6 added unit tests cover every branch (configured+sent, mrkdwn escaping of both repo name and summary, missing webhook, invalid webhook, thrown fetch, non-2xx response) against the real code paths, not fabricated states. The PR correctly closes #2246 and stays scoped to the standalone delivery function, deferring the fan-out orchestrator to the linked follow-up #2252.

Nits — 6 non-blocking
  • src/services/review-recap.ts reads `SLACK_WEBHOOK_URL` via a raw `(env as unknown as Record<string, unknown>)` cast rather than the `envString` helper already defined in notify-discord.ts — matches `notifyActionToSlack`'s existing pattern though, so this is pre-existing duplication rather than something this PR introduces.
  • The Slack POST/audit-event boilerplate in `deliverRecapToSlack` is nearly identical to `sendReviewRecapToDiscord`'s (and to `notifyActionToSlack`'s) — worth a shared helper eventually, but consistent with the existing Discord/Slack duplication precedent in this file, so not this PR's job to fix.
  • The file-header comment block in review-recap.ts is getting long (three stacked scope explanations); consider trimming once test(notifications): end-to-end recap orchestration test (build → format → both channels) #2252's fan-out lands so the header doesn't keep accreting.
  • When the fan-out orchestrator (test(notifications): end-to-end recap orchestration test (build → format → both channels) #2252) lands, consider extracting the shared 'validate webhook → build payload → fetch → record audit event' skeleton so Discord/Slack recap delivery (and the per-event notifiers in notify-discord.ts) share one implementation.
  • Consider a follow-up test asserting `deliverRecapToSlack` and `sendReviewRecapToDiscord` can be called independently without cross-channel audit-event leakage, ahead of the test(notifications): end-to-end recap orchestration test (build → format → both channels) #2252 fan-out wiring.
  • 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 #2246
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low 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: 147 registered-repo PR(s), 84 merged, 22 issue(s).
Contributor context ✅ Confirmed Gittensor contributor andriypolanski; Gittensor profile; 147 PR(s), 22 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Addressed
The PR adds deliverRecapToSlack in review-recap.ts that posts a Block Kit mrkdwn section to SLACK_WEBHOOK_URL, reuses isValidSlackWebhook and escapeSlackMrkdwnText (now exported from notify-discord.ts) instead of reimplementing them, no-ops with recorded denial on missing/invalid webhook, and swallows fetch/non-2xx errors into a recorded result rather than throwing. It also adds unit tests coverin

Review context
  • Author: andriypolanski
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript, Python, Rust, Cuda, Kotlin, MDX, Scala
  • Official Gittensor activity: 147 PR(s), 22 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.76%. Comparing base (24d058a) to head (baa8382).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4217   +/-   ##
=======================================
  Coverage   93.75%   93.76%           
=======================================
  Files         389      389           
  Lines       36499    36515   +16     
  Branches    13376    13379    +3     
=======================================
+ Hits        34221    34237   +16     
  Misses       1621     1621           
  Partials      657      657           
Files with missing lines Coverage Δ
src/services/notify-discord.ts 100.00% <ø> (ø)
src/services/review-recap.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.

@JSONbored
JSONbored merged commit 9651b55 into JSONbored:main Jul 8, 2026
8 checks passed
@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. and removed gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jul 8, 2026
@JSONbored JSONbored added gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. and removed gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 9, 2026
@andriypolanski
andriypolanski deleted the feat/recap-slack-delivery-2246 branch July 16, 2026 15:13
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): Slack delivery for the maintainer recap digest (reuse notify-discord Slack path)

2 participants