feat: add Web Push notification provider (go_notify_yourself v0.3.0) - #1340
Merged
Merged
Conversation
Pulls in the providers/webpush package (RFC 8030/8291/8292 direct browser Web Push, no third-party relay) needed for the upcoming Web Push notification provider. go.sum diff reviewed: only the version line for this module changes, no new transitive dependencies. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Encodes the Web Push notification provider's intended behavior as test.fixme specs against the approved spec's API contracts (§3.4) and frontend design (§3.6), before any backend/frontend implementation lands. Covers admin-only provisioning, per-device subscribe/unsubscribe (with the browser Push API stubbed via page.addInitScript, since real push delivery cannot be exercised in CI), and per-event-type toggle regression for the new provider type. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Adds the child table holding each browser's Web Push destination (FK to NotificationProvider, Type="webpush"), registers it in AutoMigrate, and creates the idx_webpush_singleton partial unique index (CREATE UNIQUE INDEX ... WHERE type='webpush') immediately after AutoMigrate to close a race condition where two concurrent provisioning requests could otherwise create two independent VAPID identities (service-layer COUNT-then-INSERT alone is not atomic under SQLite's single-writer connection pool). Includes a concurrency regression test firing two simultaneous INSERTs against the index and asserting exactly one succeeds. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Adds "webpush" to isSupportedNotificationProviderType and supportsJSONTemplates (its plaintext payload is JSON, same providers/internal/render convention as every other JSON-template type), a new FlagWebPushServiceEnabled dispatch-enabled flag, the providers/webpush blank import for registry self-registration, and "webpush" in the registry consistency test's supportedTypes slice. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Adds dispatchWebPushViaNotify (notify_webpush_adapter.go): fans a single logical notification out to every WebPushSubscription row under a webpush provider, one webpush.Client per row, sequentially within the provider's already-backgrounded dispatch goroutine to bound outbound concurrency. extractHTTPStatusFromNotifyError regex-parses the numeric HTTP status out of transport.Wrapper's plain formatted error string (go_notify_yourself exposes no typed status error) to detect the standard Web Push 404/410 "subscription is gone" signal. A subscription reported gone is pruned immediately; any other failure increments FailureCount and prunes at webpushMaxConsecutiveFailures (10) to bound the table without nuking a subscription on one transient failure. Wires the new "webpush" branch into SendExternal's dispatch loop, parallel to the existing "email" special case. New file is at 100% statement coverage. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Adds WebPushHandler (Provision, VAPIDPublicKey, Subscribe, ListSubscriptions, Unsubscribe) and the matching NotificationService methods (ProvisionWebPush, GetWebPushVAPIDPublicKey, RegisterWebPushSubscription, ListWebPushSubscriptionsForUser, DeleteWebPushSubscription), wired under the management route group. Provisioning is admin-only (RequireRole(admin), mirroring the Test/Preview precedent); VAPID key read, subscribe, list, and unsubscribe are self-service for any management-access user (§3.4.0). ProvisionWebPush's cheap COUNT fast-path is backstopped by the idx_webpush_singleton partial unique index added in a prior commit; a losing CreateProvider INSERT is caught and mapped to 409, never a raw 500, using this codebase's existing errors.Is(gorm.ErrDuplicatedKey)/"UNIQUE constraint failed" detection idiom. Also fixes a token-wiping bug the new type surfaced: CreateProvider and UpdateProvider's token-preserving allowlists (and the generic Update handler's provider-type allowlist) didn't include "webpush", which would have silently erased the VAPID private key on create/ update. Not in the original commit-6 file list, but required for ProvisionWebPush and any admin edit of a webpush provider's preferences to work at all. Includes the two required regression tests: concurrent Provision requests never surface a raw 500 (only one 201, the loser 409), and a RoleUser caller gets 403 from the existing generic PUT /notifications/providers/:id when setting a NotifySecurityXxx field on a webpush row (pins already-existing admin-gate behavior, no new authz code). Also updates TestManagementGroup_MutationsAreAdminGuarded's userOKMutationAllowlist for the two intentionally self-service webpush routes. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Adds the frontend half of Web Push notifications: a minimal service worker (push/notificationclick), a base64url-to-Uint8Array helper for the VAPID public key, typed API client functions for the five backend webpush endpoints, and Notifications page UI for admin-only provisioning, per-device subscribe/unsubscribe, and the not-provisioned/unsupported-browser states. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Reflects the supervisor review pass: DB-level singleton enforcement via partial unique index, the resolved auth-scoping decision, and a documented (non-blocking) risk note on VAPID key plaintext storage. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Adds Web Push to the notification-provider list and feature summary, walks through admin provisioning and per-device subscribing in docs/features/notifications.md, and notes the new WebPushSubscription model and provider entry in ARCHITECTURE.md. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
Flip the Web Push provider E2E specs from test.fixme to live tests now that the backend/frontend implementation has landed, and adapt them to the actual UI: provisioning and device subscribe/unsubscribe happen through a dedicated Web Push card rather than the generic Add Provider form, and the device toggle is a subscribe button plus a per-row remove button rather than a single on/off control. Claude-Session: https://claude.ai/code/session_01YMic1SfEixL9Ej3RvnDLFN
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 |
Two concurrent provision requests could surface a raw 500 instead of mapping the loser to 409: SQLite's shared-cache mode returns "database table is locked" (SQLITE_LOCKED) on a racing INSERT, which isn't covered by the driver's busy_timeout (that only retries SQLITE_BUSY). Retry both the fast-path COUNT check and the INSERT on this class of transient error, mirroring the existing retry pattern in security_service.go and credential_service.go.
webpush_provider_service.go had no tests in its own package (only indirect exercise via the handlers package, which Go's default per-package coverage instrumentation doesn't attribute back to this file), leaving it at 0% patch coverage. Add direct tests for ProvisionWebPush, GetWebPushVAPIDPublicKey, RegisterWebPushSubscription, ListWebPushSubscriptionsForUser, DeleteWebPushSubscription, and the SendExternal/isDispatchEnabled webpush dispatch branches in notification_service.go.
The notifications settings page now always renders a singleton
WebPushCard with its own "Name" field, so the pre-existing Discord
payload-contract E2E test's unscoped page.getByLabel('Name') resolved
to two elements (strict-mode violation) — the actual cause of the
Shard 3 E2E failures on all three browsers. Give ProviderForm's <form>
a data-testid and scope the assertion to it.
Also close out the frontend patch-coverage gaps: an isWebPush-editing
unit test (hides the URL field, shows the Web Push guidance note) and
several WebPushCard branch cases (empty provision name default,
subscribe-before-VAPID-loaded guard, missing PushSubscription JSON
fields, non-Error subscribe failure, subscription row with no user
agent).
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
go_notify_yourselfv0.3.0's newproviders/webpushpackage (VAPID/RFC 8291/8292 Web Push).NotificationProviderrow (type="webpush", DB-enforced via a partial unique index) holds the VAPID application identity, with a newWebPushSubscriptiontable holding each browser/device destination. Dispatch fans out onewebpush.Clientper subscription, auto-pruning subscriptions a push service reports as 404/410 Gone.Commits
test: add e2e specs for web push subscribe/unsubscribe flow (fixme)chore: bump go_notify_yourself to v0.3.0feat: add WebPushSubscription model, migration, and singleton indexfeat: wire webpush into notify provider allowlistfeat: add web push dispatch fan-out and subscription pruningfeat: add web push provisioning and subscription API endpointsfeat: add web push service worker and subscribe/unsubscribe UIdocs: update web push spec with review resolutionsdocs: document web push notification providertest: enable web push e2e specsTest plan
go build ./...andnpm run buildsucceedgo test ./internal/models/... ./internal/services/... ./internal/api/handlers/...), including a concurrency test proving the singleton partial-unique-index closes the create race, and a regression test provingRoleUsergets403setting security-forward toggles on a webpush provider row./scripts/scan-gorm-security.sh --check— zero CRITICAL/HIGHscripts/go-test-coverage.sh— 88.3% line coverage (≥87% gate)scripts/frontend-test-coverage.sh— 90.95% line coverage (≥87% gate)npm run type-checkcleannpx playwright test tests/e2e/notifications-webpush.spec.ts --project=firefox— 7/7 passedFull technical spec:
docs/plans/current_spec.md.