From 9d03a001cfc64450837c18361a4a68151741c1ac Mon Sep 17 00:00:00 2001 From: lucas picollo Date: Thu, 20 Aug 2026 14:36:43 -0300 Subject: [PATCH] fix: webhook update sends a body ClickUp accepts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `webhook update` could not change a webhook's endpoint. Three bugs, all in the request body. `Events` was typed `string`. ClickUp wants an array and answers 400 "Invalid events" (OAUTH_150) to a scalar — for the exact value its own GET returns, so feeding `list` output back through `update` failed. No field had `omitempty`, so an endpoint-only update still sent `"status": ""` and `"events": ""`. ClickUp answers 500 (OAUTH_152) to the empty status. The same update with those keys absent returns 200: PUT here behaves like PATCH and preserves anything the body omits. `--events '*'`, which the flag's own help recommended, is worse than broken: ClickUp answers 200 and sets `events: []`. It unsubscribes the webhook from everything instead of subscribing it to everything. Now refused with an explanation, since a silent unsubscribe on a live webhook has no symptom until the deliveries stop. Also refuse an update with nothing in it rather than sending an empty body. Verified against the live API on a throwaway webhook: endpoint-only, events round-trip, status-only, '*' refused, empty refused. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/webhook.go | 19 +++++++++-- internal/api/webhooks.go | 15 +++++++-- internal/api/webhooks_test.go | 59 +++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/cmd/webhook.go b/cmd/webhook.go index 9df1fd1..58df5f0 100644 --- a/cmd/webhook.go +++ b/cmd/webhook.go @@ -81,7 +81,7 @@ var webhookUpdateCmd = &cobra.Command{ ctx := context.Background() id, _ := cmd.Flags().GetString("id") endpoint, _ := cmd.Flags().GetString("endpoint") - events, _ := cmd.Flags().GetString("events") + events, _ := cmd.Flags().GetStringSlice("events") status, _ := cmd.Flags().GetString("status") if id == "" { @@ -89,6 +89,21 @@ var webhookUpdateCmd = &cobra.Command{ return &exitError{code: 1} } + if endpoint == "" && len(events) == 0 && status == "" { + output.PrintError("VALIDATION_ERROR", "nothing to update: pass at least one of --endpoint, --events, --status") + return &exitError{code: 1} + } + + // ClickUp accepts `*` on update and answers 200 with `events: []` — it + // unsubscribes the webhook from everything instead of subscribing it to + // everything. Refusing beats a silent unsubscribe on a live webhook. + for _, e := range events { + if e == "*" { + output.PrintError("VALIDATION_ERROR", "--events does not accept '*' on update: ClickUp clears the event list instead of subscribing to all. Name each event explicitly.") + return &exitError{code: 1} + } + } + req := &api.UpdateWebhookRequest{Endpoint: endpoint, Events: events, Status: status} resp, err := client.UpdateWebhook(ctx, id, req) if err != nil { @@ -131,7 +146,7 @@ func init() { webhookUpdateCmd.Flags().String("id", "", "Webhook ID (required)") webhookUpdateCmd.Flags().String("endpoint", "", "Webhook URL") - webhookUpdateCmd.Flags().String("events", "", "Events (use * for all)") + webhookUpdateCmd.Flags().StringSlice("events", nil, "Events to subscribe to; repeat or comma-separate. Omit to leave unchanged ('*' is rejected — ClickUp clears the list)") webhookUpdateCmd.Flags().String("status", "", "Status (active/inactive)") webhookDeleteCmd.Flags().String("id", "", "Webhook ID (required)") diff --git a/internal/api/webhooks.go b/internal/api/webhooks.go index 7b01f2c..a00586d 100644 --- a/internal/api/webhooks.go +++ b/internal/api/webhooks.go @@ -38,10 +38,19 @@ type CreateWebhookResponse struct { Webhook Webhook `json:"webhook"` } +// UpdateWebhookRequest is a PATCH in PUT's clothing: ClickUp preserves any field +// the body omits, so every field is omitempty and callers send only what changes. +// +// Both tags matter, and each cost a real outage to find: +// - `Events` must marshal as an ARRAY. As a scalar string ClickUp answers +// 400 "Invalid events" (OAUTH_150) — for the very value its own GET returns. +// - Without omitempty, an endpoint-only update still sends `"status": ""`, +// which ClickUp answers 500 (OAUTH_152). The endpoint-only update it +// rejected is one the API accepts happily when the empty field is absent. type UpdateWebhookRequest struct { - Endpoint string `json:"endpoint"` - Events string `json:"events"` - Status string `json:"status"` + Endpoint string `json:"endpoint,omitempty"` + Events []string `json:"events,omitempty"` + Status string `json:"status,omitempty"` } type UpdateWebhookResponse struct { diff --git a/internal/api/webhooks_test.go b/internal/api/webhooks_test.go index 4cc0956..7286e6a 100644 --- a/internal/api/webhooks_test.go +++ b/internal/api/webhooks_test.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "testing" @@ -60,3 +61,61 @@ func TestDeleteWebhook(t *testing.T) { t.Fatal(err) } } + +// The two shapes that broke a live repoint on 20/ago/2026: a scalar `events` +// (400 "Invalid events" for a value GET had just returned) and an empty +// `status` riding along on an endpoint-only update (500). Both are payload +// bugs, so assert on the bytes on the wire, not on the response. +func TestUpdateWebhookBody(t *testing.T) { + ctx := context.Background() + cases := []struct { + name string + req *UpdateWebhookRequest + want map[string]interface{} + }{ + { + name: "endpoint only omits events and status", + req: &UpdateWebhookRequest{Endpoint: "https://example.com/hook"}, + want: map[string]interface{}{"endpoint": "https://example.com/hook"}, + }, + { + name: "events marshal as an array", + req: &UpdateWebhookRequest{Events: []string{"taskStatusUpdated"}}, + want: map[string]interface{}{"events": []interface{}{"taskStatusUpdated"}}, + }, + { + name: "status alone", + req: &UpdateWebhookRequest{Status: "active"}, + want: map[string]interface{}{"status": "active"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var got map[string]interface{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PUT" || r.URL.Path != "/v2/webhook/wh1" { + t.Errorf("unexpected: %s %s", r.Method, r.URL.Path) + } + if err := json.NewDecoder(r.Body).Decode(&got); err != nil { + t.Fatalf("decode body: %v", err) + } + _ = json.NewEncoder(w).Encode(UpdateWebhookResponse{ID: "wh1"}) + })) + defer srv.Close() + + c := &Client{BaseURL: srv.URL, Token: "test", HTTPClient: srv.Client()} + if _, err := c.UpdateWebhook(ctx, "wh1", tc.req); err != nil { + t.Fatal(err) + } + if len(got) != len(tc.want) { + t.Fatalf("body = %v, want exactly %v", got, tc.want) + } + for k, v := range tc.want { + if fmt.Sprintf("%v", got[k]) != fmt.Sprintf("%v", v) { + t.Errorf("body[%q] = %v, want %v", k, got[k], v) + } + } + }) + } +}