Skip to content
Closed
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
2 changes: 1 addition & 1 deletion .asf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ github:
- Build Third Party Libraries (macOS)
- Build Third Party Libraries (macOS-arm64)
- COMPILE (DORIS_COMPILE)
- Need_2_Approval
- Code Review
- Cloud UT (Doris Cloud UT)
- performance (Doris Performance)
- check_coverage (Coverage)
Expand Down
10 changes: 8 additions & 2 deletions .claude/skills/code-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ Always focus on the following core invariants during review:
- **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.
- **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.

### 1.3 Critical Checkpoints (Self-Review and Review Priority)
### 1.3 Critical Checkpoints (Review Priority)

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:
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.

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:

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

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

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.
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

This added sentence has awkward repeated wording (“actually understand all actually possible…”), which reads like a copy/edit mistake. Please rephrase to remove the repetition so the guideline is clear.

Suggested change
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.
If it involves the judgment of concurrent scenarios, it is necessary to find the starting point of concurrency and fully understand all 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.

Copilot uses AI. Check for mistakes.

- [ ] **Thread context**: Does every new thread or bthread entry attach the right memory-tracking context? (See `be/src/runtime/AGENTS.md`)
- [ ] **Lock protection**: Is shared data accessed under the correct lock and mode?
- [ ] **Lock order**: Do nested acquisitions follow the module's documented lock order? (See storage, schema-change, cloud AGENTS.md)
Expand Down
75 changes: 75 additions & 0 deletions .github/workflows/opencode-review-comment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
name: Code Review Comment Dispatch

on:
issue_comment:
types: [created]

permissions:
actions: write
pull-requests: write
contents: read
issues: write

jobs:
resolve-pr:
runs-on: ubuntu-latest
if: >-
github.event.issue.pull_request &&
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.

This dispatcher is callable by any user who can comment on a public PR. Because issue_comment runs in the base-repo context and the called workflow uses secrets: inherit, an outside commenter can trigger secret-backed review runs and write-capable bot actions just by posting /review. Please gate this on github.event.comment.author_association (for example OWNER, MEMBER, or COLLABORATOR) before invoking the reusable workflow.

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.

issue_comment runs in the base repository context, but this job only checks for a /review substring before invoking a reusable workflow with secrets: inherit. That means any outside user who can comment on a public PR can trigger a secret-backed run over untrusted head code. Please gate this on trusted author_association values and refuse fork heads (or avoid inheriting sensitive secrets entirely).

contains(github.event.comment.body, '/review')
Comment on lines +16 to +18
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

As written, any user who can comment on a PR can trigger /review, and secrets: inherit will pass repository secrets into a workflow that checks out and processes untrusted PR code. This is a privilege-escalation risk (especially for PRs from forks). Recommended: (1) gate dispatch on github.event.comment.author_association (e.g., OWNER/MEMBER/COLLABORATOR) and/or an allowlist; (2) fetch PR metadata and refuse to run (or run without sensitive secrets) when .head.repo.fork == true or .head.repo.full_name != github.repository; and (3) avoid secrets: inherit—pass only the minimal required secrets explicitly.

Copilot uses AI. Check for mistakes.
outputs:
Comment on lines +14 to +19
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

This workflow can be triggered by any user who comments “/review”, but it runs with write permissions and uses inherited secrets. Please restrict triggering to trusted actors (e.g., check github.event.comment.author_association for OWNER/MEMBER/COLLABORATOR or verify org/team membership) to avoid untrusted users invoking a secrets-bearing workflow.

Copilot uses AI. Check for mistakes.
pr_number: ${{ steps.pr.outputs.pr_number }}
head_sha: ${{ steps.pr.outputs.head_sha }}
base_sha: ${{ steps.pr.outputs.base_sha }}
steps:
- name: Get PR info
id: pr
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
PR_JSON=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.issue.number }})
HEAD_SHA=$(echo "$PR_JSON" | jq -r '.head.sha')
BASE_SHA=$(echo "$PR_JSON" | jq -r '.base.sha')
echo "pr_number=${{ github.event.issue.number }}" >> "$GITHUB_OUTPUT"
echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT"
echo "base_sha=$BASE_SHA" >> "$GITHUB_OUTPUT"

code-review:
needs: resolve-pr
if: >-
github.event.issue.pull_request &&
contains(github.event.comment.body, '/review')
uses: ./.github/workflows/opencode-review-runner.yml
secrets: inherit
with:
pr_number: ${{ needs.resolve-pr.outputs.pr_number }}
head_sha: ${{ needs.resolve-pr.outputs.head_sha }}
base_sha: ${{ needs.resolve-pr.outputs.base_sha }}

refresh-required-check:
needs:
- resolve-pr
- code-review
runs-on: ubuntu-latest
if: ${{ always() && needs.resolve-pr.result == 'success' && needs.code-review.result != 'skipped' }}
steps:
- name: Rerun pull_request Code Review workflow for current head
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
HEAD_SHA: ${{ needs.resolve-pr.outputs.head_sha }}
run: |
RUNS_JSON=$(gh api repos/${REPO}/actions/workflows/opencode-review.yml/runs --paginate -f event=pull_request -f head_sha=${HEAD_SHA})
RUN_ID=$(printf '%s' "$RUNS_JSON" | jq -r '
.workflow_runs
| sort_by(.created_at)
| reverse
| map(select(.head_sha != null))
Comment on lines +60 to +66
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The rerun logic relies on gh api .../actions/workflows/.../runs query params event and especially head_sha. If head_sha is not supported/ignored, this may select and rerun the most recent workflow run from a different PR/commit. Safer approach: fetch recent runs and filter in jq by both .head_sha == HEAD_SHA and .pull_requests[].number == PR_NUMBER (you already have pr_number available) before picking the run id.

Suggested change
run: |
RUNS_JSON=$(gh api repos/${REPO}/actions/workflows/opencode-review.yml/runs --paginate -f event=pull_request -f head_sha=${HEAD_SHA})
RUN_ID=$(printf '%s' "$RUNS_JSON" | jq -r '
.workflow_runs
| sort_by(.created_at)
| reverse
| map(select(.head_sha != null))
PR_NUMBER: ${{ needs.resolve-pr.outputs.pr_number }}
run: |
RUNS_JSON=$(gh api repos/${REPO}/actions/workflows/opencode-review.yml/runs --paginate -f event=pull_request)
RUN_ID=$(printf '%s' "$RUNS_JSON" | jq -r --arg head_sha "$HEAD_SHA" --argjson pr_number "$PR_NUMBER" '
.workflow_runs
| map(
select(
.head_sha == $head_sha and
any(.pull_requests[]?; .number == $pr_number)
)
)
| sort_by(.created_at)
| reverse

Copilot uses AI. Check for mistakes.
| .[0].id // empty
')

if [ -z "$RUN_ID" ]; then
echo "No pull_request workflow run found for opencode-review.yml at head ${HEAD_SHA}."
exit 1
fi

gh api repos/${REPO}/actions/runs/${RUN_ID}/rerun -X POST
223 changes: 223 additions & 0 deletions .github/workflows/opencode-review-runner.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
name: Code Review Runner

on:
workflow_call:
inputs:
pr_number:
required: true
type: string
head_sha:
required: true
type: string
base_sha:
required: true
type: string

permissions:
pull-requests: write
contents: read
issues: write

jobs:
code-review:
runs-on: ubuntu-latest
timeout-minutes: 60
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
ref: ${{ inputs.head_sha }}
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.

Checking out ${{ inputs.head_sha }} here makes the rest of this job run against untrusted PR contents, but this reusable workflow is invoked from issue_comment with secrets: inherit. That is a concrete privilege-escalation path for fork PRs: the contributor can change AGENTS.md or .claude/skills/code-review/SKILL.md, and the prompt later requires OpenCode to read and follow those files from the checked-out workspace while GH_TOKEN and CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY are available. In that scenario the model can be steered into exfiltrating credentials or performing arbitrary PR writes. The trusted review instructions need to come from the base repository revision only, or this job must avoid exposing secrets/write permissions when operating on head-controlled content.


- name: Install ripgrep
run: |
sudo apt-get update
sudo apt-get install -y ripgrep

- name: Install OpenCode
run: |
for attempt in 1 2 3; do
if curl -fsSL https://opencode.ai/install | bash; then
echo "$HOME/.opencode/bin" >> $GITHUB_PATH
exit 0
fi
echo "Install attempt $attempt failed, retrying in 10s..."
sleep 10
done
echo "All install attempts failed"
exit 1

- name: Configure OpenCode auth
run: |
mkdir -p ~/.local/share/opencode
cat > ~/.local/share/opencode/auth.json <<EOF
{
"github-copilot": {
"type": "oauth",
"refresh": "${CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY}",
"access": "${CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY}",
"expires": 0
}
}
EOF
env:
CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY: ${{ secrets.CODE_REVIEW_ZCLLL_COPILOT_OPENCODE_KEY }}

- name: Configure OpenCode permission
run: |
echo '{"permission":"allow"}' > opencode.json

- name: Prepare review prompt
run: |
cat > /tmp/review_prompt.txt <<'PROMPT'
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.

Context:
- Repository: PLACEHOLDER_REPO
- PR number: PLACEHOLDER_PR_NUMBER
- PR Head SHA: PLACEHOLDER_HEAD_SHA
- PR Base SHA: PLACEHOLDER_BASE_SHA

Before reviewing any code, you MUST read and follow these repository instructions from the checked-out workspace:
1. Read `.claude/skills/code-review/SKILL.md` in full.
2. Read the repository root `AGENTS.md` in full.
3. For each changed source area, read any nearest relevant `AGENTS.md` in the corresponding source directory before drawing conclusions.

During review, you must strictly follow those instructions. In particular:
- Review the complete invocation chain and runtime behavior, not only the local diff.
- Give explicit conclusions for each applicable critical checkpoint.
- Do not leave uncertainty when the code context can resolve it.
- Use comments only for real issues. If something is safe because of upstream or downstream guarantees, state that clearly instead of speculating.

## Submission
- 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.
- You MUST submit exactly one final GitHub review for the current PR head SHA.
- The final review state MUST be either APPROVE or REQUEST_CHANGES. Do not leave the PR with only comments.
- If no blocking issues are found, submit: gh pr review PLACEHOLDER_PR_NUMBER --approve --body "<summary>"
- If blocking issues are found and no inline comments are needed, submit: gh pr review PLACEHOLDER_PR_NUMBER --request-changes --body "<summary>"
- If blocking issues are found and inline comments are needed, use GitHub Reviews API to submit a single REQUEST_CHANGES review with inline comments:
- Inline comment bodies may include GitHub suggested changes blocks when you can propose a precise patch.
- Prefer suggested changes for small, self-contained fixes (for example typos, trivial refactors, or narrowly scoped code corrections).
- Do not force suggested changes for broad, architectural, or multi-file issues; explain those normally.
- Build a JSON array of comments like: [{ "path": "<file>", "position": <diff_position>, "body": "..." }]
- Submit via: gh api repos/PLACEHOLDER_REPO/pulls/PLACEHOLDER_PR_NUMBER/reviews --input <json_file>
- The JSON file should contain: {"event":"REQUEST_CHANGES","body":"<summary>","comments":[...]}
- Do not submit a separate summary comment instead of the final review.
PROMPT
sed -i "s|PLACEHOLDER_REPO|${REPO}|g" /tmp/review_prompt.txt
sed -i "s|PLACEHOLDER_PR_NUMBER|${PR_NUMBER}|g" /tmp/review_prompt.txt
sed -i "s|PLACEHOLDER_HEAD_SHA|${HEAD_SHA}|g" /tmp/review_prompt.txt
sed -i "s|PLACEHOLDER_BASE_SHA|${BASE_SHA}|g" /tmp/review_prompt.txt
env:
REPO: ${{ github.repository }}
PR_NUMBER: ${{ inputs.pr_number }}
HEAD_SHA: ${{ inputs.head_sha }}
BASE_SHA: ${{ inputs.base_sha }}

- name: Run automated code review
id: review
timeout-minutes: 55
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
PROMPT=$(cat /tmp/review_prompt.txt)

set +e
opencode run "$PROMPT" -m "github-copilot/gpt-5.4" 2>&1 | tee /tmp/opencode-review.log
status=${PIPESTATUS[0]}
set -e

last_log_line=$(awk 'NF { line = $0 } END { print line }' /tmp/opencode-review.log)

failure_reason=""
if printf '%s\n' "$last_log_line" | rg -q -i '^Error:|SSE read timed out'; then
failure_reason="$last_log_line"
elif [ "$status" -ne 0 ]; then
failure_reason="OpenCode exited with status $status"
fi

if [ -n "$failure_reason" ]; then
{
echo "failure_reason<<EOF"
printf '%s\n' "$failure_reason"
echo "EOF"
} >> "$GITHUB_OUTPUT"
exit 1
fi

- name: Verify submitted review decision
if: ${{ steps.review.outcome == 'success' }}
id: review_state
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ inputs.pr_number }}
HEAD_SHA: ${{ inputs.head_sha }}
run: |
REVIEWS=$(gh api repos/${REPO}/pulls/${PR_NUMBER}/reviews)
review_state=$(printf '%s' "$REVIEWS" | jq -r --arg head_sha "$HEAD_SHA" '
[ .[]
| select(.user.login == "github-actions[bot]")
| select(.commit_id == $head_sha)
| select(.state == "APPROVED" or .state == "CHANGES_REQUESTED")
]
| sort_by(.submitted_at)
| last
| .state // ""
')

echo "review_state=$review_state" >> "$GITHUB_OUTPUT"

if [ "$review_state" = "APPROVED" ]; then
echo "Automated review approved the PR."
exit 0
fi

if [ "$review_state" = "CHANGES_REQUESTED" ]; then
failure_reason="Automated review requested changes."
else
failure_reason="No final approve/request-changes review was submitted for head SHA ${HEAD_SHA}."
fi

{
echo "failure_reason<<EOF"
printf '%s\n' "$failure_reason"
echo "EOF"
} >> "$GITHUB_OUTPUT"
exit 1

- name: Comment PR on review failure
if: ${{ always() && (steps.review.outcome != 'success' || (steps.review_state.outcome != 'success' && steps.review_state.outputs.review_state == '')) }}
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REVIEW_FAILURE_REASON: ${{ steps.review.outputs.failure_reason }}
STATE_FAILURE_REASON: ${{ steps.review_state.outputs.failure_reason }}
REVIEW_OUTCOME: ${{ steps.review.outcome }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
run: |
error_msg="${REVIEW_FAILURE_REASON:-${STATE_FAILURE_REASON:-Review step was $REVIEW_OUTCOME (possibly timeout or cancelled)}}"
gh pr comment "${{ inputs.pr_number }}" --body "$(cat <<EOF
OpenCode automated review failed and did not complete.

Error: ${error_msg}
Workflow run: ${RUN_URL}

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.
EOF
)"

- name: Fail workflow if review failed
if: ${{ always() && (steps.review.outcome != 'success' || steps.review_state.outcome != 'success') }}
env:
REVIEW_FAILURE_REASON: ${{ steps.review.outputs.failure_reason }}
STATE_FAILURE_REASON: ${{ steps.review_state.outputs.failure_reason }}
REVIEW_STATE: ${{ steps.review_state.outputs.review_state }}
REVIEW_OUTCOME: ${{ steps.review.outcome }}
run: |
if [ "$REVIEW_STATE" = "CHANGES_REQUESTED" ]; then
error_msg="${STATE_FAILURE_REASON:-Automated review requested changes.}"
else
error_msg="${REVIEW_FAILURE_REASON:-${STATE_FAILURE_REASON:-Review step was $REVIEW_OUTCOME (possibly timeout or cancelled)}}"
fi
echo "OpenCode automated review failed: ${error_msg}"
exit 1
Loading
Loading