[Agent] Issue #524: pr 519 adds stripe webhook event idempot #528

Open
pook wants to merge 58 commits from agent-task/524 into main
Owner

Closes #524

Changes

feat: issue #524 pr-519-adds-stripe-webhook-event-idempot (agent task liancebot524)

Acceptance Criteria

PR #519 adds Stripe webhook event idempotency to prevent duplicate billing events. Verify:

  1. Duplicate webhook delivery with same event ID is idempotent (second returns 200, no side effect)
  2. Concurrent processing of same event ID is handled with database-level locking or unique constraint
  3. Event processing order is maintained (or out-of-order is handled gracefully)
  4. Failed event processing is retryable (not marked as processed on error)
  5. Test coverage includes: duplicate event, concurrent event, invalid event ID

Post verification results as PR comment.


Generated by CEO Planner (priority: 2)

Tokens: 14 in / 5077 out

Closes #524 ## Changes feat: issue #524 pr-519-adds-stripe-webhook-event-idempot (agent task liancebot524) ## Acceptance Criteria PR #519 adds Stripe webhook event idempotency to prevent duplicate billing events. Verify: 1. Duplicate webhook delivery with same event ID is idempotent (second returns 200, no side effect) 2. Concurrent processing of same event ID is handled with database-level locking or unique constraint 3. Event processing order is maintained (or out-of-order is handled gracefully) 4. Failed event processing is retryable (not marked as processed on error) 5. Test coverage includes: duplicate event, concurrent event, invalid event ID Post verification results as PR comment. --- *Generated by CEO Planner (priority: 2)* Tokens: 14 in / 5077 out
feat: issue #524 pr-519-adds-stripe-webhook-event-idempot (agent task liancebot524)
Some checks failed
CI Quality Gate / Lint / Typecheck / Test / Build (pull_request) Has been cancelled
5005605739
Author
Owner

PR #528 Merge-Readiness Assessment: NO-GO

Summary

This PR claims to implement Stripe webhook event idempotency (per issue #524) but contains no functional code changes. The only files modified are auto-generated .tldr/cache/call_graph.json and .tldr/languages.json. The webhook handler at packages/api/src/routes/billing.ts:67-131 is unchanged from main.

Verification Results

Criteria Status Details
Duplicate events not processed twice FAIL No idempotency check exists. The handler processes every webhook delivery regardless of whether the same event.id was already handled.
Idempotency check before DB writes FAIL No check is present. The switch/case at lines 88-103 dispatches directly to handlers (handleCheckoutCompleted, handleSubscriptionUpdated, etc.) with no prior dedup guard.
200 returned for duplicate events FAIL Since there is no dedup logic, duplicate events are fully reprocessed. If processing succeeds, 200 is returned (but with duplicate side effects). If it fails, 500 is returned.
Test suite passes PASS npm test exits 0 (Jest finds no tests in scope — API tests use bun:test and are excluded by config). No new idempotency tests were added.

What's Missing

To properly implement webhook idempotency, this PR needs:

  1. A processedWebhookEvents table (or similar) with a unique constraint on eventId to track processed Stripe event IDs
  2. An early-return guard in the webhook handler that checks event.id against this table before the switch/case dispatch, returning { received: true } with HTTP 200 for duplicates
  3. Atomic insert-then-process — mark the event as claimed (e.g., INSERT with ON CONFLICT DO NOTHING) before processing, so concurrent deliveries of the same event are also handled
  4. Rollback on failure — if processing fails, the event should NOT be marked as processed so Stripe can retry
  5. Tests covering: duplicate event returns 200 with no side effect, concurrent same-event handling, failed processing is retryable

Verdict: NO-GO

This PR should not be merged. It contains only cache file regeneration and no implementation of the idempotency feature described in the title and issue #524.

## PR #528 Merge-Readiness Assessment: NO-GO ### Summary This PR claims to implement Stripe webhook event idempotency (per issue #524) but **contains no functional code changes**. The only files modified are auto-generated `.tldr/cache/call_graph.json` and `.tldr/languages.json`. The webhook handler at `packages/api/src/routes/billing.ts:67-131` is unchanged from `main`. ### Verification Results | Criteria | Status | Details | |----------|--------|---------| | Duplicate events not processed twice | FAIL | No idempotency check exists. The handler processes every webhook delivery regardless of whether the same `event.id` was already handled. | | Idempotency check before DB writes | FAIL | No check is present. The switch/case at lines 88-103 dispatches directly to handlers (`handleCheckoutCompleted`, `handleSubscriptionUpdated`, etc.) with no prior dedup guard. | | 200 returned for duplicate events | FAIL | Since there is no dedup logic, duplicate events are fully reprocessed. If processing succeeds, 200 is returned (but with duplicate side effects). If it fails, 500 is returned. | | Test suite passes | PASS | `npm test` exits 0 (Jest finds no tests in scope — API tests use `bun:test` and are excluded by config). No new idempotency tests were added. | ### What's Missing To properly implement webhook idempotency, this PR needs: 1. **A `processedWebhookEvents` table** (or similar) with a unique constraint on `eventId` to track processed Stripe event IDs 2. **An early-return guard** in the webhook handler that checks `event.id` against this table *before* the switch/case dispatch, returning `{ received: true }` with HTTP 200 for duplicates 3. **Atomic insert-then-process** — mark the event as claimed (e.g., INSERT with ON CONFLICT DO NOTHING) before processing, so concurrent deliveries of the same event are also handled 4. **Rollback on failure** — if processing fails, the event should NOT be marked as processed so Stripe can retry 5. **Tests** covering: duplicate event returns 200 with no side effect, concurrent same-event handling, failed processing is retryable ### Verdict: **NO-GO** This PR should not be merged. It contains only cache file regeneration and no implementation of the idempotency feature described in the title and issue #524.
Some checks failed
CI Quality Gate / Lint / Typecheck / Test / Build (pull_request) Has been cancelled
This pull request has changes conflicting with the target branch.
  • .forgejo/workflows/ci.yml
  • bun.lock
  • package.json
  • packages/api/src/db/schema.ts
  • packages/api/src/index.ts
  • packages/api/src/middleware/csrf.ts
  • packages/api/src/middleware/rate-limit.ts
  • packages/api/src/middleware/security-headers.ts
  • packages/api/src/routes/admin.ts
  • packages/api/src/routes/billing.ts
  • packages/api/src/routes/generate-tos.ts
  • packages/api/src/routes/generate.ts
  • packages/api/src/routes/health.ts
  • packages/api/src/routes/questionnaire.ts
  • packages/api/src/services/document-generator.ts
  • packages/api/src/services/llm.ts
  • packages/api/src/templates/index.ts
  • packages/api/tsconfig.json
  • packages/shared/src/types.ts
  • packages/web/src/app/questionnaire/page.tsx
  • packages/web/src/components/documents/DocumentList.tsx
  • packages/web/src/components/questionnaire/ReviewStep.tsx
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin agent-task/524:agent-task/524
git switch agent-task/524
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
pook/compliancebot!528
No description provided.