[webhooks] add sender/destination fields#167
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 371c4d935d
ℹ️ 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".
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md
Outdated
Show resolved
Hide resolved
Deploying docs with
|
| Latest commit: |
7eacf4c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://75314d79.docs-7wm.pages.dev |
| Branch Preview URL: | https://webhooks-sender-destination.docs-7wm.pages.dev |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds documentation and examples introducing explicit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/features/webhooks.md (1)
175-191:⚠️ Potential issue | 🟡 MinorExample payloads in Step 5 omit
phoneNumber, contradicting the backward-compat guarantee stated elsewhere.Every event tab in Step 5 (lines 175–283) silently drops
phoneNumber, while:
- The Supported Events reference above (lines 12–63) explicitly states
phoneNumber ⚠️is still present in every event.- The blog post's "Expected Payload Structure" includes
phoneNumber.docs/features/status-tracking.mdanddocs/features/mms.mdincludephoneNumberin their examples.This inconsistency could mislead developers into thinking the field was removed, or cause confusion when real payloads include a field that none of the canonical examples show.
The same applies to all six example tabs (sms:received, sms:data-received, mms:received, sms:sent, sms:delivered, sms:failed).
📝 Suggested fix — example for `sms:received` tab (apply analogously to other tabs)
"payload": { "messageId": "abc123", "message": "Android is always a sweet treat!", "sender": "6505551212", "destination": "+1234567890", + "phoneNumber": "6505551212", "simNumber": 1, "receivedAt": "2024-06-22T15:46:11.000+07:00" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/webhooks.md` around lines 175 - 191, The example payloads for the event tabs (e.g., "sms:received", "sms:data-received", "mms:received", "sms:sent", "sms:delivered", "sms:failed") omit the phoneNumber field, conflicting with the Supported Events and other docs; update each example JSON in Step 5 so it includes the phoneNumber property (with a representative value) alongside existing fields (e.g., inside payload for "sms:received" add "phoneNumber": "+1234567890"), ensuring consistency across all six example tabs and matching the "Expected Payload Structure" and examples in status-tracking/mms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md`:
- Around line 303-304: The FAQ line telling users to "replace `phoneNumber` with
`sender`" is incorrect for outbound events; update the sentence to instruct
replacing `phoneNumber` with `destination` for outbound events (`sms:sent`,
`sms:delivered`, `sms:failed`) and add a brief clarifier that `sender` refers to
the device's own number (used for inbound events) while `destination` is the
recipient (used for outbound events), keeping the terms `phoneNumber`, `sender`,
and `destination` consistent with the semantic table earlier in the doc.
- Line 5: Replace the non-standard public-facing author string in the post (the
"author" frontmatter key currently set to "SMSGate Team LLM / StepFun: Step 3.5
Flash") with a standard authors reference and a human-friendly name: change the
frontmatter to use the plural authors property (authors:) and set it to an
identifier listed in .authors.yml (or use a simple value like "smsgate-team"
that maps to an entry in .authors.yml), and in the .authors.yml entry ensure the
displayed name is "SMSGate Team" or a specific human author; in short, replace
the bare author: value with authors: [smsgate-team] and add/update the
corresponding .authors.yml record to show the human-readable name.
In `@docs/faq/webhooks.md`:
- Around line 138-145: The note about `destination` being nullable is misplaced
in the `sms:delivered` multipart delivery section: update the `sms:delivered`
section to state that `destination` is the recipient's phone number and is not
nullable for outbound delivery reports, and either remove the existing
"destination Field May Be null" note or relocate it into the `sms:received`
event docs; alternatively, if you keep a note here, explicitly qualify it (e.g.,
"Note: inbound `sms:received` messages may have `destination` null due to
missing READ_PHONE_STATE or carrier data — this does not apply to outbound
`sms:delivered`").
In `@docs/features/mms.md`:
- Around line 31-33: The example for the mms:received event uses identical
values for the sender and destination fields which hides their semantic
difference; update the JSON so "sender" is an external number (e.g.,
"+15551234567") and "destination" (and "phoneNumber" if it represents the
device) is a different device-owned number (e.g., "+15557654321"), and apply the
same change to the other mms:received example elsewhere; edit the entries
referencing the keys "sender", "destination", and "phoneNumber" to use distinct
values so the distinction is clear.
---
Outside diff comments:
In `@docs/features/webhooks.md`:
- Around line 175-191: The example payloads for the event tabs (e.g.,
"sms:received", "sms:data-received", "mms:received", "sms:sent",
"sms:delivered", "sms:failed") omit the phoneNumber field, conflicting with the
Supported Events and other docs; update each example JSON in Step 5 so it
includes the phoneNumber property (with a representative value) alongside
existing fields (e.g., inside payload for "sms:received" add "phoneNumber":
"+1234567890"), ensuring consistency across all six example tabs and matching
the "Expected Payload Structure" and examples in status-tracking/mms.
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md
Show resolved
Hide resolved
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md
Outdated
Show resolved
Hide resolved
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md (1)
5-5:⚠️ Potential issue | 🟠 MajorAI model attribution still present in the
authorfield.The
author: "SMSGate Team LLM / StepFun: Step 3.5 Flash"frontmatter key exposes AI model provenance publicly and uses a non-standard singular key. Replace with the standardauthors(plural) referencing an entry in.authors.yml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md` at line 5, Replace the singular frontmatter key 'author' with the standard plural 'authors' and remove the AI model provenance string ("SMSGate Team LLM / StepFun: Step 3.5 Flash"); instead reference an existing author entry from .authors.yml by using the 'authors' key (e.g., point to the appropriate .authors.yml identifier), ensuring the public frontmatter only lists approved human/team author IDs and not AI-model attribution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md`:
- Around line 117-119: The "destination" bullet in the "What's New" list
incorrectly says "Device's receiving number" only; update that bullet for the
`destination` field to state that it represents the device's receiving number
for inbound events and the external recipient's number for outbound events
(e.g., `sms:sent`, `sms:delivered`, `sms:failed`), matching the semantic table;
ensure you reference the `destination` field name in the text and keep the
deprecation note for `phoneNumber` unchanged.
In `@docs/faq/webhooks.md`:
- Around line 144-145: The tail of the note is malformed and ambiguous; update
the sentence mentioning `sender` and `destination` to remove the dangling ",
`destination`" and clearly state which field can be null for outbound events.
Specifically, in the paragraph that references inbound events (`sms:received`,
`sms:data-received`, `mms:received`) and the fields `destination` and `sender`,
rewrite the outbound example (e.g., `sms:delivered`) to say something explicit
such as: for outbound events the `sender` field represents the device's own
number and may be null for the same reasons (missing READ_PHONE_STATE or carrier
omission), while `destination` contains the recipient number. Ensure you
reference `destination`, `sender`, and the example event names (`sms:received`,
`sms:delivered`) so the intent is unambiguous.
---
Duplicate comments:
In
`@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md`:
- Line 5: Replace the singular frontmatter key 'author' with the standard plural
'authors' and remove the AI model provenance string ("SMSGate Team LLM /
StepFun: Step 3.5 Flash"); instead reference an existing author entry from
.authors.yml by using the 'authors' key (e.g., point to the appropriate
.authors.yml identifier), ensuring the public frontmatter only lists approved
human/team author IDs and not AI-model attribution.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.mddocs/faq/webhooks.mddocs/features/mms.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/features/mms.md
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md
Outdated
Show resolved
Hide resolved
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md (1)
5-5:⚠️ Potential issue | 🟠 MajorRemove AI attribution and use standard author format.
The author field still publicly exposes AI generation ("SMSGate Team LLM / StepFun: Step 3.5 Flash"), which can undermine trust in official documentation. Additionally, Material for MkDocs blog plugin expects
authors:(plural) with references to.authors.yml, not a bare string.📝 Suggested fix
-author: "SMSGate Team LLM / StepFun: Step 3.5 Flash" +authors: + - capcom6🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md` at line 5, Replace the exposed AI attribution string "SMSGate Team LLM / StepFun: Step 3.5 Flash" with a standard authors reference expected by the MkDocs blog plugin: change the frontmatter key from author: to authors: and set its value to a list containing the appropriate author ID defined in .authors.yml (e.g., authors: - smsgate-team), removing any "LLM" or step-fun procedural text so only the canonical author key remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md`:
- Line 5: Replace the exposed AI attribution string "SMSGate Team LLM / StepFun:
Step 3.5 Flash" with a standard authors reference expected by the MkDocs blog
plugin: change the frontmatter key from author: to authors: and set its value to
a list containing the appropriate author ID defined in .authors.yml (e.g.,
authors: - smsgate-team), removing any "LLM" or step-fun procedural text so only
the canonical author key remains.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.mddocs/faq/webhooks.md
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md (1)
5-5:⚠️ Potential issue | 🟠 MajorRemove AI model attribution from the public-facing author field.
Publishing
"SMSGate Team LLM / StepFun: Step 3.5 Flash"as the author explicitly discloses AI generation, which can undermine trust in official documentation. Additionally, the Material for MkDocs blog plugin expects anauthors:(plural) property that references author identifiers defined in.authors.yml, not a bareauthor:string.📝 Suggested fix
-author: "SMSGate Team LLM / StepFun: Step 3.5 Flash" +authors: + - capcom6Alternatively, if a team attribution is preferred:
-author: "SMSGate Team LLM / StepFun: Step 3.5 Flash" +authors: + - smsgate-teamEnsure the corresponding entry exists in
.authors.ymlwith a human-readable name like "SMSGate Team" or "capcom6".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md` at line 5, Remove the public-facing AI attribution by replacing the single-line author: "SMSGate Team LLM / StepFun: Step 3.5 Flash" with an authors: property that references an author identifier (e.g. authors: ["smsgate-team"]) and ensure a matching entry exists in .authors.yml with a human-readable name like "SMSGate Team" or "capcom6"; specifically, edit the author field (string) in the blog post to use authors: [...], remove any "LLM" or model-specific text, and add/update the corresponding identifier in .authors.yml so the MkDocs Material authors plugin can resolve it.
🧹 Nitpick comments (1)
docs/faq/webhooks.md (1)
144-145: Consider relocating the nullability note to a more general section.The note about
recipientandsendernullability is now grammatically correct and covers both inbound and outbound scenarios. However, it's placed within the "Why do I receive multiple delivery reports?" section, which specifically discusses outboundsms:deliveredevents.Since the note primarily explains inbound behavior (
recipientcan be null for inbound events) and only briefly mentions outbound, consider moving this note to a more general "Webhook Concepts" or "Field Semantics" section where it would provide better context for readers learning about webhook payloads across all event types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/faq/webhooks.md` around lines 144 - 145, The nullability note about `recipient` and `sender` is placed under the "Why do I receive multiple delivery reports?" section but belongs in a global webhook field reference; move this paragraph (the note starting with "`recipient` May Be `null` for Inbound Events") out of that delivery-reports section and place it under a general section such as "Webhook Concepts" or "Field Semantics" (or the global webhook payload / field reference page) so it applies to all event types; ensure the note still mentions both inbound events (`sms:received`, `sms:data-received`, `mms:received`) and outbound behavior (`sms:delivered`) and retain the exact wording about `READ_PHONE_STATE` permission and carrier-provided numbers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.md`:
- Line 5: Remove the public-facing AI attribution by replacing the single-line
author: "SMSGate Team LLM / StepFun: Step 3.5 Flash" with an authors: property
that references an author identifier (e.g. authors: ["smsgate-team"]) and ensure
a matching entry exists in .authors.yml with a human-readable name like "SMSGate
Team" or "capcom6"; specifically, edit the author field (string) in the blog
post to use authors: [...], remove any "LLM" or model-specific text, and
add/update the corresponding identifier in .authors.yml so the MkDocs Material
authors plugin can resolve it.
---
Nitpick comments:
In `@docs/faq/webhooks.md`:
- Around line 144-145: The nullability note about `recipient` and `sender` is
placed under the "Why do I receive multiple delivery reports?" section but
belongs in a global webhook field reference; move this paragraph (the note
starting with "`recipient` May Be `null` for Inbound Events") out of that
delivery-reports section and place it under a general section such as "Webhook
Concepts" or "Field Semantics" (or the global webhook payload / field reference
page) so it applies to all event types; ensure the note still mentions both
inbound events (`sms:received`, `sms:data-received`, `mms:received`) and
outbound behavior (`sms:delivered`) and retain the exact wording about
`READ_PHONE_STATE` permission and carrier-provided numbers.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/blog/posts/2026-02-17_webhook-payload-enhancement-sender-destination.mddocs/faq/webhooks.mddocs/features/mms.mddocs/features/status-tracking.mddocs/features/webhooks.mddocs/getting-started/webhooks.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/features/mms.md
- docs/features/webhooks.md
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/features/webhooks.md (1)
222-223: Clarify the MMS example with distinct phone numbers.The
mms:receivedexample shows identical phone numbers for bothsenderandrecipient("+1234567890"). While technically possible for a device to send an MMS to itself, this is not representative of typical usage and may confuse readers about the semantic distinction between the fields.💡 Suggested improvement
- "sender": "+1234567890", - "recipient": "+1234567890", + "sender": "+9876543210", + "recipient": "+1234567890",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/webhooks.md` around lines 222 - 223, The mms:received example uses identical phone numbers which is confusing; update the example payload for the "mms:received" event so the "sender" and "recipient" fields use distinct realistic phone numbers (e.g., different country-code numbers) to clearly show who sent the MMS and who received it, keeping the keys "sender" and "recipient" unchanged and only altering their values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/webhooks.md`:
- Line 41: The outbound event docs incorrectly state that `sender` replaces
deprecated `phoneNumber`; instead update the descriptions for `sender` in the
`sms:sent`, `sms:delivered`, and `sms:failed` sections to remove the deprecation
reference to `phoneNumber` and instead mark `recipient` as the field that
replaces the deprecated `phoneNumber`. Specifically, change the `sender` bullet
(currently "Device's phone number (use instead of deprecated `phoneNumber`)") to
simply "Device's phone number" and ensure the `recipient` bullet references
deprecated `phoneNumber` as the replacement for prior behavior; apply the same
fix for the three occurrences noted.
---
Nitpick comments:
In `@docs/features/webhooks.md`:
- Around line 222-223: The mms:received example uses identical phone numbers
which is confusing; update the example payload for the "mms:received" event so
the "sender" and "recipient" fields use distinct realistic phone numbers (e.g.,
different country-code numbers) to clearly show who sent the MMS and who
received it, keeping the keys "sender" and "recipient" unchanged and only
altering their values.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/faq/webhooks.mddocs/features/webhooks.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/faq/webhooks.md
c9df59e to
ffe3779
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
ffe3779 to
7eacf4c
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary by CodeRabbit