Skip to content

feat(notifications): per-repo Slack webhook routing to match Discord/PagerDuty#8471

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
xfodev:feat/per-repo-slack-webhook-8371
Jul 24, 2026
Merged

feat(notifications): per-repo Slack webhook routing to match Discord/PagerDuty#8471
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
xfodev:feat/per-repo-slack-webhook-8371

Conversation

@xfodev

@xfodev xfodev commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Slack senders read a single global SLACK_WEBHOOK_URL, so a self-host operator running LoopOver across more than one repo gets every repo's terminal-action and review-recap messages mixed into one channel — unlike Discord (resolveDiscordWebhook) and PagerDuty (resolvePagerDutyRoutingKey), which already resolve a per-repo channel/routing key first.

  • New resolveSlackWebhook(env, repoFullName) in src/services/notify-discord.ts, mirroring resolveDiscordWebhook's exact shape and precedence: a SLACK_REPO_WEBHOOKS per-repo JSON map entry (keys lower-cased) wins, else the global SLACK_WEBHOOK_URL, else disabled. Returns the same discriminated union (configured/disabled with the same reason literals). Every URL still passes isValidSlackWebhook (https://hooks.slack.com/services/...). Two-source only — no legacy WEBHOOK_SECRET_BY_REPO-style tier (Slack has no historical migration to preserve). A repo WITH a per-repo entry never silently falls back to the global channel.
  • notifyActionToSlack (notify-discord.ts) now routes via resolveSlackWebhook and audits the denied reason + threads source into completed/error audits, exactly like notifyActionToDiscord.
  • deliverRecapToSlack (review-recap.ts, the per-repo ReviewRecap sender) now routes via resolveSlackWebhook(env, recap.repoFullName), mirroring its Discord sibling sendReviewRecapToDiscord.
  • Out of scope, untouched: the multi-repo maintainer-digest deliverRecapToSlack in notify-discord.ts (correctly global-only by design), notify-pagerduty.ts, and resolveDiscordWebhook.
  • Added a generic repoJsonMap(env, envName) in notify-discord.ts (same shape as notify-pagerduty.ts's) for the Slack map; the Discord repoWebhookMap/resolveDiscordWebhook paths are byte-identical.
  • Documented SLACK_REPO_WEBHOOKS in .env.example beside SLACK_WEBHOOK_URL/DISCORD_REPO_WEBHOOKS.

Tests

Extended test/unit/notify-discord.test.ts and test/unit/review-recap.test.ts (69 pass): resolveSlackWebhook repo-map hit (valid/invalid/non-string), global fallback (valid/invalid/unset), malformed SLACK_REPO_WEBHOOKS JSON → {}; notifyActionToSlack posts to the per-repo URL not the global, falls back when absent, denies an invalid per-repo entry (no silent global fallback); deliverRecapToSlack routes two different recap.repoFullName values independently (the core regression). Existing global-only Slack assertions updated (not deleted), global-fallback case kept covered.

Validation

  • tsc --noEmit clean (changed files); both test files green (69).
  • selfhost:env-reference:check clean.
  • Rebased on latest main; no base conflict.
  • Discord/PagerDuty paths untouched; isValidSlackWebhook not weakened; no secret/wallet/hotkey/trust/reward terms.

Closes #8371

…PagerDuty

Slack senders read a single global SLACK_WEBHOOK_URL, so a self-host operator
running LoopOver across more than one repo gets every repo's terminal-action
and review-recap messages mixed into one channel — unlike Discord and PagerDuty,
which already resolve a per-repo channel/routing key first.

Add resolveSlackWebhook(env, repoFullName) in notify-discord.ts, mirroring
resolveDiscordWebhook's precedence: a SLACK_REPO_WEBHOOKS per-repo JSON map
entry wins, else the global SLACK_WEBHOOK_URL, else disabled — every URL still
gated by isValidSlackWebhook, two-source only (no legacy secret tier). Rewire
the two per-repo senders (notifyActionToSlack, review-recap.ts's
deliverRecapToSlack) to route through it and thread the resolution source into
their audit events, exactly like their Discord siblings. The multi-repo
maintainer digest deliverRecapToSlack stays global-only by design. Document
SLACK_REPO_WEBHOOKS in .env.example.

Closes JSONbored#8371
@xfodev
xfodev requested a review from JSONbored as a code owner July 24, 2026 14:20
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.67%. Comparing base (6c25eb6) to head (3efbbcf).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8471      +/-   ##
==========================================
+ Coverage   79.69%   89.67%   +9.97%     
==========================================
  Files         791       99     -692     
  Lines       79319    22900   -56419     
  Branches    23954     3920   -20034     
==========================================
- Hits        63216    20535   -42681     
+ Misses      13298     2187   -11111     
+ Partials     2805      178    -2627     
Flag Coverage Δ
shard-1 67.52% <75.00%> (?)
shard-2 78.86% <85.71%> (+27.75%) ⬆️
shard-3 13.91% <21.42%> (-39.51%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/services/notify-discord.ts 100.00% <100.00%> (ø)
src/services/review-recap.ts 100.00% <100.00%> (ø)

... and 692 files with indirect coverage changes

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

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-24 15:41:41 UTC

5 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds resolveSlackWebhook mirroring the existing resolveDiscordWebhook precedence (per-repo SLACK_REPO_WEBHOOKS map wins over global SLACK_WEBHOOK_URL, fail-closed on an invalid per-repo entry), then wires it through notifyActionToSlack and the per-repo deliverRecapToSlack in review-recap.ts, leaving the multi-repo maintainer-digest deliverRecapToSlack in notify-discord.ts untouched as stated. The implementation is a faithful, minimal port of the already-proven Discord pattern (repoJsonMap generalizes repoWebhookMap), and the new tests directly exercise every branch (repo_map hit, invalid repo entry fails closed instead of falling back, non-string/blank entries, malformed JSON, global fallback, missing/invalid global) including cross-checking that resolveSlackWebhook's REGRESSION cases match resolveDiscordWebhook's proven fail-closed behavior.

Nits — 4 non-blocking
  • The maintainer-digest deliverRecapToSlack in notify-discord.ts still uses the old `invalid_webhook`/`missing_webhook` reason literals while the new per-repo resolveSlackWebhook (and its consumer in review-recap.ts) use `invalid_global_webhook`/`missing_global_webhook` — two Slack senders in the same file now report differently-shaped denial reasons for the same underlying condition, worth a follow-up note even if intentionally out of scope here.
  • `SlackWebhookResolution`'s `missing_repo_webhook` reason literal can never actually be produced by resolveSlackWebhook's own logic (the code comment already acknowledges this) — consider dropping it from the union rather than keeping a dead reason for shape-parity with Discord's three-tier resolution.
  • Consider a short follow-up to align the maintainer-digest Slack reason strings with the per-repo resolver's `_global_webhook` suffix convention for consistency across the file.
  • If `missing_repo_webhook` is genuinely unreachable for Slack, drop it from `SlackWebhookResolution` instead of carrying it purely for shape-parity with `DiscordWebhookResolution`.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8371
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: 64 registered-repo PR(s), 29 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor xfodev; Gittensor profile; 64 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The diff adds resolveSlackWebhook with the exact required precedence and discriminated union shape, updates both notifyActionToSlack and review-recap.ts's deliverRecapToSlack to use it with matching audit behavior, exports the function, leaves the multi-repo Discord/PagerDuty paths untouched, and documents SLACK_REPO_WEBHOOKS in .env.example with substantial test coverage.

Review context
  • Author: xfodev
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, TypeScript
  • Official Gittensor activity: 64 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 73fef0a into JSONbored:main Jul 24, 2026
12 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(notifications): per-repo Slack webhook routing to match Discord/PagerDuty (notify-discord.ts, review-recap.ts)

1 participant