Repository navigation
Harden webhook delivery prune and clarify delivery-log docs - #913
Conversation
Clarify that URL-secret mismatches and rate-limited requests do not write delivery rows, move webhook table notes out of the JSON-shadow section, and bind user_id into delivery retention deletes.
📝 WalkthroughWalkthroughWebhook documentation now defines when delivery records are written, while retention cleanup scopes retained rows by both user and endpoint. ChangesWebhook delivery behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-913.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/contributing/architecture/webhooks.md`:
- Around line 50-51: Update the “Authenticated deliveries” statement in the
webhook architecture documentation to replace “missing declaration” with
“missing verification secret,” preserving the existing delivery-row and
no-payload details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ec92159-73f0-4a5f-ba63-2aa74ea4fadf
📒 Files selected for processing (2)
docs/contributing/architecture/webhooks.mdpackages/worker/src/webhooks/repo.ts
| 9. Authenticated deliveries (and post-auth rejects such as HMAC / size / missing | ||
| declaration) record a `webhook_deliveries` row (no payload body); ~50 rows |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the post-auth rejection list.
“Missing declaration” is handled as an earlier 404 with no delivery row in lines 36-39. Remove it here or replace it with the actual post-auth case, such as a missing verification secret.
Suggested wording
- record a `webhook_deliveries` row (no payload body); ~50 rows
+ record a `webhook_deliveries` row (no payload body); ~50 rowsReplace “missing declaration” in the preceding line with “missing verification secret.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/contributing/architecture/webhooks.md` around lines 50 - 51, Update the
“Authenticated deliveries” statement in the webhook architecture documentation
to replace “missing declaration” with “missing verification secret,” preserving
the existing delivery-row and no-payload details.
Summary
Small follow-up after #910 / #912 review feedback:
DELETEbyuser_idas well asendpoint_idSystem recap — extends Inbound webhooks (medium risk)
Mode: recap · Base:
main· Head: current branch tipClassification: extends — retention SQL isolation + architecture doc accuracy for
webhooks.Primitives touched
webhooksuser_id; docs match ingress behaviord1-app-dbSystem map
Delivery-log retention deletes now include
user_idin both the outer delete and the keep-set subquery.Legend: green = composes (wiring only) · amber = extended by this PR · red =
new primitive · gray = context (unchanged, included only when an edge crosses
it).
Test plan
Summary by CodeRabbit