Skip to content

fix(notifications): route Discord actions through audited webhooks#2881

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-notification-routing
Jul 4, 2026
Merged

fix(notifications): route Discord actions through audited webhooks#2881
JSONbored merged 1 commit into
mainfrom
codex/fix-notification-routing

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • route Discord terminal-action notifications through one audited path
  • prefer DISCORD_REPO_WEBHOOKS for exact per-repo channels and fail closed for mapped repos with missing or invalid repo webhooks
  • stop emitting duplicate Discord messages from the PR outcome webhook path
  • add self-host runtime and persistence paths to the built-in manual-review guardrails
  • keep self-host env reference generation current after the notification module move

What changed

  • Removed the legacy PR-outcome Discord sender so merge/close notifications only come from the action executor.
  • Added audited Discord/Slack notification outcomes for sent, suppressed, and failed delivery attempts.
  • Added src/selfhost/**, database, migration, container, Compose, and systemd surfaces to hard guardrails.
  • Updated env-reference generation to scan the active notification module and helper-based env reads.

Why

Duplicate notification producers caused repeated Discord embeds for the same PR. The legacy fallback path could also post first-party repo notifications to the global webhook when repo-specific secrets were unset. Self-host runtime paths should be held for manual review because they control the review stack itself.

Validation

  • npx vitest run test/unit/notify-discord.test.ts test/unit/outcomes-wire.test.ts test/unit/guardrail-config.test.ts test/unit/change-guardrail.test.ts test/unit/agent-actions.test.ts test/unit/selfhost-env-reference-script.test.ts
  • npm run typecheck
  • npm run selfhost:env-reference:check
  • git diff --check
  • Targeted coverage over changed src/** files: 100% changed-line coverage and 100% changed-branch coverage

@JSONbored JSONbored self-assigned this Jul 4, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 4, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 8629c29 Commit Preview URL

Branch Preview URL
Jul 04 2026, 04:50 AM

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.05%. Comparing base (647808d) to head (8629c29).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2881      +/-   ##
==========================================
+ Coverage   96.03%   96.05%   +0.02%     
==========================================
  Files         259      258       -1     
  Lines       28412    28415       +3     
  Branches    10339    10336       -3     
==========================================
+ Hits        27285    27294       +9     
+ Misses        491      489       -2     
+ Partials      636      632       -4     
Files with missing lines Coverage Δ
src/review/guardrail-config.ts 100.00% <100.00%> (ø)
src/review/outcomes-wire.ts 88.80% <ø> (-0.33%) ⬇️
src/services/notify-discord.ts 100.00% <100.00%> (+10.52%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 4, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-04 04:28:27 UTC

11 files · 1 AI reviewer · no blockers · readiness 93/100 · CI failing · dirty

🛑 Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change consolidates terminal-action Discord delivery into src/services/notify-discord.ts, adds audited delivery outcomes, removes the duplicate pull_request.closed notification path, and expands hard guardrails for self-host runtime surfaces. The routing behavior is materially safer: repo-specific Discord mappings now fail closed instead of falling back to the global channel, and webhook HTTP failures are surfaced as audit errors while still preserving best-effort notification semantics. The generated self-host env reference and generator were updated coherently for the module move.

Nits — 6 non-blocking
  • nit: src/services/notify-discord.ts:105 now audits every disabled Discord/Slack notification, including the common unconfigured case, so consider whether missing webhook noise should be sampled or kept out of the operator-facing audit feed.
  • nit: src/services/notify-discord.ts:171 still reads SLACK_WEBHOOK_URL directly from Env while Discord moved to envString/process.env fallback; align Slack with envString if self-host Node parity is intended for both providers.
  • nit: scripts/gen-selfhost-env-reference.mjs:74 hard-codes only a helper named envString; add a short comment or a test case around the intended narrowness so future helper renames do not silently stale the env reference.
  • src/services/notify-discord.ts:171: use envString(env, "SLACK_WEBHOOK_URL") for Slack too, or document why Slack intentionally does not use the self-host process.env fallback.
  • src/services/notify-discord.ts:105: consider recording denied audit events only when a webhook-like setting exists but is invalid, while treating fully missing notification config as a quiet no-op.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.

CI checks failing

  • codecov/patch — 97.77% of diff hit (target 99.00%)
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 60 registered-repo PR(s), 51 merged, 436 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 60 PR(s), 436 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 60 PR(s), 436 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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

@gittensory-orb gittensory-orb Bot added the gittensor Gittensor contributor context label Jul 4, 2026
@JSONbored JSONbored force-pushed the codex/fix-notification-routing branch from 0e73c7a to e49f8d8 Compare July 4, 2026 04:01
@JSONbored JSONbored force-pushed the codex/fix-notification-routing branch from e49f8d8 to e6eb4ec Compare July 4, 2026 04:07
@JSONbored JSONbored force-pushed the codex/fix-notification-routing branch from e6eb4ec to c0e4922 Compare July 4, 2026 04:22
@JSONbored JSONbored force-pushed the codex/fix-notification-routing branch from c0e4922 to 8629c29 Compare July 4, 2026 04:48
@JSONbored JSONbored merged commit b2b6e72 into main Jul 4, 2026
14 checks passed
@JSONbored JSONbored deleted the codex/fix-notification-routing branch July 4, 2026 04:55
JSONbored added a commit that referenced this pull request Jul 4, 2026
…ument gate-check-publish audit decision (#2933)

Two doc-only clarifications from a self-host observability audit:

- src/selfhost/audit.ts's logAudit is a stdout-only logger for 4 queue-
  lifecycle events, not the durable audit_events DB writer -- a naming trap
  a future reader (including this repo's own prior audit) can easily fall
  into. Cross-references recordAuditEvent in db/repositories.ts.
- The successful gate-check-run publish path writes to check_summaries but
  not audit_events. Documents this as an intentional design decision
  (check_summaries is the purpose-built canonical record for this
  high-frequency event; downstream merge/close/hold actions are already
  audited separately) rather than a gap needing a fix.

No behavior change. The two other items originally flagged in this area are
already resolved elsewhere: notification send/suppress/fail audit coverage
was added by #2881 (notify-discord.ts's auditExternalNotification), and the
observability exports flagged as possibly-dead (registerMetricMeta,
DEFAULT_BUCKETS, resetMetrics, flushOpenTelemetry) turned out to all be
legitimately in use (extensively by tests, or as internal defaults) or a
standalone utility with a distinct purpose from shutdown -- none warrant
removal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. gittensor Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant