Skip to content

Add idempotency to the GitHub webhook consumer using X-GitHub-Delivery de-duplication #49

Description

@Jagadeeshftw

📌 Description

internal/worker/github_webhook_consumer.go subscribes with a queue group and processes each message, but there is no idempotency: if a webhook event is redelivered (NATS redelivery or GitHub retrying), it is processed again. Downstream upserts mostly tolerate this, but actions like posting bot comments or enqueueing sync jobs can duplicate. GitHub provides a unique X-GitHub-Delivery ID that is currently unused for de-duplication.

💡 Why it matters: Redelivered webhooks can cause duplicate side effects (duplicate jobs/comments); a delivery-id de-dup makes processing exactly-once-ish and safe to retry.

🧩 Requirements and context

  • Persist processed X-GitHub-Delivery IDs (e.g. a processed_deliveries table with a TTL/cleanup) and skip already-seen IDs.
  • Ensure the delivery ID is captured at internal/handlers/github_webhooks.go and carried through the event payload to the consumer.
  • Make the check atomic to avoid races between concurrent workers.
  • Add a migration for the dedup table with a .down.sql and a retention cleanup.
  • Add tests proving a duplicate delivery is processed only once.

Non-functional requirements

  • Must be secure, tested, and documented.
  • Should be efficient and easy to review.

🛠️ Suggested execution

1. Fork the repo and create a branch

git checkout -b feat/webhook-idempotency

2. Implement changes

  • Write/modify the relevant source: internal/worker/github_webhook_consumer.go, internal/handlers/github_webhooks.go, internal/events/types.go, new migration
  • Write comprehensive tests: internal/worker/github_webhook_consumer_test.go
  • Add documentation: PATCHWORK_BACKEND_ARCHITECTURE.md
  • Include GoDoc comments on the dedup helper
  • Validate security assumptions: dedup must not let a forged delivery suppress a real one

3. Test and commit

  • Run tests:
go test ./internal/worker/... ./internal/handlers/...
  • Cover edge cases: concurrent duplicate, expired record re-processed intentionally
  • Include test output and security notes in the PR description.

Example commit message

feat(worker): de-duplicate webhook processing by delivery id

✅ Acceptance criteria

  • Delivery ID captured and propagated to the consumer
  • Duplicate deliveries are skipped atomically
  • Dedup table migration with down + retention cleanup
  • Tests prove single-processing under duplicates

🔒 Security notes

Delivery IDs are attacker-controllable; pair de-dup with the existing signature verification so a forged ID cannot poison or replay state.

📋 Guidelines

  • Minimum 95% test coverage
  • Clear documentation
  • Timeframe: 96 hours

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionOfficial CampaignGrantFox official campaign issuearchitecturebackendBackend / API worksecuritySecurity hardening / audit
    No fields configured for Feature.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions