Register webhook and script in create PR command#3998
Register webhook and script in create PR command#3998richardhjtan wants to merge 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1b34cd1f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Preview deployments |
Host Test Results 1 files ±0 1 suites ±0 1h 38m 42s ⏱️ + 4m 52s Results for commit 500702b. ± Comparison against base commit 61ceac3. This pull request removes 1 and adds 8 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
This pull request adds webhook registration functionality to enable GitHub webhook notifications for pull requests. The implementation includes infrastructure for creating and managing incoming webhooks, filtering webhook commands by event type and PR number, and a script for easy webhook setup during local development with ngrok.
Changes:
- Added webhook registration capability to the create-listing-pr command to automatically set up GitHub webhooks for PR status updates
- Implemented command filtering logic in the webhook receiver to filter by event type and PR number
- Created a standalone script for manual webhook registration with idempotent behavior
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/pr-manifest.ts | Adds webhook metadata (id, path, signingSecret) to PrManifest interface |
| packages/realm-server/handlers/handle-webhook-receiver.ts | Implements webhook command lookup and filtering by event type and PR number |
| packages/realm-server/tests/server-endpoints/webhook-receiver-test.ts | Adds comprehensive tests for command execution and filtering scenarios |
| packages/matrix/scripts/register-github-webhook.ts | New script for registering GitHub webhooks with idempotent behavior |
| packages/host/app/services/realm-server.ts | Adds service methods for creating webhooks and webhook commands |
| packages/host/app/commands/create-listing-pr.ts | Integrates webhook registration into PR creation flow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| webhook?: { | ||
| id: string; // UUID from incoming_webhooks table | ||
| path: string; // webhook_path (e.g., "whk_abc123...") | ||
| signingSecret: string; // For GitHub webhook configuration |
There was a problem hiding this comment.
The signing secret is sensitive information used to verify GitHub webhook signatures. Storing it in the PrManifest may pose a security risk if this manifest is persisted or exposed through any API endpoints. Consider whether the signing secret needs to be stored in the manifest at all - it may be sufficient to only store the webhook ID and path, and retrieve the signing secret from the database when needed for configuration purposes. If the manifest is only used transiently and never persisted or exposed, this is acceptable, but the comment "Store minimal manifest in room state" suggests it may be stored.
| signingSecret: string; // For GitHub webhook configuration | |
| signingSecretId: string; // Reference/ID for secret used to verify GitHub webhooks (not the raw secret) |
| const webhookConfig = { | ||
| verificationType: 'HMAC_SHA256_HEADER' as const, | ||
| verificationConfig: { | ||
| header: 'x-hub-signature-256', |
There was a problem hiding this comment.
The header name should be 'X-Hub-Signature-256' with capital letters instead of 'x-hub-signature-256'. While HTTP headers are case-insensitive, the verification logic in handle-webhook-receiver.ts normalizes headers to lowercase before comparison (line 178), but the GitHub API documentation and all existing tests in the codebase consistently use 'X-Hub-Signature-256' with capital letters. This inconsistency could cause confusion and should be corrected for consistency with the rest of the codebase.
| header: 'x-hub-signature-256', | |
| header: 'X-Hub-Signature-256', |
64d9e7e to
9f276e7
Compare
9f276e7 to
acb7522
Compare
c1e3cf9 to
172fcd6
Compare
GitHub Webhook Setup for Local Testing
Quick guide for testing webhooks locally with ngrok.
Quick Start
1. Start ngrok
Copy the URL from output:
https://abc123.ngrok.io2. Register webhook
Copy the webhook URL and signing secret from output.
3. Add to GitHub
application/json4. Test
Create a PR or use GitHub's webhook test feature to verify it works.
When ngrok Restarts (URL Changed)
Troubleshooting
Webhooks not working after ngrok restart?
→ ngrok URL changed. Re-run script and update URL in GitHub.
Getting 401 errors?
→ Verify signing secret in GitHub matches the script output.
Webhooks not received?
Monitor webhook deliveries
For more details, see GitHub Webhooks Documentation.