fix: fix webhook docs for semantics and accuracy#25
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughDocumentation across webhook guides was updated: platform-specific wording was generalized, webhook management endpoints moved to project-scoped ChangesWebhook docs: platform generalization, reply flow, idempotency, and project-scoped endpoints
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webhooks/overview.mdx (1)
83-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the warning endpoint to the project-scoped path.
Line 83 still says
POST /webhooks/, which conflicts with the new project-scoped registration flow and can cause failed attempts.Suggested fix
-Your signing secret is returned exactly once, in the response of `POST /webhooks/`. There is no "show me my secret" endpoint. Store it in your secret manager immediately. If you lose it, delete the webhook and re-register the URL — you'll get a new id and a new secret. +Your signing secret is returned exactly once, in the response of `POST /projects/$PROJECT_ID/webhooks/`. There is no "show me my secret" endpoint. Store it in your secret manager immediately. If you lose it, delete the webhook and re-register the URL — you'll get a new id and a new 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 `@webhooks/overview.mdx` at line 83, Update the documentation text that currently references the global endpoint "POST /webhooks/" to the project-scoped registration endpoint "POST /projects/:projectId/webhooks" used by the new flow; edit the sentence in overview.mdx to state the secret is returned in the response to POST /projects/:projectId/webhooks, and ensure the guidance to store the secret and to delete+re-register if lost remains unchanged.
🧹 Nitpick comments (1)
webhooks/troubleshooting.mdx (1)
80-80: ⚡ Quick winSplit this into shorter second-person sentences.
Line 80 packs multiple ideas and shifts away from direct “you” language. Splitting improves scanability in the checklist flow.
Suggested rewrite
- A platform that's enabled in the dashboard but not actually connected — an unpaired iMessage line, an expired WhatsApp token, a custom provider whose lifecycle handler is throwing — produces zero inbound events. Webhooks deliver what the SDK receives, so if the SDK is silent for a platform, that platform's webhooks are silent too. Check the SDK side first. + A platform can be enabled in the dashboard but still disconnected. You get zero inbound events in that state. This can happen when your iMessage line is unpaired, your WhatsApp token is expired, or your custom provider lifecycle handler throws. Webhook delivery mirrors what the SDK receives. If your SDK is silent for a platform, your webhooks are silent too. Check the SDK side first.As per coding guidelines, “Use active voice and second person ("you") in documentation” and “Keep sentences concise with one idea per sentence in documentation”.
🤖 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 `@webhooks/troubleshooting.mdx` at line 80, The long sentence starting "A platform that's enabled in the dashboard but not actually connected — an unpaired iMessage line, an expired WhatsApp token, a custom provider whose lifecycle handler is throwing — produces zero inbound events." should be rewritten into short, second‑person sentences using active voice; split the checklist into 2–3 concise sentences such as "If a platform is enabled in your dashboard but not actually connected, you will see no inbound events." then list examples in a separate sentence like "For example, an unpaired iMessage line, an expired WhatsApp token, or a failing custom provider will cause the SDK to receive no events." Ensure each sentence contains one idea and keeps "you"/"your" phrasing (edit the line in webhooks/troubleshooting.mdx).
🤖 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 `@webhooks/events.mdx`:
- Around line 107-116: The example currently scopes deduplication by default to
`${webhookId}:${payload.message.id}` which contradicts the doc text; change the
default dedupe key to use `payload.message.id` alone and update the explanatory
text so project-wide deduplication is the default, and then add the scoped
example showing how to combine `webhookId` and `payload.message.id` when
per-webhook idempotency is desired; update references to `dedupeKey`,
`payload.message.id`, `webhookId`, `store.exists`, `processOnce`, and
`store.set` in the doc to match this corrected behavior.
In `@webhooks/managing-webhooks.mdx`:
- Around line 57-58: Rewrite the two URL-requirement bullet points to use
second-person active voice (addressing the reader as "you") rather than
subjectless fragments: change the HTTPS bullet to something like "You must use
an https:// URL; we accept http:// today but delivery is plaintext and can be
read or forged on the network," and change the reachability bullet to something
like "Your webhook endpoint must be reachable from the public internet — if
Spectrum can't reach it, deliveries will fail after retries." Update the
existing bullets that mention HTTPS and public reachability to match this tone
and wording.
---
Outside diff comments:
In `@webhooks/overview.mdx`:
- Line 83: Update the documentation text that currently references the global
endpoint "POST /webhooks/" to the project-scoped registration endpoint "POST
/projects/:projectId/webhooks" used by the new flow; edit the sentence in
overview.mdx to state the secret is returned in the response to POST
/projects/:projectId/webhooks, and ensure the guidance to store the secret and
to delete+re-register if lost remains unchanged.
---
Nitpick comments:
In `@webhooks/troubleshooting.mdx`:
- Line 80: The long sentence starting "A platform that's enabled in the
dashboard but not actually connected — an unpaired iMessage line, an expired
WhatsApp token, a custom provider whose lifecycle handler is throwing — produces
zero inbound events." should be rewritten into short, second‑person sentences
using active voice; split the checklist into 2–3 concise sentences such as "If a
platform is enabled in your dashboard but not actually connected, you will see
no inbound events." then list examples in a separate sentence like "For example,
an unpaired iMessage line, an expired WhatsApp token, or a failing custom
provider will cause the SDK to receive no events." Ensure each sentence contains
one idea and keeps "you"/"your" phrasing (edit the line in
webhooks/troubleshooting.mdx).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ce70e69-4f1d-461c-aa5d-dcdc7c51afe2
📒 Files selected for processing (6)
api-reference/endpoint/webhook.mdxwebhooks/events.mdxwebhooks/managing-webhooks.mdxwebhooks/overview.mdxwebhooks/quickstart.mdxwebhooks/troubleshooting.mdx
💤 Files with no reviewable changes (1)
- api-reference/endpoint/webhook.mdx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (1)
**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.mdx: Pages should be written as MDX files with YAML frontmatter
Use active voice and second person ("you") in documentation
Keep sentences concise with one idea per sentence in documentation
Use sentence case for headings in documentation
Bold UI elements in documentation (e.g., Click Settings)
Use code formatting for file names, commands, paths, and code references in documentation
Files:
webhooks/overview.mdxwebhooks/quickstart.mdxwebhooks/managing-webhooks.mdxwebhooks/troubleshooting.mdxwebhooks/events.mdx
🪛 LanguageTool
webhooks/managing-webhooks.mdx
[style] ~57-~57: To form a complete sentence, be sure to include a subject.
Context: ...nd try again | ### URL requirements - Should be https://. We accept http:// URLs...
(MISSING_IT_THERE)
🔇 Additional comments (6)
webhooks/managing-webhooks.mdx (1)
10-14: LGTM!Also applies to: 22-28, 51-51, 65-65, 101-101, 129-129, 139-139, 180-184
webhooks/events.mdx (1)
23-24: LGTM!Also applies to: 27-28, 32-33, 36-37, 86-89, 140-140
webhooks/troubleshooting.mdx (1)
60-61: LGTM!Also applies to: 157-157, 186-186, 188-189, 192-193
webhooks/overview.mdx (1)
3-3: LGTM!Also applies to: 24-24, 50-50
webhooks/quickstart.mdx (2)
12-12: LGTM!Also applies to: 54-54, 147-150, 160-182, 195-195, 204-204
185-192: ⚡ Quick winThe review is based on a misunderstanding of Hono's request body handling. Hono implements automatic body caching, allowing multiple
c.req.text()calls within a single handler to safely return the cached result without re-consuming the stream. The code snippet shown does not require refactoring to avoid "double-reading."> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Pull request overview
This PR updates webhook documentation for newer Spectrum webhook semantics, including project-scoped management endpoints, platform-agnostic wording, and SDK-based reply guidance.
Changes:
- Replaces legacy
/webhooks/management URLs with/projects/$PROJECT_ID/webhooks/. - Generalizes webhook docs beyond iMessage/WhatsApp-only language.
- Updates event examples, idempotency guidance, and removes an obsolete generated API reference page.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
webhooks/troubleshooting.mdx |
Updates troubleshooting examples and platform connection guidance. |
webhooks/quickstart.mdx |
Revises setup, registration, reply, and end-to-end examples. |
webhooks/overview.mdx |
Generalizes webhook overview language to supported platforms. |
webhooks/managing-webhooks.mdx |
Updates webhook management endpoint paths and response/error details. |
webhooks/events.mdx |
Updates payload examples and adds idempotency guidance. |
api-reference/endpoint/webhook.mdx |
Removes obsolete placeholder webhook API reference content. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fb5af04 to
358b842
Compare
Need help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit