Stream and process CLI listen webhooks#188
Merged
Merged
Conversation
Store webhook deliveries in D1 before returning success, then use a Durable Object websocket to push pending deliveries to paykitjs listen. Adds sent_at delivery tracking for in-flight websocket deliveries.
Default listen delivery to PayKit handleWebhook instead of forwarding over localhost HTTP. Keep --forward-to as an escape hatch. Use async Stripe webhook verification for Bun direct mode and tune CLI pretty logs.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements an end-to-end WebSocket-based tunnel for delivering webhooks from a Cloudflare Worker to a CLI client, replacing polling with persistent connections. It adds database ChangesWebSocket Tunnel Delivery System
Sequence DiagramsequenceDiagram
participant CLI as CLI Client
participant Worker as Webhook Worker
participant DO as TunnelObject DO
participant DB as D1 Database
participant Webhook as Webhook Source
CLI->>Worker: WebSocket upgrade /api/tunnels/:id/connect + auth
Worker->>DB: Verify tunnel & device token
Worker->>DO: Forward WebSocket
DO->>DB: Count pending deliveries
DO->>CLI: hello message + pending count
Webhook->>Worker: POST /provider-webhook
Worker->>DB: Store delivery, prune old
Worker->>DO: POST /internal/push
DO->>DB: Mark delivery sentAt=now()
DO->>CLI: delivery payload
CLI->>DO: ack message
DO->>DB: Set deliveredAt, clear sentAt
DO->>DB: Query next eligible delivery
DO->>CLI: delivery payload
CLI->>DO: fail message with error
DO->>DB: Set failedAt, clear sentAt
CLI->>Worker: [reconnect]
Worker->>DB: Reset sentAt for tunnel
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/paykit/src/core/logger.ts (1)
18-33: ⚡ Quick winAdd brief JSDoc to new core helper functions.
dimDetailLines()andcreatePrettyStream()are new library-core helpers; add short JSDoc so intent stays discoverable and consistent with repo standards.Suggested patch
const DETAIL_LINE_PATTERN = /(^|\n)(\s+[^\n]+)/g; +/** Dim indented detail lines in pretty log output. */ function dimDetailLines(input: string): string { return input.replace(DETAIL_LINE_PATTERN, (_match, prefix: string, line: string) => { return `${prefix}${dim(line)}`; }); } +/** Create a pretty logger stream that dims detail lines before writing to stdout. */ function createPrettyStream() { const output = new Transform({ transform(chunk, _encoding, callback) { callback(null, dimDetailLines(String(chunk))); },As per coding guidelines, "Add JSDoc on most functions in the library core and on some object properties, mainly if the API is used in many places or is user-facing".
🤖 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 `@packages/paykit/src/core/logger.ts` around lines 18 - 33, Add concise JSDoc comments for the two new helpers so their intent and signature are discoverable: place a short description, `@param` and `@returns` on dimDetailLines(input: string) explaining it finds detail lines using DETAIL_LINE_PATTERN and returns the input with those lines dimmed (string -> string); and add a brief JSDoc on createPrettyStream() describing it creates a Transform stream that pipes dimmed log output to process.stdout and returns the configured pretty logger/stream. Ensure the JSDoc style matches existing repo conventions (one-line summary plus param/return tags) and attach them directly above the dimDetailLines and createPrettyStream declarations.packages/paykit/src/cli/commands/listen.ts (1)
609-611: ⚡ Quick winUse a protocol-level replacement signal instead of a reason regex.
This stop path depends on the worker keeping the exact phrase
"replaced by a newer session". Any wording drift turns an intentional session replacement into the generic reconnect loop below. A shared app-specific close code or explicit control message would make this contract much safer.Also applies to: 722-727
🤖 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 `@packages/paykit/src/cli/commands/listen.ts` around lines 609 - 611, The current isReplacedSessionClose function relies on matching the human-readable close.reason string, which is fragile; instead define and use a protocol-level replacement signal (e.g., a dedicated close code constant like REPLACED_SESSION_CODE or a structured reason payload token) and update isReplacedSessionClose to check that constant (or parse the structured payload) first; also replace any other regex-based checks of close.reason elsewhere (the similar check currently duplicated later in the file) to use the same constant/structured token so the client and worker share a deterministic contract.
🤖 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 `@apps/wh/src/tunnel-object.ts`:
- Around line 162-169: The close handler webSocketClose currently calls
resetInFlightDeliveries unconditionally using the attachment.tunnelId, which
lets an old socket close clear in-flight deliveries for a newly-created session;
change webSocketClose to first read the attachment (readSocketAttachment) and
then verify that the attachment's session identifier (e.g., attachment.sessionId
or attachment.socketId) still matches the active session/socket stored for that
tunnel before calling resetInFlightDeliveries(attachment.tunnelId); if they
differ, return without resetting. Apply the same session-match guard to the
other close/error handler that calls resetInFlightDeliveries so only the current
active socket can clear in-flight deliveries.
- Around line 316-327: The update that claims a delivery by setting sentAt
currently ignores the DB result so concurrent callers can still send the same
payload; change the claim to inspect the update result (e.g., const res = await
this.db.update(delivery).set({ sentAt: now() }).where(...)) and only proceed
with sending if the update affected exactly one row
(res.rowCount/res.affectedRows/res.numUpdated === 1), otherwise abort/skip;
apply the same pattern to the analogous update that sets deliveredAt (the other
update block using nextDelivery, attachment, delivery, sentAt/deliveredAt) so
only the successful claimer sends/marks the delivery.
In `@packages/paykit/src/cli/commands/listen.ts`:
- Around line 898-901: normalizeLocalOrigin() still throws an error message that
references the old flag (--url) instead of the renamed flag (--forward-to);
update the validation/error text inside normalizeLocalOrigin (and the similar
validation code around the second occurrence at the other block) to reference
"--forward-to" (and/or both flags if you want backward clarity) so users see the
correct flag name when a non-origin value is passed.
- Around line 572-582: deliverWebhook currently awaits replayDelivery without a
bound, which can hang the socket consumer; when params.forwardTo is set, wrap
the replayDelivery call in a bounded timeout (e.g., Promise.race or
AbortController if replayDelivery supports an AbortSignal) so the call fails
after a configurable short timeout (e.g., a few seconds). On timeout return a
ReplayResult indicating failure (consistent shape used elsewhere) and include
context (e.g., reason "forward-to timeout" and the localWebhookUrl) so the
caller can ack/fail the delivery; modify deliverWebhook to use this timed call
path while leaving applyDeliveryDirectly untouched.
---
Nitpick comments:
In `@packages/paykit/src/cli/commands/listen.ts`:
- Around line 609-611: The current isReplacedSessionClose function relies on
matching the human-readable close.reason string, which is fragile; instead
define and use a protocol-level replacement signal (e.g., a dedicated close code
constant like REPLACED_SESSION_CODE or a structured reason payload token) and
update isReplacedSessionClose to check that constant (or parse the structured
payload) first; also replace any other regex-based checks of close.reason
elsewhere (the similar check currently duplicated later in the file) to use the
same constant/structured token so the client and worker share a deterministic
contract.
In `@packages/paykit/src/core/logger.ts`:
- Around line 18-33: Add concise JSDoc comments for the two new helpers so their
intent and signature are discoverable: place a short description, `@param` and
`@returns` on dimDetailLines(input: string) explaining it finds detail lines using
DETAIL_LINE_PATTERN and returns the input with those lines dimmed (string ->
string); and add a brief JSDoc on createPrettyStream() describing it creates a
Transform stream that pipes dimmed log output to process.stdout and returns the
configured pretty logger/stream. Ensure the JSDoc style matches existing repo
conventions (one-line summary plus param/return tags) and attach them directly
above the dimDetailLines and createPrettyStream declarations.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b2117921-6cbe-428d-9c63-a9de89138e62
📒 Files selected for processing (14)
apps/wh/migrations/0002_delivery-sent-tracking.sqlapps/wh/migrations/meta/0002_snapshot.jsonapps/wh/migrations/meta/_journal.jsonapps/wh/package.jsonapps/wh/src/db/schema.tsapps/wh/src/index.tsapps/wh/src/tunnel-object.tsapps/wh/wrangler.jsoncpackages/paykit/src/cli/commands/listen.tspackages/paykit/src/cli/utils/get-config.tspackages/paykit/src/core/__tests__/logger.test.tspackages/paykit/src/core/logger.tspackages/paykit/src/webhook/webhook.service.tspackages/stripe/src/stripe-provider.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
paykitjs listento direct PayKit webhook handling, with--forward-toas an optional localhost fallbackTesting
bun --filter wh typecheckbun --filter paykitjs typecheckbun --filter @paykitjs/stripe typecheckbun lintnode ./node_modules/vitest/vitest.mjs run --config vitest.unit.config.ts packages/paykit/src/core/__tests__/logger.test.tsSummary by CodeRabbit
New Features
--forward-tooption to forward webhooks to a local originBug Fixes