Skip to content

Commit e89a16a

Browse files
authored
[chore](ci) refactor code review workflow to run on PR events with comment-triggered dispatch (#62369)
make AI review to be official access control workflow. Split code review workflow into two parts: 1. Main workflow (opencode-review.yml) now triggers on PR events (opened/synchronize/reopened/ready_for_review) as a required check, but only runs actual review when called via workflow_call 2. New comment dispatcher (opencode-review-comment.yml) triggers on /review comments and invokes the main workflow with PR context and remove `Need 2 Approval`. tested in zclllyybb#24 and zclllyybb#25 still need to enable `Allow GitHub Actions to create and approve pull requests` and set workflow permissions to `Read and write permissions`
1 parent 191061a commit e89a16a

File tree

8 files changed

+369
-271
lines changed

8 files changed

+369
-271
lines changed

.asf.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ github:
6262
- Build Third Party Libraries (macOS)
6363
- Build Third Party Libraries (macOS-arm64)
6464
- COMPILE (DORIS_COMPILE)
65+
- code-review
6566
- Cloud UT (Doris Cloud UT)
6667
- performance (Doris Performance)
6768
- check_coverage (Coverage)

.claude/skills/code-review/SKILL.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@ Always focus on the following core invariants during review:
4040
- **Evidence Speaks**: All issues with code itself (not memory or environment) must be clearly identified as either having problems or not. For any erroneous situation, if it cannot be confirmed locally, you must provide the specific path or logic where the error occurs. That is, if you believe that if A then B, you must specify a clear scenario where A occurs.
4141
- **Review Holistically**: For any new feature or modification, you must analyze its upstream and downstream code to understand the real invocation chain. Identify all implicit assumptions and constraints throughout the flow, then verify carefully that the current change works correctly within the entire end-to-end process. Also determine whether a seemingly problematic local pattern is actually safe due to strong guarantees from upstream or downstream, or whether a conventional local implementation fails to achieve optimal performance because it does not leverage additional information available from the surrounding context.
4242

43-
### 1.3 Critical Checkpoints (Self-Review and Review Priority)
43+
### 1.3 Critical Checkpoints (Review Priority)
4444

45-
The following checkpoints must be **individually confirmed with conclusions** during self-review and review. If the code is too long or too complex, you should delve into the code again as needed when analyzing specific issues, especially the entire logic chain where there are doubts:
45+
For PRs with not only local minor modifications, before answering specific questions, you must read and deeply understand the complete process involved in the code modification, thoroughly comprehend the role, function, and expected functionality of the reviewed functions and modules, the actual triggering methods, potential concurrency, and lifecycle. It is necessary to understand the specific triggering methods and runtime interaction relationships, as well as dependencies, of the specific code in a concrete visual manner.
46+
47+
The following checkpoints must be **individually confirmed with conclusions** during review. If the code is too long or too complex, you should delve into the code again as needed when analyzing specific issues, especially the entire logic chain where there are doubts:
4648

4749
- What is the goal of the current task? Does the current code accomplish this goal? Is there a test that proves it?
4850
- Is this modification as small, clear, and focused as possible?
@@ -67,6 +69,8 @@ The following checkpoints must be **individually confirmed with conclusions** du
6769
- Are end-to-end functional tests comprehensive?
6870
- Are there comprehensive negative test cases from various angles?
6971
- For complex features, are key functions sufficiently modular with unit tests?
72+
- Has the test result been added/modified?
73+
- Are ALL the new test results correct?
7074
- Does the feature need increased observability? If yes:
7175
- When bugs occur, are existing logs and VLOGs sufficient to investigate key issues?
7276
- Are INFO logs lightweight enough?
@@ -86,6 +90,8 @@ After checking all the above items with code. Use the remaining parts of this sk
8690

8791
#### 1.3.1 Concurrency and Thread Safety (Highest Priority)
8892

93+
If it involves the judgment of concurrent scenarios, it is necessary to find the starting point of concurrency and actually understand all actually possible concurrent situations (which thread initiated what at what stage, and what concurrent operations there will be). Due to the clear program semantics, some functions of the same module are executed in stages, so concurrency is definitely not present, there should be no misjudgment.
94+
8995
- [ ] **Thread context**: Does every new thread or bthread entry attach the right memory-tracking context? (See `be/src/runtime/AGENTS.md`)
9096
- [ ] **Lock protection**: Is shared data accessed under the correct lock and mode?
9197
- [ ] **Lock order**: Do nested acquisitions follow the module's documented lock order? (See storage, schema-change, cloud AGENTS.md)
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
name: Code Review Comment Dispatch
2+
3+
on:
4+
issue_comment:
5+
types: [created]
6+
7+
permissions:
8+
statuses: write
9+
pull-requests: write
10+
contents: read
11+
issues: write
12+
13+
jobs:
14+
resolve-pr:
15+
runs-on: ubuntu-latest
16+
if: >-
17+
github.event.issue.pull_request &&
18+
contains(github.event.comment.body, '/review')
19+
outputs:
20+
pr_number: ${{ steps.pr.outputs.pr_number }}
21+
head_sha: ${{ steps.pr.outputs.head_sha }}
22+
base_sha: ${{ steps.pr.outputs.base_sha }}
23+
steps:
24+
- name: Get PR info
25+
id: pr
26+
env:
27+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
28+
run: |
29+
PR_JSON=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.issue.number }})
30+
HEAD_SHA=$(echo "$PR_JSON" | jq -r '.head.sha')
31+
BASE_SHA=$(echo "$PR_JSON" | jq -r '.base.sha')
32+
echo "pr_number=${{ github.event.issue.number }}" >> "$GITHUB_OUTPUT"
33+
echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT"
34+
echo "base_sha=$BASE_SHA" >> "$GITHUB_OUTPUT"
35+
36+
code-review:
37+
needs:
38+
- resolve-pr
39+
- mark-review-pending
40+
if: >-
41+
github.event.issue.pull_request &&
42+
contains(github.event.comment.body, '/review')
43+
uses: ./.github/workflows/opencode-review-runner.yml
44+
secrets: inherit
45+
with:
46+
pr_number: ${{ needs.resolve-pr.outputs.pr_number }}
47+
head_sha: ${{ needs.resolve-pr.outputs.head_sha }}
48+
base_sha: ${{ needs.resolve-pr.outputs.base_sha }}
49+
50+
mark-review-pending:
51+
needs: resolve-pr
52+
runs-on: ubuntu-latest
53+
if: >-
54+
github.event.issue.pull_request &&
55+
contains(github.event.comment.body, '/review')
56+
steps:
57+
- name: Mark Code Review status as pending
58+
env:
59+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
60+
REPO: ${{ github.repository }}
61+
HEAD_SHA: ${{ needs.resolve-pr.outputs.head_sha }}
62+
run: |
63+
gh api repos/${REPO}/statuses/${HEAD_SHA} \
64+
-X POST \
65+
-f state='pending' \
66+
-f context='code-review' \
67+
-f description="Automated review is running for ${HEAD_SHA}." \
68+
-f target_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
69+
70+
refresh-required-check:
71+
needs:
72+
- resolve-pr
73+
- code-review
74+
runs-on: ubuntu-latest
75+
if: ${{ always() && needs.resolve-pr.result == 'success' && needs.code-review.result != 'skipped' }}
76+
steps:
77+
- name: Sync Code Review check for current head
78+
env:
79+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
80+
REPO: ${{ github.repository }}
81+
PR_NUMBER: ${{ needs.resolve-pr.outputs.pr_number }}
82+
HEAD_SHA: ${{ needs.resolve-pr.outputs.head_sha }}
83+
run: |
84+
REVIEWS=$(gh api --paginate repos/${REPO}/pulls/${PR_NUMBER}/reviews)
85+
review_state=$(printf '%s' "$REVIEWS" | jq -r --arg head_sha "$HEAD_SHA" '
86+
([ .[]
87+
| select(.user.login == "github-actions[bot]" or .user.login == "github-actions")
88+
| select(.commit_id == $head_sha)
89+
| select(.state == "APPROVED" or .state == "CHANGES_REQUESTED")
90+
]
91+
| sort_by(.submitted_at)
92+
| last
93+
| .state) // ""
94+
')
95+
96+
conclusion="failure"
97+
summary="No automated approve/request-changes review exists for ${HEAD_SHA}."
98+
99+
if [ "$review_state" = "APPROVED" ]; then
100+
conclusion="success"
101+
summary="Automated review approved the PR for ${HEAD_SHA}."
102+
elif [ "$review_state" = "CHANGES_REQUESTED" ]; then
103+
summary="Automated review requested changes for ${HEAD_SHA}."
104+
fi
105+
106+
state="failure"
107+
[ "$conclusion" = "success" ] && state="success"
108+
109+
gh api repos/${REPO}/statuses/${HEAD_SHA} \
110+
-X POST \
111+
-f state="${state}" \
112+
-f context='code-review' \
113+
-f description="${summary}" \
114+
-f target_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
name: Code Review Runner
2+
3+
on:
4+
workflow_call:
5+
inputs:
6+
pr_number:
7+
required: true
8+
type: string
9+
head_sha:
10+
required: true
11+
type: string
12+
base_sha:
13+
required: true
14+
type: string
15+
16+
permissions:
17+
pull-requests: write
18+
contents: read
19+
issues: write
20+
21+
jobs:
22+
code-review:
23+
runs-on: ubuntu-latest
24+
timeout-minutes: 60
25+
steps:
26+
- name: Checkout repository
27+
uses: actions/checkout@v4
28+
with:
29+
ref: ${{ inputs.head_sha }}
30+
31+
- name: Install ripgrep
32+
run: |
33+
sudo apt-get update
34+
sudo apt-get install -y ripgrep
35+
36+
- name: Install OpenCode
37+
run: |
38+
for attempt in 1 2 3; do
39+
if curl -fsSL https://opencode.ai/install | bash; then
40+
echo "$HOME/.opencode/bin" >> $GITHUB_PATH
41+
exit 0
42+
fi
43+
echo "Install attempt $attempt failed, retrying in 10s..."
44+
sleep 10
45+
done
46+
echo "All install attempts failed"
47+
exit 1
48+
49+
- name: Configure OpenCode auth
50+
run: |
51+
mkdir -p ~/.local/share/opencode
52+
cat > ~/.local/share/opencode/auth.json <<EOF
53+
{
54+
"github-copilot": {
55+
"type": "oauth",
56+
"refresh": "${CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY}",
57+
"access": "${CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY}",
58+
"expires": 0
59+
}
60+
}
61+
EOF
62+
env:
63+
CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY: ${{ secrets.CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY }}
64+
65+
- name: Configure OpenCode permission
66+
run: |
67+
echo '{"permission":"allow"}' > opencode.json
68+
69+
- name: Prepare review prompt
70+
run: |
71+
cat > /tmp/review_prompt.txt <<'PROMPT'
72+
You are performing an automated code review inside a GitHub Actions runner. The gh CLI is available and authenticated via GH_TOKEN. You can comment on the pull request.
73+
74+
Context:
75+
- Repository: PLACEHOLDER_REPO
76+
- PR number: PLACEHOLDER_PR_NUMBER
77+
- PR Head SHA: PLACEHOLDER_HEAD_SHA
78+
- PR Base SHA: PLACEHOLDER_BASE_SHA
79+
80+
Before reviewing any code, you MUST read and follow the code review skill in this repository. During review, you must strictly follow those instructions.
81+
In addition, you can perform any desired review operations to observe suspicious code and details in order to identify issues as much as possible.
82+
83+
## Final response format
84+
- After completing the review, you MUST provide a final summary opinion based on the rules defined in AGENTS.md and the code-review skill. The summary must include conclusions for each applicable critical checkpoint.
85+
- If the overall quality of PR is good and there are no critical blocking issues (even if there are some tolerable minor issues), submit an opinion on approval using: gh pr review PLACEHOLDER_PR_NUMBER --approve --body "<summary>"
86+
- If issues found, submit a review with inline comments plus a comprehensive summary body. Use GitHub Reviews API to ensure comments are inline:
87+
- Inline comment bodies may include GitHub suggested changes blocks when you can propose a precise patch.
88+
- Prefer suggested changes for small, self-contained fixes (for example typos, trivial refactors, or narrowly scoped code corrections).
89+
- Do not force suggested changes for broad, architectural, or multi-file issues; explain those normally.
90+
- Build a JSON array of comments like: [{ "path": "<file>", "position": <diff_position>, "body": "..." }]
91+
- Submit via: gh api repos/PLACEHOLDER_REPO/pulls/PLACEHOLDER_PR_NUMBER/reviews --input <json_file>
92+
- The JSON file should contain: {"event":"REQUEST_CHANGES","body":"<summary>","comments":[...]}
93+
- After publish your opinion, your final response MUST end with exactly one machine-readable result block in this format:
94+
RESULT_START
95+
{
96+
"decision": "APPROVE" | "REQUEST_CHANGES"
97+
}
98+
RESULT_END
99+
PROMPT
100+
sed -i "s|PLACEHOLDER_REPO|${REPO}|g" /tmp/review_prompt.txt
101+
sed -i "s|PLACEHOLDER_PR_NUMBER|${PR_NUMBER}|g" /tmp/review_prompt.txt
102+
sed -i "s|PLACEHOLDER_HEAD_SHA|${HEAD_SHA}|g" /tmp/review_prompt.txt
103+
sed -i "s|PLACEHOLDER_BASE_SHA|${BASE_SHA}|g" /tmp/review_prompt.txt
104+
env:
105+
REPO: ${{ github.repository }}
106+
PR_NUMBER: ${{ inputs.pr_number }}
107+
HEAD_SHA: ${{ inputs.head_sha }}
108+
BASE_SHA: ${{ inputs.base_sha }}
109+
110+
- name: Run automated code review
111+
id: review
112+
timeout-minutes: 55
113+
continue-on-error: true
114+
env:
115+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
116+
run: |
117+
PROMPT=$(cat /tmp/review_prompt.txt)
118+
119+
set +e
120+
opencode run "$PROMPT" -m "github-copilot/gpt-5.4" 2>&1 | tee /tmp/opencode-review.log
121+
status=${PIPESTATUS[0]}
122+
set -e
123+
124+
last_log_line=$(awk 'NF { line = $0 } END { print line }' /tmp/opencode-review.log)
125+
126+
failure_reason=""
127+
if printf '%s\n' "$last_log_line" | rg -q -i '^Error:|SSE read timed out'; then
128+
failure_reason="$last_log_line"
129+
elif [ "$status" -ne 0 ]; then
130+
failure_reason="OpenCode exited with status $status"
131+
fi
132+
133+
result_json=$(awk '
134+
/^RESULT_START$/ {capture=1; current=""; next}
135+
/^RESULT_END$/ {capture=0; result=current; found=1; next}
136+
capture {current = current $0 ORS}
137+
END {
138+
if (found) {
139+
printf "%s", result
140+
}
141+
}
142+
' /tmp/opencode-review.log)
143+
144+
if [ -z "$failure_reason" ] && [ -z "$result_json" ]; then
145+
failure_reason="OpenCode did not emit a RESULT_START/RESULT_END result block"
146+
fi
147+
148+
if [ -z "$failure_reason" ] && ! printf '%s' "$result_json" > /tmp/opencode-review-result.json; then
149+
failure_reason="Failed to persist OpenCode review result"
150+
fi
151+
152+
if [ -z "$failure_reason" ] && ! jq -e '.decision == "APPROVE" or .decision == "REQUEST_CHANGES"' /tmp/opencode-review-result.json > /dev/null; then
153+
failure_reason="OpenCode emitted an invalid review decision"
154+
fi
155+
156+
if [ -n "$failure_reason" ]; then
157+
{
158+
echo "failure_reason<<EOF"
159+
printf '%s\n' "$failure_reason"
160+
echo "EOF"
161+
} >> "$GITHUB_OUTPUT"
162+
exit 1
163+
fi
164+
165+
decision=$(jq -r '.decision' /tmp/opencode-review-result.json)
166+
echo "decision=$decision" >> "$GITHUB_OUTPUT"
167+
168+
if [ "$decision" == "REQUEST_CHANGES" ]; then
169+
exit 1
170+
fi
171+
172+
- name: Comment PR on review failure
173+
if: ${{ always() && steps.review.outcome != 'success' && steps.review.outputs.decision != 'REQUEST_CHANGES' }}
174+
env:
175+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
176+
REVIEW_FAILURE_REASON: ${{ steps.review.outputs.failure_reason }}
177+
REVIEW_OUTCOME: ${{ steps.review.outcome }}
178+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
179+
run: |
180+
error_msg="${REVIEW_FAILURE_REASON:-Review step was $REVIEW_OUTCOME (possibly timeout or cancelled)}"
181+
gh pr comment "${{ inputs.pr_number }}" --body "$(cat <<EOF
182+
OpenCode automated review failed and did not complete.
183+
184+
Error: ${error_msg}
185+
Workflow run: ${RUN_URL}
186+
187+
Please inspect the workflow logs and rerun the review after the underlying issue is resolved.
188+
EOF
189+
)"
190+
191+
- name: Fail workflow if review failed
192+
if: ${{ always() && steps.review.outcome != 'success' }}
193+
env:
194+
REVIEW_FAILURE_REASON: ${{ steps.review.outputs.failure_reason }}
195+
REVIEW_OUTCOME: ${{ steps.review.outcome }}
196+
run: |
197+
error_msg="${REVIEW_FAILURE_REASON:-Review step was $REVIEW_OUTCOME (possibly timeout or cancelled)}"
198+
echo "OpenCode automated review failed: ${error_msg}"
199+
exit 1

0 commit comments

Comments
 (0)