Skip to content

fix: prevent duplicate benchmark PRs on stable releases#304

Merged
carlos-alm merged 1 commit intomainfrom
fix/benchmark-duplicate-prs
Mar 3, 2026
Merged

fix: prevent duplicate benchmark PRs on stable releases#304
carlos-alm merged 1 commit intomainfrom
fix/benchmark-duplicate-prs

Conversation

@carlos-alm
Copy link
Contributor

Summary

  • Filter benchmark workflow_run triggers to exclude push-initiated Publish completions — dev builds should not benchmark the latest stable tag
  • Add duplicate-PR guard before gh pr create in all 4 benchmark jobs as a safety net

Root cause

During the 3.0.0 release, two Publish workflow runs completed successfully within minutes of each other:

  1. A push event (dev build from merging a PR to main) at 10:05
  2. The release event (actual v3.0.0 publish) at 10:07

Both triggered separate Benchmark workflow runs. Each resolved to version 3.0.0 via git tag --sort=-version:refname, creating duplicate PRs (#298-#300 and #301-#303).

Test plan

  • Verify next stable release creates only one set of benchmark PRs
  • Verify dev pushes to main no longer trigger benchmark runs
  • Verify workflow_dispatch benchmarks still work as expected

Closes duplicate PRs: #301, #302, #303 (already closed)

The Benchmark workflow triggers on any Publish workflow completion.
During a stable release, both a dev-push Publish (from merging PRs)
and the release-event Publish complete successfully, each spawning
a Benchmark run that resolves to the same stable version via git tag
lookup — creating duplicate PRs.

Two fixes:
1. Filter workflow_run triggers to exclude push-initiated Publish
   completions (dev builds should not benchmark the latest stable tag)
2. Check for existing open PRs before creating new ones as a safety net
@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


PR Review: Fix Duplicate Benchmark PRs

Todo List:

  • Read and analyze the modified benchmark.yml workflow file
  • Phase 1: Root Cause Analysis
  • Phase 2: Configuration Change Detection
  • Phase 3: Test Integrity Assessment
  • Phase 4: Code Quality Review
  • Provide final rating and recommendation

Phase 1: Root Cause Analysis

Problem Being Solved: During the v3.0.0 release, two separate Publish workflow runs completed successfully within 2 minutes (push event at 10:05, release event at 10:07). Both triggered the Benchmark workflow, which resolved to the same version 3.0.0 via git tag --sort=-version:refname, creating duplicate PRs.

Root Cause Identified: The workflow_run trigger was too broad - it triggered on ALL Publish workflow completions regardless of the triggering event type. This meant both development pushes to main AND actual releases would trigger benchmarks.

Why This Approach: The PR adds two defensive measures:

  1. Primary fix: Filter workflow_run triggers to exclude push-initiated runs (github.event.workflow_run.event != 'push') - lines 20-21, 158-159, 309-310, 446-447
  2. Secondary safety net: Check for existing PRs before creating new ones (lines 144-146, 295-297, 432-434, 569-571)

This is a proper root cause fix, not symptom hiding. The logic is sound: only releases should trigger benchmarks, not development pushes.

Phase 2: Configuration Change Detection

No High-Risk Changes Detected:

  • ✅ No linting rules disabled or relaxed
  • ✅ No test timeouts increased or coverage thresholds lowered
  • ✅ No build configuration changes
  • ✅ No TypeScript strict mode modifications
  • ✅ This is purely workflow logic enhancement, not configuration relaxation

The changes are additive safety measures, not rule relaxation.

Phase 3: Test Integrity Assessment

No Test Integrity Issues:

  • ✅ No test files modified
  • ✅ No assertions weakened or removed
  • ✅ No test skipping or conditional validation added
  • ✅ No eslint-disable or workaround comments

This is a pure workflow change with no impact on test integrity.

Phase 4: Code Quality Review

Strengths:

  1. Defense in depth: Both filtering at trigger level AND duplicate PR prevention
  2. Consistent implementation: Same fix applied to all 4 benchmark jobs
  3. Proper GitHub Actions syntax: Correct use of multiline if conditions
  4. Preserved functionality: workflow_dispatch triggers remain unaffected

Technical Analysis:

  • Event filtering logic is sound: github.event.workflow_run.event != 'push' correctly excludes dev builds
  • PR duplicate detection works: Uses gh pr list --json title --jq ".[].title" | grep -qF "$TITLE" for exact title matching
  • No race conditions introduced: The duplicate check happens immediately before gh pr create

Minor Observation:

  • All 4 jobs have identical trigger conditions and duplicate PR logic - this could be DRY'd up with a reusable workflow, but that's not a blocker for this fix.

Security: No security concerns - no new credentials, proper use of existing GITHUB_TOKEN.

Final Assessment

## Root Cause Analysis
**Problem Being Solved:** Duplicate benchmark PRs triggered by both push and release events during v3.0.0 release
**Why This Approach:** Proper root cause fix with defense in depth - filters undesired triggers plus safety net
**Risk Assessment:** Low risk - additive safety measures with no configuration relaxation

## Critical Concerns
None identified. This is a well-implemented fix addressing a real operational issue.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)  
- Action: APPROVE
- Reasoning: Solid root cause analysis, proper implementation, consistent across all jobs, no risky changes. Minor deduction for potential DRY opportunity, but that's not a blocking concern.

@greptile-apps
Copy link

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR prevents duplicate benchmark PRs by implementing a two-layer defense: filtering out push-initiated Publish workflow runs and adding duplicate PR checks before PR creation.

Changes:

  • Modified if conditions in all 4 benchmark jobs to exclude workflow_run events triggered by push (only allow release-triggered runs and manual workflow_dispatch)
  • Added duplicate PR guard using gh pr list to check for existing open PRs with the same title before calling gh pr create
  • Each guard uses grep -qF for exact title matching and skips PR creation with a notice if a duplicate exists

Root cause addressed:
During the v3.0.0 release, both a push-event Publish run (dev build) and a release-event Publish run (stable release) completed successfully within 2 minutes. Both triggered separate Benchmark workflows that resolved to the same version 3.0.0 via git tag --sort=-version:refname, creating duplicate PRs for all 4 benchmark types.

The primary fix (event filtering) prevents push-initiated dev builds from triggering benchmarks since they should only benchmark released versions. The secondary fix (duplicate PR check) serves as a safety net for any edge cases.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are configuration-only, implement defense-in-depth with two complementary safeguards, and directly address a well-documented root cause. The event filter logic is correct per GitHub Actions workflow_run context, and the duplicate PR check uses safe string matching with -qF. Changes are consistently applied across all 4 jobs.
  • No files require special attention

Important Files Changed

Filename Overview
.github/workflows/benchmark.yml Added workflow_run event filter to exclude push events and duplicate PR guards in all 4 benchmark jobs

Last reviewed commit: b6f55fa

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@carlos-alm carlos-alm merged commit 51ff640 into main Mar 3, 2026
18 checks passed
@carlos-alm carlos-alm deleted the fix/benchmark-duplicate-prs branch March 3, 2026 22:40
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant