Skip to content

fix: run benchmark after publish to prevent workflow cancellation#61

Merged
carlos-alm merged 1 commit intomainfrom
feat/explain-command
Feb 23, 2026
Merged

fix: run benchmark after publish to prevent workflow cancellation#61
carlos-alm merged 1 commit intomainfrom
feat/explain-command

Conversation

@carlos-alm
Copy link
Contributor

Summary

  • Fix benchmark workflow to run after publish completes, preventing workflow cancellation

Test plan

  • CI-only change — no code modifications

Change benchmark trigger from release:published to workflow_run on
Publish so the two workflows no longer race. Benchmark now starts
only after Publish completes successfully.
@carlos-alm carlos-alm merged commit 123a418 into main Feb 23, 2026
13 of 14 checks passed
@claude
Copy link

claude bot commented Feb 23, 2026

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


PR Review Analysis

Todo List:

  • Read the changed workflow file to understand modifications
  • Analyze root cause and problem being solved
  • Assess configuration change risks
  • Check test integrity implications
  • Provide final rating and recommendation

Root Cause Analysis

Problem Being Solved: Workflow cancellation preventing benchmarks from running. The Publish workflow has cancel-in-progress: true concurrency control, which would cancel running benchmark workflows when new pushes occur.

Why This Approach: Using workflow_run trigger with types: [completed] ensures the benchmark only runs after the Publish workflow fully completes, preventing cancellation conflicts. The conditional github.event.workflow_run.conclusion == 'success' ensures benchmarks only run on successful publishes.

Risk Assessment: LOW RISK - This is a well-architected solution following GitHub Actions best practices.

Configuration Analysis

APPROPRIATE WORKFLOW CHANGES:

  • workflow_run trigger prevents race conditions between Publish and Benchmark workflows
  • Success condition prevents wasted benchmark runs on failed publishes
  • workflow_dispatch preserved for manual testing
  • No security implications (no permission changes)

Code Quality Assessment

WELL-DESIGNED:

  • Correct trigger pattern: workflow_runworkflows: ["Publish"]types: [completed] is the standard GitHub Actions pattern for dependent workflows
  • Proper conditional: github.event.workflow_run.conclusion == 'success' prevents running on failed publishes
  • Maintains flexibility: Still allows manual dispatch for testing
  • No performance impact: Workflow only triggers when needed

NO TEST INTEGRITY ISSUES:

  • This is pure CI configuration with no test weakening
  • No changes to test assertions or coverage requirements
  • Benchmark logic unchanged, only timing improved

Technical Merit

This change demonstrates strong understanding of GitHub Actions workflow orchestration:

  1. Problem identification: Recognized that cancel-in-progress: true in Publish workflow was causing benchmark cancellations
  2. Correct solution: Used workflow_run dependency pattern instead of trying to work around concurrency controls
  3. Defensive programming: Added success condition to prevent wasted runs on failed publishes

The 6-line change (+6/-2) is minimal and surgical, addressing exactly the stated problem without side effects.


Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: Clean, targeted fix using GitHub Actions best practices. Solves workflow cancellation issue with minimal, well-tested approach. No security or test integrity concerns. Only withholding 5th star because this is infrastructure maintenance rather than exceptional engineering.

Strengths:

  • Correct technical approach using workflow_run dependency
  • Minimal, surgical change
  • Preserves manual testing capability
  • No security or performance implications

Minor considerations:

  • Could have included more detail in commit message about the cancellation root cause
  • No regression risk as this only affects workflow timing

@carlos-alm carlos-alm deleted the feat/explain-command branch February 23, 2026 23:34
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 23, 2026

Greptile Summary

Changed benchmark workflow trigger from release.published event to workflow_run on Publish workflow completion to prevent workflow cancellation due to cancel-in-progress setting in publish.yml.

  • Replaced on.release.types: [published] with on.workflow_run targeting the "Publish" workflow with types: [completed]
  • Added conditional check to ensure benchmark only runs when Publish workflow succeeds or via manual dispatch
  • Preserves manual workflow_dispatch trigger as fallback

Confidence Score: 5/5

  • Safe to merge - workflow configuration fix with no code changes
  • The change correctly addresses the workflow cancellation issue by using workflow_run instead of release event, includes proper success check, and maintains backward compatibility with manual dispatch
  • No files require special attention

Important Files Changed

Filename Overview
.github/workflows/benchmark.yml Changed trigger from release.published to workflow_run on Publish workflow completion, added conditional to ensure benchmark only runs on successful publish or manual dispatch

Last reviewed commit: 24e3e23

Copy link
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant