Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions internal/plane/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ func VerifyAndParse(secret string, headers http.Header, body []byte) (*Event, er

func workItemKind(action string) (EventKind, error) {
switch action {
case actionCreate:
case actionCreate, actionCreated:
return EventWorkItemCreated, nil
case actionUpdate:
case actionUpdate, actionUpdated:
return EventWorkItemUpdated, nil
case actionDelete:
case actionDelete, actionDeleted:
return EventWorkItemDeleted, nil
default:
return "", fmt.Errorf("%w: issue action %q", ErrUnsupportedEvent, action)
Expand All @@ -112,11 +112,11 @@ func workItemKind(action string) (EventKind, error) {

func commentKind(action string) (EventKind, error) {
switch action {
case actionCreate:
case actionCreate, actionCreated:
return EventCommentCreated, nil
case actionUpdate:
case actionUpdate, actionUpdated:
return EventCommentUpdated, nil
case actionDelete:
case actionDelete, actionDeleted:
return EventCommentDeleted, nil
default:
return "", fmt.Errorf("%w: issue_comment action %q", ErrUnsupportedEvent, action)
Expand Down
69 changes: 69 additions & 0 deletions internal/plane/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"os"
"path/filepath"
"strings"
"testing"
)

Expand Down Expand Up @@ -265,3 +266,71 @@ func TestNewClient(t *testing.T) {
t.Errorf("client fields wrong: %+v", c)
}
}

// TestParse_RealPlaneActionVerbsAccepted pins the past-tense action
// verbs that real Plane (v1.3.1+) actually sends. The bridge originally
// shipped recognising only present-tense create/update/delete; every
// real Plane delivery hit ErrUnsupportedEvent and silently 204'd. See
// GH#10 (PFB-22) for the full reproduction.
//
// This test fails if anyone tightens the parser back to present-tense
// only.
func TestParse_RealPlaneActionVerbsAccepted(t *testing.T) {
t.Parallel()

const realPlanePayload = `{
"event": "issue",
"action": "updated",
"webhook_id": "3eda452e-0000-0000-0000-000000000001",
"workspace_id": "00000000-0000-0000-0000-000000000aaa",
"data": {
"id": "33333333-3333-3333-3333-333333333333",
"name": "real plane payload",
"project": "77777777-7777-7777-7777-777777777777",
"workspace": "00000000-0000-0000-0000-000000000aaa",
"sequence_id": 1
}
}`

cases := []struct {
event string
action string
wantKind EventKind
}{
{planeEventIssue, "created", EventWorkItemCreated},
{planeEventIssue, "updated", EventWorkItemUpdated},
{planeEventIssue, "deleted", EventWorkItemDeleted},
// Present-tense aliases — kept so hand-rolled clients / older
// Plane code paths that emit "create"/"update"/"delete" still
// work. Removing these would be a silent behavior change.
{planeEventIssue, "create", EventWorkItemCreated},
{planeEventIssue, "update", EventWorkItemUpdated},
{planeEventIssue, "delete", EventWorkItemDeleted},
{planeEventIssueComment, "created", EventCommentCreated},
{planeEventIssueComment, "updated", EventCommentUpdated},
{planeEventIssueComment, "deleted", EventCommentDeleted},
{planeEventIssueComment, "create", EventCommentCreated},
{planeEventIssueComment, "update", EventCommentUpdated},
{planeEventIssueComment, "delete", EventCommentDeleted},
}

for _, tc := range cases {
t.Run(tc.event+"_"+tc.action, func(t *testing.T) {
t.Parallel()
body := []byte(strings.ReplaceAll(
strings.ReplaceAll(realPlanePayload, `"event": "issue"`, `"event": "`+tc.event+`"`),
`"action": "updated"`, `"action": "`+tc.action+`"`,
))
h := http.Header{}
h.Set(HeaderEvent, tc.event)
h.Set(HeaderDelivery, "plane-action-verb-test")
ev, err := Parse(h, body)
if err != nil {
t.Fatalf("Parse failed for %s/%s: %v", tc.event, tc.action, err)
}
if ev.Kind != tc.wantKind {
t.Errorf("Kind = %q, want %q", ev.Kind, tc.wantKind)
}
})
}
}
2 changes: 1 addition & 1 deletion internal/plane/testdata/comment_created.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"event": "issue_comment",
"action": "create",
"action": "created",
"webhook_id": "11111111-1111-1111-1111-111111111111",
"workspace_id": "22222222-2222-2222-2222-222222222222",
"data": {
Expand Down
2 changes: 1 addition & 1 deletion internal/plane/testdata/comment_updated.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"event": "issue_comment",
"action": "update",
"action": "updated",
"webhook_id": "11111111-1111-1111-1111-111111111111",
"workspace_id": "22222222-2222-2222-2222-222222222222",
"data": {
Expand Down
2 changes: 1 addition & 1 deletion internal/plane/testdata/work_item_created.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"event": "issue",
"action": "create",
"action": "created",
"webhook_id": "11111111-1111-1111-1111-111111111111",
"workspace_id": "22222222-2222-2222-2222-222222222222",
"data": {
Expand Down
2 changes: 1 addition & 1 deletion internal/plane/testdata/work_item_deleted.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"event": "issue",
"action": "delete",
"action": "deleted",
"webhook_id": "11111111-1111-1111-1111-111111111111",
"workspace_id": "22222222-2222-2222-2222-222222222222",
"data": {
Expand Down
2 changes: 1 addition & 1 deletion internal/plane/testdata/work_item_updated.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"event": "issue",
"action": "update",
"action": "updated",
"webhook_id": "11111111-1111-1111-1111-111111111111",
"workspace_id": "22222222-2222-2222-2222-222222222222",
"data": {
Expand Down
16 changes: 12 additions & 4 deletions internal/plane/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,19 @@ const (
planeEventIssueComment = "issue_comment"
)

// Plane "action" values in the body envelope.
// Plane "action" values in the body envelope. Real Plane (verified
// against v1.3.1's webhook_task.py) sends past-tense verbs
// ("created"/"updated"/"deleted"), matching the GitHub/GitLab webhook
// convention. The present-tense form ("create"/"update"/"delete") shows
// up in older / non-bgtask code paths and in some hand-rolled clients,
// so the parser accepts both — the past-tense form is canonical.
const (
actionCreate = "create"
actionUpdate = "update"
actionDelete = "delete"
actionCreate = "create"
actionUpdate = "update"
actionDelete = "delete"
actionCreated = "created"
actionUpdated = "updated"
actionDeleted = "deleted"
)

// Actor is the (often partial) record of who triggered the event. Plane
Expand Down
19 changes: 16 additions & 3 deletions internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,14 +368,27 @@ func (s *Server) respondVerifyParseErr(w http.ResponseWriter, r *http.Request, s
case errors.Is(err, forge.ErrMissingEventHeader),
errors.Is(err, plane.ErrMissingEventHeader):
http.Error(w, "missing event header", http.StatusBadRequest)
case errors.Is(err, forge.ErrUnsupportedEvent),
errors.Is(err, plane.ErrUnsupportedEvent):
// Acknowledge events we don't handle so the forge stops retrying.
case errors.Is(err, forge.ErrUnsupportedEvent):
// Forges legitimately fan out events the operator didn't subscribe
// to (e.g. push events from a repo whose webhook also covers
// issues). Log at DEBUG so production isn't flooded.
s.log.LogAttrs(r.Context(), slog.LevelDebug, "webhook event ignored",
slog.String("side", side),
slog.String("err", err.Error()),
)
w.WriteHeader(http.StatusNoContent)
case errors.Is(err, plane.ErrUnsupportedEvent):
// Plane only sends event types the operator subscribed to, so
// any unsupported event here means a contract mismatch with
// real Plane (e.g. an action-verb spelling we don't recognise).
// Log at INFO so it surfaces at the default log level — silent
// 204s on a wholly-broken integration are a brutal failure
// mode. See GH#10 (PFB-22).
s.log.LogAttrs(r.Context(), slog.LevelInfo, "plane webhook event ignored",
slog.String("side", side),
slog.String("err", err.Error()),
)
w.WriteHeader(http.StatusNoContent)
case errors.Is(err, forge.ErrMalformedPayload),
errors.Is(err, plane.ErrMalformedPayload):
s.log.LogAttrs(r.Context(), slog.LevelWarn, "webhook payload malformed",
Expand Down
Loading