Skip to content

fix(ci): skip verify after merge-group cancellation (BLO-21953) - #1289

Merged
kkroo merged 1 commit into
masterfrom
kkroo/blo-21953-skip-cancelled-verify
Aug 11, 2026
Merged

fix(ci): skip verify after merge-group cancellation (BLO-21953)#1289
kkroo merged 1 commit into
masterfrom
kkroo/blo-21953-skip-cancelled-verify

Conversation

@kkroo

@kkroo kkroo commented Aug 11, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip relies on its protected merge queue to land reviewed changes without breaking master.
  • Merge-group workflow generations now share a stable concurrency key per base queue, so a re-stage cancels the superseded generation.
  • The aggregate verify job still used always(), which caused a canceled generation to materialize that job after cancellation.
  • While that canceled finalizer waited for an ARC runner, the old workflow generation retained the shared merge-group concurrency lock and delayed the replacement generation.
  • The finalizer must still aggregate failed, skipped, and canceled lane results during a live workflow, but it must not start after the workflow itself is canceled.
  • This pull request gates verify with always() && !cancelled() and pins that condition in the existing workflow regression suite.
  • The benefit is prompt lock release after merge-queue re-staging without weakening the required aggregate check on live generations.

Linked Issues or Issue Description

No GitHub issue exists for this queue incident. It is tracked in BLO-21953.

Related work:

What Changed

  • Prevent the aggregate verify job from materializing after workflow cancellation while preserving its always() behavior for live-workflow lane outcomes.
  • Extend the existing verify workflow regression test to pin the cancellation-aware job condition.

Verification

  • node --test scripts/__tests__/pr-verify-lane-outcome.test.mjs - 13/13 pass.
  • node --test scripts/__tests__/merge-group-concurrency.test.mjs - 3/3 pass.
  • node scripts/check-github-runner-labels.mjs - all 20 workflows use ARC labels.
  • ruby -ryaml -e 'YAML.load_file(%q[.github/workflows/pr.yml])' - workflow YAML parses.
  • git diff --check - clean.

Risks

Low and bounded to canceled workflow generations. Live workflows still run verify after upstream failures, skips, or lane cancellations and preserve the required-check name. A manually canceled workflow no longer spends a runner on a finalizer whose result cannot make that canceled generation mergeable.

Model Used

OpenAI Codex based on GPT-5, with reasoning, repository editing, shell execution, GitHub CLI, and live GitHub Actions inspection.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots - N/A, CI-only change
  • I have updated relevant documentation to reflect my changes - workflow rationale and regression coverage
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups - N/A, not configured
  • I will address all Greptile and reviewer comments before requesting merge

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@allyblockcast

allyblockcast Bot commented Aug 11, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-21953

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 11, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-21953

@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@kkroo
kkroo requested a review from allyblockcast August 11, 2026 02:46
@kkroo
kkroo enabled auto-merge August 11, 2026 02:47

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 8fe30a9

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The job condition preserves always() for live lane aggregation while preventing canceled merge-group runs from retaining the concurrency lock.
  • The regression test directly asserts the workflow condition, and the existing lane-outcome coverage remains intact.

Recommended Action

  1. Ready to merge.

@allyblockcast allyblockcast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 8fe30a9

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The job condition preserves always() for live lane aggregation while preventing canceled merge-group runs from retaining the concurrency lock.
  • The regression test directly asserts the workflow condition, and the existing lane-outcome coverage remains intact.

Recommended Action

  1. Ready to merge.

@kkroo
kkroo merged commit 0b1eab8 into master Aug 11, 2026
4 checks passed
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.

2 participants