Skip to content

fix(webhook): thread scheduled_at into the review-hold event payloads - #944

Merged
jiashuoz merged 1 commit into
tokencanopy:mainfrom
AmirF194:fix/836-webhook-scheduled-at
Sep 1, 2026
Merged

fix(webhook): thread scheduled_at into the review-hold event payloads#944
jiashuoz merged 1 commit into
tokencanopy:mainfrom
AmirF194:fix/836-webhook-scheduled-at

Conversation

@AmirF194

Copy link
Copy Markdown
Contributor

Summary

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. Adds scheduled_at to both payloads when the row carries a schedule, the same optional-field pattern the same builders already use for provider_message_id and lifecycle_transitions.

On the issue's third fix step (regenerate the OpenAPI + generated SDK event models): email.review_requested and email.review_approved are deliberately kept as map[string]any at 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-check and 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 on main, pass on this branch.
  • TestBuildPendingApprovalEvent_OmitsScheduledAtForImmediateSend / TestBuildApprovedEvent_OmitsScheduledAtForImmediateSend: cover the unset case.
  • go test ./... (full suite, go1.26 per CI) and make fmt-check both pass.
  • Did not check: the coverage-gate job (make cover) needs Postgres and doesn't gate internal/agent, so I didn't run it.

Fixes #836

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
@AmirF194
AmirF194 requested a review from jiashuoz as a code owner August 27, 2026 21:09

@jiashuoz jiashuoz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the changes and verified that the required CI checks pass.

@jiashuoz
jiashuoz merged commit cbfba9b into tokencanopy:main Sep 1, 2026
29 checks passed
@AmirF194

AmirF194 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the quick review and merge.

@AmirF194
AmirF194 deleted the fix/836-webhook-scheduled-at branch September 1, 2026 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

webhook events don't carry scheduled_at for held/scheduled sends (#815 follow-up)

2 participants