Repository navigation
Add --phone-offline-start-day to webhook create and update - #82
Conversation
Sets the first day (7-13) a webhook receives session.phone_offline, or off to remove the event; update reads saved event types when the input has none and refuses off when no event would remain. webhook view shows the setting. API contract: polymorfa/polymorfa#631.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedThe included review limit has been reached and this organization has disabled usage-based review continuation. Wait for reviews to reset or ask a billing admin to change After included review limits.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Next included review available in 38 minutes. View limit details
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50b95def09
ℹ️ 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".
| if (eventTypes === undefined) { | ||
| throw new Error('Add eventTypes to the input before using off.') | ||
| } | ||
| const remaining = eventTypes.filter((event) => event !== PHONE_OFFLINE_EVENT) |
There was a problem hiding this comment.
When a webhook's eventTypes contains the valid trailing-wildcard pattern session.*, filtering only the exact session.phone_offline string leaves that subscription unchanged, so --phone-offline-start-day off succeeds while the webhook continues receiving the warning event. Either rewrite wildcard subscriptions so they exclude this event or reject off with an actionable error when exclusion cannot be represented.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed. Delivery matches subscriptions by exact event type (matchesSubscription uses events.includes(event.type), apps/api/packages/api/src/services/webhook-delivery.ts:600), so a stored session.* entry matches no event and the webhook does not receive session.phone_offline through it. Filtering the exact name is the complete opt-out.
| readonly eventTypes: readonly string[] | ||
| readonly phoneOfflineStartDay?: number | ||
| }): string { | ||
| if (!webhook.eventTypes.includes('session.phone_offline')) return 'off' |
There was a problem hiding this comment.
For a webhook subscribed through session.*, this exact-membership check displays Phone offline warning: off even though the wildcard includes session.phone_offline. The renderer should apply the same trailing-wildcard matching semantics as event subscription handling before deciding that the warning is disabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed, for the same reason: delivery has no wildcard matching, so a webhook subscribed only through session.* does not receive session.phone_offline and 'off' is the accurate display.
Summary
Adds
--phone-offline-start-day <7-13|off>topolymorfa webhook createandpolymorfa webhook update(API contract polymorfa/polymorfa#631).phoneOfflineStartDayinto the JSON body. The API default is 10.offremovessession.phone_offlinefromeventTypes. Onupdate, if--inputsupplies no event types, the CLI reads the saved ones withwebhooks.retrievefirst. Platform webhooks need at least one event type, so the CLI refusesoffwhen no event would remain, with exit code 2 and no request.webhook updateaccepts the flag without--input. With neither, it exits 2 before any request.webhook viewshows Phone offline warning:from day N,offwhen unsubscribed, ornot reportedwhen the API doesn't return the field.Impact
Impact table
webhook createandwebhook update;--inputoptional on update with the flagsrc/commands/webhook/create.ts,update.ts,src/developer/phone-offline.tswebhook viewrowsrc/developer/render.tsREADME.md@polymorfa/sdkto the first prerelease with the typed field, then drop the optional castNo impact table
create.tssecret handling unchanged; existing tests passgit diff --statdeveloperMutationanddeveloperResponseenvelopesVerification
Tested at
50b95de.npm run verifypasses: prettier, eslint, typecheck, vitest (110 files, 1,051 tests), build,npm pack --dry-run, andnpm auditwith 0 vulnerabilities.--input;offremoving the event from the saved list viawebhooks.retrieve; refusingoffwhen it would leave no event (exit 2, no update call); rejecting 6, 14, 9.5 andneverbefore any SDK call; requiring--inputor the flag (exit 2); and thewebhook viewrow for subscribed, unsubscribed and unreported cases.Session provenance and UI evidence
purpshell-t3-clauderegistration in polymorfa/polymorfa.pnpm agent:record capturefails for T3 threadcdeecc57-bc5b-483f-a725-030e5bf3dc69(known T3 v2 gap).50b95de; claude-opus-5-5.Rollout and rollback
Need help on this PR? Tag
@codesmithwith what you need.