Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/checkstyle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ jobs:
java-checkstyle:
name: "CheckStyle"
runs-on: ubuntu-latest
concurrency:
group: fe-code-style-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Keep historical reruns out of the current-PR group

These groups are keyed only by PR number, so rerunning an older workflow attempt after head B's check starts lets the old attempt join the same group and cancel (or replace while pending) B's validation. GitHub reruns retain the original GITHUB_SHA/GITHUB_REF, so the replacement still checks head A and cannot restore validation for B. The same issue is present in the new Clang Formatter, Gitleaks, and License Check blocks. Please isolate github.run_attempt > 1 with a run-unique group such as github.run_id; changing only cancel-in-progress does not protect a pending current-head job because the default queue replaces an existing pending member.

if: github.event_name == 'pull_request'
steps:
- name: Checkout
Expand Down Expand Up @@ -56,4 +59,3 @@ jobs:
if: steps.filter.outputs.fe_changes == 'true'
run:
cd fe && mvn clean checkstyle:check

3 changes: 3 additions & 0 deletions .github/workflows/clang-format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ jobs:
clang-format:
name: "Clang Formatter"
runs-on: ubuntu-latest
concurrency:
group: code-formatter-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true
if: github.event_name == 'pull_request'
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
Expand Down
22 changes: 15 additions & 7 deletions .github/workflows/code-review-runner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,27 @@ permissions:
jobs:
code-review:
runs-on: ubuntu-latest
concurrency:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Preserve status ownership across manual supersession

This shared group includes issue-comment runs, which always manage the code-review status, and manual dispatches where manage_status can be set to false. A false-status dispatch can cancel /review after it posts pending, complete its own review, and skip both terminal status writes; code-review-sync-result.yml has no review-completion trigger to repair the stranded status. Please require every replacement admitted to this group to take over status management, or isolate non-status-managing runs until there is an explicit ownership handoff.

group: code-review-${{ inputs.pr_number || github.event.issue.number || github.run_id }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Canonicalize the PR number before admission

inputs.pr_number is a raw string here, and GitHub accepts equivalent forms such as 67119, 067119, and 00067119 for the same pull request. Those supported dispatch/call inputs create different group names, so they can run in parallel with each other or with /review instead of coalescing per PR. Please strictly canonicalize and validate the numeric identifier in a prerequisite resolver, then key this job from its canonical output.

cancel-in-progress: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Make expected supersession cancellation-safe

Once a running review has resolved its inputs, a newer request cancels the review step, but the existing always() path treats steps.review.outcome == 'cancelled' as a genuine error. Canceled Doris run 31655483365 concretely posted "Review step was cancelled" to PR 66473 and reset the status to pending. The same run then spent the rest of GitHub's five-minute cancellation window in Litefuse and was killed before session or refreshed-auth sync began; commit 296b8f2bc0d explicitly requires that auth write-back after unrelated review failure because Codex may already have refreshed the credential. Please exclude cancellation from genuine-failure reporting and give refreshed auth a strictly bounded early persistence path (or cooperative/external handoff) before optional telemetry/session work.

# Pre-finalization steps can use 183 minutes and auth sync can use 8 more,
# leaving 12 minutes for runner setup and post-job cleanup.
timeout-minutes: 203
# Retry with a new /review request so an older workflow rerun cannot replace
# the latest review for the same PR.
if: >-
inputs.pr_number != '' ||
github.run_attempt == 1 &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Fence by logical request order, not concurrency admission order

run_attempt == 1 excludes historical reruns, but two fresh requests still both pass. GitHub documents that concurrency is ordered by when a job starts waiting for the group, not workflow dispatch/event time, and ordering is not guaranteed. If older request A reaches this job after newer B, A becomes the latest entrant and cancels B, preserving A's older focus or explicit head/base inputs. Please add a monotonic latest-request ownership check/fence rather than treating scheduler admission order as request freshness.

(
github.event_name == 'issue_comment' &&
github.event.issue.pull_request &&
startsWith(github.event.comment.body, '/review') &&
inputs.pr_number != '' ||
(
github.event.comment.author_association == 'MEMBER' ||
github.event.comment.author_association == 'OWNER' ||
github.event.comment.author_association == 'COLLABORATOR'
github.event_name == 'issue_comment' &&
github.event.issue.pull_request &&
startsWith(github.event.comment.body, '/review') &&
(
github.event.comment.author_association == 'MEMBER' ||
github.event.comment.author_association == 'OWNER' ||
github.event.comment.author_association == 'COLLABORATOR'
)
)
)
steps:
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/gitleaks-pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ jobs:
gitleaks:
name: Check for secrets
runs-on: ubuntu-latest
concurrency:
group: gitleaks-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true
env:
GITLEAKS_VERSION: 8.30.0
GITLEAKS_SHA256: 79a3ab579b53f71efd634f3aaf7e04a0fa0cf206b7ed434638d1547a2470a66e
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/license-eyes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ jobs:
license-check:
name: "License Check"
runs-on: ubuntu-latest
concurrency:
group: license-check-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true
if: |
(github.event_name == 'pull_request') ||
(github.event_name == 'push' && github.ref == 'refs/heads/master')
Expand Down
Loading