fix(webhook): thread scheduled_at into the review-hold event payloads - #944
Open
AmirF194 wants to merge 1 commit into
Open
fix(webhook): thread scheduled_at into the review-hold event payloads#944AmirF194 wants to merge 1 commit into
AmirF194 wants to merge 1 commit into
Conversation
buildPendingApprovalEvent and buildApprovedEvent (internal/agent/webhooks_api.go) never read msg.ScheduledAt / sent.ScheduledAt, so a webhook-only subscriber of email.review_requested / email.review_approved cannot tell a deferred send from an immediate one without a follow-up GET on the message. Include scheduled_at in both payloads when the row carries a schedule, matching the optional-field pattern already used for provider_message_id and lifecycle_transitions in the same builders. Fixes tokencanopy#836
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
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
buildPendingApprovalEventandbuildApprovedEvent(internal/agent/webhooks_api.go) never readmsg.ScheduledAt/sent.ScheduledAt, so a webhook-only subscriber ofemail.review_requested/email.review_approvedcannot tell a deferred send from an immediate one without a follow-upGETon the message. Addsscheduled_atto both payloads when the row carries a schedule, the same optional-field pattern the same builders already use forprovider_message_idandlifecycle_transitions.On the issue's third fix step (regenerate the OpenAPI + generated SDK event models):
email.review_requestedandemail.review_approvedare deliberately kept asmap[string]anyat their trigger sites and are not registered as OpenAPI component schemas (internal/eventpayload/payloads.go's package doc: these two stay untyped "until their shape settles"). There is no generated model or golden fixture for either event, so there is nothing to regenerate;make generate-sdk-checkand the OpenAPI contract gates don't cover this payload.Client surface checklist
Not applicable: this is an additive field on an already-untyped, beta webhook payload, not a typed API or client-SDK surface (see note above).
Test plan
TestBuildPendingApprovalEvent_CarriesScheduledAt/TestBuildApprovedEvent_CarriesScheduledAt: fail onmain, pass on this branch.TestBuildPendingApprovalEvent_OmitsScheduledAtForImmediateSend/TestBuildApprovedEvent_OmitsScheduledAtForImmediateSend: cover the unset case.go test ./...(full suite, go1.26 per CI) andmake fmt-checkboth pass.make cover) needs Postgres and doesn't gateinternal/agent, so I didn't run it.Fixes #836