feat(ci): add PR review workflow using bundled /review skill#4549
Merged
Conversation
Trigger review via `@qwen-code /review` in PR comments, or automatically on PR open/reopen for OWNER/MEMBER/COLLABORATOR. Supports workflow_dispatch for manual dry-run testing. Key design choices: - Direct qwen CLI invocation (not qwen-code-action) for real-time logs - --ci flag skips build/test/autofix, bounds verification budget - --kill-after=10s ensures clean termination on timeout - Fork PRs are skipped (fail-closed) - Anti-injection prefix on user-provided review instructions
yiliang114
commented
May 26, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
Summary
This PR replaces the qwen-code-action with direct qwen CLI invocation and adds a --ci flag for non-interactive reviews. Overall a solid improvement with good security practices (base-branch checkout, fork PR skip, removed id-token: write). However, there are 3 critical findings and 9 suggestions to address.
8 findings reported by review agents, 7 confirmed after verification, 4 additional from reverse audit. Deterministic analysis skipped (CI mode).
Critical
-
Prompt injection via comment body into yolo-mode LLM — User comment text is appended verbatim to the LLM prompt with only a natural-language "treat as untrusted" caveat. Combined with
--approval-mode yoloand secrets (OPENAI_API_KEY,GITHUB_TOKEN) in the environment, a collaborator could attempt prompt injection to exfiltrate secrets. See inline comment at line 157. -
cancel-in-progress: trueposts misleading tombstone comments — When a new trigger cancels an in-progress review, the cancelled run posts a "did not complete" fallback comment before the replacement finishes. PR authors see a confusing failure comment. See inline comment at line 58. -
Hardcoded
ref: 'main'— Will break silently if the default branch is ever renamed. See inline comment at line 72.
Suggestions
QWEN_CLI_VERSIONdefaults tolatest— unpinned npm supply chain risk. See inline comment at line 66.- Missing draft PR filter — draft PRs get reviewed on open/reopen AND again on ready_for_review.
- Fallback step condition fragile on
review_mode— should checkpr_number != ''instead ofreview_mode == 'comment'. 2>&1 | teemerges stderr into JSONL — crash traces corrupt structured output.- Bash greedy
##in instruction extraction — strips to LAST occurrence instead of first. - Fork PR skip posts no user-visible feedback — green workflow with no review or explanation on fork PRs.
- No PR state check —
@qwen-code /reviewon a closed/merged PR wastes CI minutes. - SKILL.md agent count ambiguity — "8 agents for cross-repo or CI mode" could be misread as 9 agents for same-repo CI reviews.
Verdict
Comment (downgraded from Request Changes due to self-PR). The critical findings around prompt injection, cancel-in-progress UX, and hardcoded branch name should be addressed before merging.
— qwen3.7-max via Qwen Code /review
- Add --auth-type openai for non-interactive CI mode - Use dynamic default_branch instead of hardcoded 'main' - Add fetch-depth: 0 for full git history - Fix greedy ## pattern to non-greedy # in instruction extraction - Improve fallback condition to cover cancelled runs - Align setup-node to v6.4.0 matching other workflows - Clarify CI mode docs: allow gh pr review/comment write calls - Clarify agent count for CI mode (skip Build & Test regardless of repo)
Collaborator
Author
CI Review Workflow 测试报告在 yiliang114/qwen-code fork 上完成了端到端验证，覆盖不同规模 PR 和多种触发方式。 测试环境
测试用例
验证的功能点
发现的问题（已修复）
相关 Actions Runs
|
Use self-hosted ECS runner for lower API latency and faster cold start.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Restore bot PAT so review comments appear as qwen-code-ci-bot instead of github-actions[bot].
Remove setup-node and npm install steps since ecs-qwen runner already has Node.js and qwen CLI pre-installed. Also use full runner label array [self-hosted, linux, x64, ecs-qwen] to match the gate-review workflow convention.
Full clone is unnecessary since qwen CLI fetches diffs via GitHub API. Shallow clone avoids the slow full-history fetch on self-hosted runner.
- Quote runs-on array items with single quotes - Quote ref expression value - Replace dynamic timeout-minutes expression with fixed 60 (shell-level timeout already handles the configurable value)
wenshao
reviewed
May 27, 2026
wenshao
reviewed
May 27, 2026
Collaborator
|
Qwen Code review did not complete successfully (it may have been superseded by a newer review request). See workflow logs. |
Collaborator
|
建议将 原因： 当前 workflow 本身已经可以工作（通过 — qwen3.7-max via Qwen Code /review |
Collaborator
|
Qwen Code review did not complete successfully (it may have been superseded by a newer review request). See workflow logs. |
qwen-code-ci-bot
previously requested changes
May 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Qwen Code Review — PR #4549
3 Critical · 4 Suggestions
This PR rewrites the CI review workflow and adds --ci mode to the review skill. The overall direction is sound — moving to a bounded, non-interactive CI review mode with an explicit shell-command allowlist is a clear improvement. However, several security and reliability gaps remain in the workflow that need addressing before this runs on a persistent self-hosted runner with repository secrets.
Critical findings:
- Self-hosted persistent runner +
--approval-mode yoloamplifies blast radius (F2) timeout --kill-after=10scan orphan subprocesses on the persistent runner (F3)- No early validation that secrets are non-empty before invoking qwen (F5)
Suggestions:
write_fileandeditremain inallowedToolsdespite CI mode (F1)- No post-review exit-code validation — qwen crash is invisible (F4)
blockOnExistingCommentsnot bypassed for CI — would block automation (R1)- No check that the PR is still open before starting review (R2)
Full report saved to .qwen/reviews/pr-4549-review.md.
wenshao
reviewed
May 27, 2026
yiliang114
force-pushed
the
feat/ci-code-review
branch
from
5e3e8d1 to
b33fe54
Compare
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No new review findings after thorough analysis. The 14-commit iteration has addressed all previously raised concerns — draft PR guard, strict trigger matching, timeout validation, log cleanup, and cancel-in-progress tombstone fix are all correctly implemented in the latest code. Downgraded from Approve to Comment: CI still running. LGTM! ✅ — qwen3.7-max via Qwen Code /review
wenshao
approved these changes
May 27, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review
Collaborator
Maintainer Verification Report — PR #4549Reviewer: wenshao T1: YAML Syntax Validation ✅
T2: Shell Script Analysis ✅
T3: Security Analysis ✅ (17/17 checks passed)
T4: Workflow Logic & Edge Cases ✅
T5: Action Version Pinning ✅
T6: Cross-Reference with Existing Workflows ✅
T7: Build Verification ✅
Summary
Verdict: All 7 verification categories passed. No blockers for merge.
|
wenshao
requested changes
May 27, 2026
| contains(github.event.comment.body, '@qwen /review') && | ||
| github.event.issue.state == 'open' && | ||
| (github.event.comment.body == '@qwen-code /review' || | ||
| startsWith(github.event.comment.body, '@qwen-code /review ')) && |
Collaborator
There was a problem hiding this comment.
[Critical] Multi-line trigger command silently rejected by if: guard.
The startsWith(body, '@qwen-code /review ') check requires a trailing space after /review. When a user writes a natural multi-line trigger comment like:
@qwen-code /review
Focus on security
== '@qwen-code /review'→ false (body has more text)startsWith(body, '@qwen-code /review ')→ false (char after/reviewis\n, not
The job is silently skipped — no notification, no fallback comment. This affects all three comment-based triggers (lines 46, 53, 60).
Suggested change
| startsWith(github.event.comment.body, '@qwen-code /review ')) && | |
| startsWith(github.event.comment.body, '@qwen-code /review ') || | |
| startsWith(github.event.comment.body, '@qwen-code /review\n')) && |
Apply the same fix at lines 53 and 60 for pull_request_review_comment and pull_request_review triggers.
— qwen3.7-max via Qwen Code /review
The `startsWith(body, '@qwen-code /review ')` check requires a trailing space after `/review`. When a user writes a multi-line comment like: @qwen-code /review Focus on security the character after `/review` is `\n`, not ` `, so the job was silently skipped. Use `format()` to construct a newline-containing string for an additional `startsWith` check on all three comment-based triggers. Resolves review feedback from wenshao.
Collaborator
Author
|
Fixed the multi-line trigger issue in 240ba91 — added When a user writes: The |
wenshao
reviewed
May 29, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
R5 Review Summary
This round reviewed commit 240ba914e (previous: b33fe54f). The latest commit fixes the R4 Critical (multi-line trigger silently rejected). 9 agents ran in parallel; 24 raw findings deduplicated and verified to 3 high-confidence + 3 low-confidence Suggestions. No Critical findings.
Test plan gaps (not mappable to a diff line): The PR test plan (6 items) omits coverage for ready_for_review auto-trigger, pull_request_review_comment trigger, pull_request_review trigger, the multi-line comment fix (latest commit), and concurrency cancellation behavior. Consider adding these to the checklist.
— qwen3.7-max via Qwen Code /review
| type: 'number' | ||
|
|
||
| jobs: | ||
| review-pr: |
Collaborator
There was a problem hiding this comment.
[Suggestion] Missing github.repository guard in the job if: condition.
All other workflows in this repo (10+ files including ci.yml, release.yml, gemini-scheduled-pr-triage.yml, etc.) guard with github.repository == 'QwenLM/qwen-code' to prevent execution on forks. Without it, forks that inherit this workflow produce noisy failed runs.
Suggested change
| review-pr: | |
| if: |- | |
| github.repository == 'QwenLM/qwen-code' && | |
| (github.event_name == 'workflow_dispatch' || |
— qwen3.7-max via Qwen Code /review
| fail "timeout_minutes must not exceed the 60 minute job timeout" | ||
| fi | ||
|
|
||
| if ! PR_STATE="$(gh pr view "$PR_NUMBER" --repo "$REPO" --json state --jq '.state')"; then |
Collaborator
There was a problem hiding this comment.
[Suggestion] Two separate gh pr view API calls (here and line 187) fetch one field each. These can be combined into a single call to reduce API round-trips:
if ! PR_INFO="$(gh pr view "$PR_NUMBER" --repo "$REPO" --json state,isCrossRepository)"; then
fail "Failed to fetch PR #${PR_NUMBER} metadata."
fi
PR_STATE="$(echo "$PR_INFO" | jq -r '.state')"
IS_FORK="$(echo "$PR_INFO" | jq -r '.isCrossRepository')"— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
Two bugs in
Neither is blocking the |
wenshao
approved these changes
Jun 4, 2026
tanzhenxin
approved these changes
Jun 8, 2026
tanzhenxin
left a comment
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Approving for merge.
The Actions-level hardening here is solid: PR-controlled strings reach the shell via env: (no template injection), triggers are anchored and gated to OWNER/MEMBER/COLLABORATOR, fork PRs are skipped fail-closed, id-token: write is dropped, and the action is SHA-pinned.
Noting one accepted tradeoff for follow-up: for same-repo PRs the /review skill takes its worktree path (dependency install + build/test) under --approval-mode yolo, so the effective guardrail today is the member-only trigger gating plus the prompt constraints rather than a hard sandbox. Merging on that basis, with a tracked follow-up to add a hard control — a diff-only/lightweight mode for same-repo PRs, a restricted coreTools set, or a sandboxed runner — plus a workflow-level worktree cleanup step. The multiline-trigger job-level if also still needs the fromJSON('"\\n"') form to actually match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/reviewskill@qwen-code /reviewin PR comments, or automatically on PR open/reopen/ready-for-review for org membersworkflow_dispatchfor manual dry-run testing with configurable timeout/review --commentinterface with CI-specific lightweight, non-interactive safety instructions in the workflow promptKey design choices
qwenCLI invocation with| teefor real-time logs (notqwen-code-action)/reviewto a lightweight review path: no dependency install, deterministic analysis, build/test, autofix, or Agent 7--kill-after=10skeeps the review process bounded before the job timeoutqwenCLI is checked and its version is logged before reviewRUNNER_TEMP, mirrored to action logs through a checkedteepipeline, validated for non-empty output, and removed after the runSKILL.mdchanges in this PR; CI review mode can be introduced separately after broader validationFiles changed
.github/workflows/qwen-code-pr-review.yml— new workflowTest plan
workflow_dispatchdry-run on a small PR@qwen-code /reviewcomment trigger posts inline comments