fix: prevent duplicate webhook tasks for same PR branch#1156
Open
j-bennet wants to merge 1 commit into
Open
Conversation
* fix: prevent duplicate webhook tasks for same PR branch The webhook handler generated task names using sha256(deliveryID), so multiple GitHub events for the same PR (labeled, ready_for_review, etc.) each created a separate task. Add branch-based deduplication that skips task creation when an active task already exists for the same spawner and branch. Also make task names human-readable by including the repo name and PR/issue number (e.g., babysitter-dquality-pr-30170-a1b2c3d4). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: sanitize repo name in task naming and fall through on dedup error Address review feedback: - Lowercase and replace underscores in RepositoryName before embedding in K8s task names to comply with DNS label requirements - On dedup check failure, log the error but proceed with task creation rather than silently dropping the webhook event Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: sanitize Generic and Linear IDs for K8s DNS label compliance User-supplied IDs from generic webhook field mappings and Linear events can contain uppercase, dots, slashes, and other chars invalid in K8s resource names. Extract a sanitizeK8sNameSegment helper that lowercases and strips non-[a-z0-9-] characters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use sanitizeK8sNameSegment consistently in all task name paths Apply the shared sanitizer to GitHub repo names (handles dots) and Generic eventType (handles uppercase/underscores in URL path segments). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
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
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.
/kind bug
What this PR does / why we need it:
The webhook handler generated task names using
sha256(deliveryID), so multiple GitHub events for the same PR (labeled, ready_for_review, etc.) each created a separate task. This adds branch-based deduplication that skips task creation when an active task already exists for the same spawner and branch. Also makes task names human-readable by including the repo name and PR/issue number (e.g.,babysitter-dquality-pr-30170-a1b2c3d4).Which issue(s) this PR is related to:
Fixes #1155
Special notes for your reviewer:
buildWebhookTaskNameconstructs human-readable names with repo/PR number and a short delivery hash for uniquenesssanitizeK8sNameSegmentensures all name segments comply with DNS label requirementsDoes this PR introduce a user-facing change?
Summary by cubic
Prevents duplicate webhook tasks for the same PR branch by adding branch-based deduplication in the
internal/webhookhandler. Also switches to human-readable, DNS-safe task names that include repo and PR/issue number. Fixes #1155.<spawner>-<repo>-<pr|issue>-<number>-<hash>(fallback:<spawner>-<event>-<hash>), with all segments sanitized for Kubernetes DNS rules.Written for commit ce77926. Summary will update on new commits. Review in cubic