feat: extract notification delivery into go_notify_yourself module - #1253
Open
Wikid82 wants to merge 16 commits into
Open
feat: extract notification delivery into go_notify_yourself module#1253Wikid82 wants to merge 16 commits into
Wikid82 wants to merge 16 commits into
Conversation
Scopes the extraction of Charon's notification delivery (HTTP wrapper, provider payload builders, email dispatch) into the standalone go_notify_yourself module, including the decoupling design, new module's public API, and the two-repo commit slicing strategy.
Discord dispatch now routes through buildNotifySender/transport.Wrapper (the extracted go_notify_yourself module) instead of the legacy sendJSONPayload path. This is a deliberate behavior change, not just a refactor: today's Discord dispatch bypasses the old HTTPWrapper entirely (direct network/security calls, no retry/backoff). Discord notifications now retry on transient failures, consistent with every other provider. SendExternal and TestProvider route Discord through a shared dispatchViaNotify/testProviderViaNotify seam, gated by a notifyMigratedProviderTypes allowlist that will grow one provider at a time as the rest of the migration lands. The notify.Message sent for Discord carries HostName/HostIP/ServiceCount/Services under Data so a provider configured with the old "detailed" template keeps rendering via legacyDetailedTemplate's backward-compat shape. Tests exercising Discord dispatch now inject a capturing fake RoundTripper via a new WithNotifyTransportWrapper test option, since the extracted module's own Discord webhook validation only accepts discord.com/canary.discord.com hosts and can no longer be pointed at an httptest.Server the way pre-cutover tests were. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
Slack dispatch now routes through buildNotifySender/transport.Wrapper via the shared dispatchViaNotify/testProviderViaNotify seam, joining Discord in notifyMigratedProviderTypes. Field mapping matches the old sendJSONPayload branch exactly: the Slack webhook URL comes from provider.Token (provider.URL remains an unused placeholder), so no provider-facing behavior changes. Tests exercising Slack dispatch now inject a capturing fake RoundTripper via WithNotifyTransportWrapper and use a real hooks.slack.com-shaped webhook URL, since the extracted module's own Slack webhook validation enforces the same URL shape Charon's service-level validator did and can no longer be pointed at an httptest.Server via a validator override. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
Gotify dispatch now routes through buildNotifySender/transport.Wrapper via the shared dispatchViaNotify/testProviderViaNotify seam, joining Discord and Slack in notifyMigratedProviderTypes. Field mapping is unchanged from the old sendJSONPayload branch: URL from provider.URL, token sent as X-Gotify-Key when non-empty. Two TestProvider tests that dispatch to a local httptest.Server now set CHARON_ENV=test explicitly, since the extracted module's transport wrapper gates plain-HTTP/localhost dispatch on that env var rather than the old implicit os.Args[0]-based test-binary detection. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
Pushover dispatch now routes through buildNotifySender/transport.Wrapper via the shared dispatchViaNotify/testProviderViaNotify seam, joining Discord, Slack, and Gotify in notifyMigratedProviderTypes. Field mapping matches the old sendJSONPayload branch: user key from provider.URL, API token from provider.Token, injected into the payload's token/user fields server-side after template rendering (same anti-injection behavior as before). The extracted module's Config leaves BaseURL empty, so dispatch targets Pushover's real production API exactly as before (the old svc.pushoverAPIBaseURL test-only override doesn't apply to this path); new tests use a capturing fake RoundTripper to verify the notify-path dispatch without a real network call. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
Ntfy dispatch now routes through buildNotifySender/transport.Wrapper via the shared dispatchViaNotify/testProviderViaNotify seam, joining Discord, Slack, Gotify, and Pushover in notifyMigratedProviderTypes. Field mapping is unchanged from the old sendJSONPayload branch: URL from provider.URL, token sent as an "Authorization: Bearer <token>" header when non-empty. New TestProvider/SendExternal tests dispatch to a local httptest.Server with CHARON_ENV=test set explicitly, since the extracted module's transport wrapper gates plain-HTTP dispatch on that env var rather than the old implicit os.Args[0]-based test-binary detection. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
Telegram dispatch now routes through buildNotifySender/transport.Wrapper via the shared dispatchViaNotify/testProviderViaNotify seam, joining Discord, Slack, Gotify, Pushover, and Ntfy in notifyMigratedProviderTypes. Field mapping matches the old sendJSONPayload branch: bot token from provider.Token (embedded in the dispatch URL path, not a header), chat ID from provider.URL injected into the payload's chat_id field after template rendering. The extracted module's Config leaves BaseURL empty, so dispatch targets the real Telegram Bot API exactly as before (the old svc.telegramAPIBaseURL test-only override doesn't apply to this path); new tests use a capturing fake RoundTripper to verify the notify-path dispatch without a real network call. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
Webhook and generic-webhook dispatch now route through
buildNotifySender/transport.Wrapper via the shared
dispatchViaNotify/testProviderViaNotify seam, joining every other
provider type except email in notifyMigratedProviderTypes.
CreateProvider and UpdateProvider's custom-template preview validation
now calls providers/webhook.RenderPreview instead of the old
RenderTemplate, so the preview payload matches the same
Title/Message/Time/EventType/Data shape actual dispatch uses — a custom
template referencing {{index .Data "..."}} now validates correctly at
save time instead of failing against a flat map that had no Data field.
RenderTemplate itself is untouched and still backs the provider/template
preview API handlers, which are out of scope for this cutover.
Three TestProvider tests that dispatch to a local httptest.Server now set
CHARON_ENV=test explicitly, matching the earlier Gotify/Ntfy commits'
rationale: the extracted module's transport wrapper gates plain-HTTP
dispatch on that env var rather than the old implicit os.Args[0]-based
test-binary detection.
Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
SendExternal's email branch and TestEmailProvider now dispatch through
providers/email (the extracted notify module's email package) via
NewNotifyEmailConfig (notify_email_adapter.go) instead of calling the
mail service directly. dispatchEmail itself is left in place, unused by
production code after this commit — cleanup is a separate, later commit.
TestEmailProvider builds its own inline email.Config (reusing the same
Renderer/Mailer adapters) because its test-send subject prefix
("[Charon Test] ") and forced "email_system_event.html" template differ
from NewNotifyEmailConfig's production values.
This is a deliberate, documented behavior change: dispatchEmail's old
fallback — building a manual plain HTML body and still sending when
template rendering fails — does not exist in the extracted module.
email.Client.Send aborts before calling Mailer.Send when the configured
Renderer returns an error, so a broken/missing email template now fails
the notification (or test send) closed instead of degrading gracefully
to a generic fallback body. Tests that relied on the old fallback path
were rewritten to assert the new fail-closed behavior.
Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
The extracted notify module's email.Client.Send aborts before calling Mailer.Send whenever the configured TemplateRenderer returns an error, so the email cutover to providers/email (commit c073f4b) silently started failing dispatch closed on any template-render failure, instead of degrading gracefully like pre-extraction dispatchEmail did. Fix lives entirely in mailServiceTemplateRendererAdapter.Render (notify_email_adapter.go): on a RenderNotificationEmail failure it now logs a warning and returns a locally-composed plain-HTML fallback body (fallbackEmailBody) instead of propagating the error, restoring the old fail-open-with-degraded-body behavior for both dispatchEmailViaNotify and TestEmailProvider without touching the extracted module or any other already-migrated provider. Real Mailer/SMTP transport failures are unaffected and still propagate as errors. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
The notification delivery engine (HTTP dispatch wrapper, per-provider payload/validation/template logic, and unused engine/router scaffolding) now lives in the external github.com/Wikid82/go_notify_yourself module, consumed via the Charon-side adapters added in earlier commits on this branch. This commit removes the now-dead in-repo copy of that logic: - Deletes internal/notifications/ in full (http_wrapper, http_client_executor, engine, router + tests) now that every provider type dispatches through the extracted module's adapters. - Folds internal/notifications/feature_flags.go's Setting-table key constants into internal/services (Charon policy, not engine logic) so the notifications package can be removed entirely rather than left behind as a single-file package. - Removes the now-unreachable legacy JSON-payload dispatch path (sendJSONPayload and its per-provider validation/header/dispatch-URL helpers, the old dispatchEmail/sanitizeForEmail path, and the dead isPrivateIP wrapper) from notification_service.go, along with the test coverage that exercised those functions directly. Validation logic still reachable from live CRUD/test-send code paths (Discord/Slack URL validation, the template-preview endpoint) is kept unchanged. - Deletes the old engine-level integration test, superseded by the DI-seam-level coverage already added alongside the transport adapter. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
Notes that outbound notification dispatch (all seven HTTP provider types plus email) now goes through the external go_notify_yourself module via Charon-supplied SSRF/SMTP/template adapters, replacing the removed internal/notifications package. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
dispatchViaNotify and testProviderViaNotify each have a defensive error-handling branch for buildNotifySender rejecting an unrecognized provider type — unreachable through the public SendExternal/TestProvider entry points since their upstream type allowlists only ever pass buildNotifySender a type it supports, but worth covering directly to close the patch-coverage gap left by the surrounding dead-code removal. Claude-Session: https://claude.ai/code/session_01VeiFv1TDjzjnxbjbyNuSQX
go_notify_yourself has now been pushed to GitHub and tagged v0.1.0. Remove the local filesystem replace directive used during development and depend on the real published module instead.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Contributor
✅ Supply Chain Verification Results✅ PASSED 📦 SBOM Summary
🔍 Vulnerability Scan
📎 Artifacts
Generated by Supply Chain Verification workflow • View Details |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts Charon's notification delivery (HTTP dispatch, retries, SSRF guards, and all provider payload/template logic) into a standalone, reusable Go module —
go_notify_yourselfv0.1.0— and cuts Charon over to consume it. Full design/scoping is indocs/plans/notifications_extraction_spec.md(included in this PR).Motivation: the maintainer now runs multiple projects that all need notification delivery. Rather than re-implementing it per project, this pulls the engine out behind a small, dependency-free public API (
notify.Message,notify.Sender,transport.Wrapper) that any Go project cango get, with a longer-term direction of becoming a Go equivalent of Apprise — scoped for now to exactly the providers Charon already ships (no new integrations added).What moved vs. what stayed
Mailer+TemplateRenderer— now live ingo_notify_yourself, decoupled from Charon via two DI seams (ClientFactory,URLValidator) so the module has zerogithub.com/Wikid82/charon/*imports.security_notification_service.go/enhanced_security_notification_service.go, and the entire frontend.notify_client_adapter.go,notify_provider_adapter.go,notify_email_adapter.go) wiringinternal/network/internal/securityinto the module's DI seams.Deliberate behavior changes (both explicitly approved during scoping)
transport.Wrapperas every other provider — a real, user-visible reliability improvement, not just a refactor.The
detailedJSON template's backward compatibility was preserved via alegacyDetailedTemplatecompatibility shim innotify_provider_adapter.go— existing custom integrations parsing the old flat JSON shape see zero payload change.Validation
go build/go vet/staticcheck: cleanlefthook run pre-commit: cleanDeferred follow-ups (flagged by Supervisor review, not blocking this PR)
RenderTemplatepath rather than the new module — minor divergence from real dispatch, worth a follow-up ticket.Test plan
go test ./...)🤖 Generated with Claude Code