Skip to content

Conversation

@youbek
Copy link
Collaborator

@youbek youbek commented Dec 11, 2025

Description

This PR updates python generated types after push notification schema changes made to the adcp protocol:

Also, updates/adds utility functions for handling and creating webhook payloads specifically utilities:

  • create_a2a_webhook_payload
  • create_mcp_webhook_payload
  • get_adcp_signed_headers_for_webhook

Demo (MCP)

Demo (A2A)

@youbek youbek force-pushed the feat/update-types-for-latest-schemas branch from fdff941 to 6cba9ef Compare December 11, 2025 19:06
@youbek youbek changed the title feat: update types for latest schemas feat: update adcp types for latest push notification schemas and update utility functions around webhooks Dec 15, 2025
Copy link

@nastassiafulconis nastassiafulconis left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! The webhook utilities are a great addition. I found a few issues that need addressing before we can merge - mostly around signature verification security. See inline comments below.

Copy link

@nastassiafulconis nastassiafulconis left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! The webhook utilities are a great addition. Found a few things that need addressing - mostly around signature verification. See inline comments.

@nastassiafulconis
Copy link

A few more things I noticed:

client.py signature verification - double check that the verification strips the sha256= prefix before comparing. the header includes it but hmac.compare_digest expects raw hex

tests - solid coverage! one thing that might be worth adding is an end-to-end test that creates a payload with create_mcp_webhook_payload, signs it with get_adcp_signed_headers_for_webhook, then verifies with handle_webhook. would catch any mismatches between the generation and verification sides

breaking changes - might be worth noting in the PR description that the handle_webhook signature changed, in case anyone is already using it

@youbek youbek requested review from BaiyuScope3, bokelley and nastassiafulconis and removed request for BaiyuScope3 and nastassiafulconis December 18, 2025 20:41
@youbek youbek merged commit 1975ca9 into main Dec 19, 2025
7 checks passed
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.

4 participants