From 517cb0ce735234a6ad73c7ce41318f82f55915f8 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 3 Mar 2026 09:49:04 +0000 Subject: [PATCH] refactor(webpush): use RequireExperimentWithDevBypass middleware (#22525) Replace manual experiment checks in web-push handlers with the `RequireExperimentWithDevBypass` middleware on the route group, matching the pattern used by OAuth2, Agents, and MCP experiments. ## Changes - **`coderd/coderd.go`**: Add `RequireExperimentWithDevBypass` middleware to `/webpush` route group - **`coderd/webpush.go`**: Remove inline `api.Experiments.Enabled(codersdk.ExperimentWebPush)` checks from all three handlers - **`cli/server.go`**: Gate webpush dispatcher initialization with `buildinfo.IsDev()` fallback so dev builds always init the real dispatcher - **`coderd/webpush_test.go`**: Remove experiment enablement from tests (dev bypass handles it) Net effect: -26 lines removed, +5 added. Created using whatchamacallits (Opus 4.6 Max) --- cli/server.go | 2 +- coderd/coderd.go | 1 + coderd/webpush.go | 15 --------------- coderd/webpush_test.go | 13 +++---------- 4 files changed, 5 insertions(+), 26 deletions(-) diff --git a/cli/server.go b/cli/server.go index 08f835c67a..0df991e56e 100644 --- a/cli/server.go +++ b/cli/server.go @@ -845,7 +845,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. // Manage push notifications. experiments := coderd.ReadExperiments(options.Logger, options.DeploymentValues.Experiments.Value()) - if experiments.Enabled(codersdk.ExperimentWebPush) { + if experiments.Enabled(codersdk.ExperimentWebPush) || buildinfo.IsDev() { if !strings.HasPrefix(options.AccessURL.String(), "https://") { options.Logger.Warn(ctx, "access URL is not HTTPS, so web push notifications may not work on some browsers", slog.F("access_url", options.AccessURL.String())) } diff --git a/coderd/coderd.go b/coderd/coderd.go index 353ae7e0e4..0d27dfd711 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1480,6 +1480,7 @@ func New(options *Options) *API { }) }) r.Route("/webpush", func(r chi.Router) { + r.Use(httpmw.RequireExperimentWithDevBypass(api.Experiments, codersdk.ExperimentWebPush)) r.Post("/subscription", api.postUserWebpushSubscription) r.Delete("/subscription", api.deleteUserWebpushSubscription) r.Post("/test", api.postUserPushNotificationTest) diff --git a/coderd/webpush.go b/coderd/webpush.go index 3e84aa012e..4d7687f8e9 100644 --- a/coderd/webpush.go +++ b/coderd/webpush.go @@ -28,11 +28,6 @@ import ( func (api *API) postUserWebpushSubscription(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) - if !api.Experiments.Enabled(codersdk.ExperimentWebPush) { - httpapi.ResourceNotFound(rw) - return - } - var req codersdk.WebpushSubscription if !httpapi.Read(ctx, rw, r, &req) { return @@ -77,11 +72,6 @@ func (api *API) deleteUserWebpushSubscription(rw http.ResponseWriter, r *http.Re ctx := r.Context() user := httpmw.UserParam(r) - if !api.Experiments.Enabled(codersdk.ExperimentWebPush) { - httpapi.ResourceNotFound(rw) - return - } - var req codersdk.DeleteWebpushSubscription if !httpapi.Read(ctx, rw, r, &req) { return @@ -137,11 +127,6 @@ func (api *API) postUserPushNotificationTest(rw http.ResponseWriter, r *http.Req ctx := r.Context() user := httpmw.UserParam(r) - if !api.Experiments.Enabled(codersdk.ExperimentWebPush) { - httpapi.ResourceNotFound(rw) - return - } - // We need to authorize the user to send a push notification to themselves. if !api.Authorize(r, policy.ActionCreate, rbac.ResourceNotificationMessage.WithOwner(user.ID.String())) { httpapi.Forbidden(rw) diff --git a/coderd/webpush_test.go b/coderd/webpush_test.go index 467f507881..b36af9e236 100644 --- a/coderd/webpush_test.go +++ b/coderd/webpush_test.go @@ -30,11 +30,7 @@ func TestWebpushSubscribeUnsubscribe(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) - dv := coderdtest.DeploymentValues(t) - dv.Experiments = []string{string(codersdk.ExperimentWebPush)} - client := coderdtest.New(t, &coderdtest.Options{ - DeploymentValues: dv, - }) + client := coderdtest.New(t, &coderdtest.Options{}) owner := coderdtest.CreateFirstUser(t, client) memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) _, anotherMember := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) @@ -112,12 +108,9 @@ func TestDeleteWebpushSubscription(t *testing.T) { store, ps := dbtestutil.NewDB(t) wrappedStore := &testWebpushErrorStore{Store: store} - dv := coderdtest.DeploymentValues(t) - dv.Experiments = []string{string(codersdk.ExperimentWebPush)} client := coderdtest.New(t, &coderdtest.Options{ - DeploymentValues: dv, - Database: wrappedStore, - Pubsub: ps, + Database: wrappedStore, + Pubsub: ps, }) owner := coderdtest.CreateFirstUser(t, client) memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)