Repository navigation
refactor(ci): split PR triage into 4-job pipeline - #4866
Closed
yiliang114 wants to merge 50 commits into
Closed
yiliang114 wants to merge 50 commits into
yiliang114 wants to merge 50 commits into
Conversation
Replace the monolithic /triage skill + standalone qwen-code-pr-review.yml with a single qwen-pr-triage.yml workflow that orchestrates 4 jobs: 1. product-decision — template check + direction + approach (ubuntu-latest) 2. review — code review via /review skill (self-hosted, parallel) 3. tmux-testing — real-scenario testing (self-hosted, parallel, internal only) 4. approval-decision — reads all verdicts, approves/rejects (ubuntu-latest) Key design decisions: - Fork PRs run full pipeline except tmux-testing (no code execution) - settings_json restricts commands per job (gh/curl only, no general shell) - tmux-testing has no write token; approval-decision posts on its behalf - Each job posts comments with unique markers for dedup on re-run - qwen-triage.yml stripped to issue-only (PR triggers removed) - qwen-code-pr-review.yml deleted (folded into review job) Refs: #4570
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the repo’s GitHub Actions automation by separating issue triage from PR triage, and replacing the prior monolithic PR review workflow with a new multi-stage PR triage pipeline (resolve → product-decision → review/tmux in parallel → approval-decision).
Changes:
- Added two new Qwen skills to gate PRs on product alignment (
product-decision) and to synthesize a final approve/request-changes decision (approval-decision). - Updated
qwen-triage.ymlto become issue-only (removed PR triggers/logic). - Introduced a new
qwen-pr-triage.ymlworkflow to orchestrate the 4-job PR pipeline and deleted the oldqwen-code-pr-review.yml.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
.qwen/skills/product-decision/SKILL.md |
New skill definition for template + direction + approach gating before code review/testing. |
.qwen/skills/approval-decision/SKILL.md |
New skill definition for final synthesis and PR approve/request-changes action. |
.github/workflows/qwen-triage.yml |
Renamed and constrained existing triage workflow to issues only. |
.github/workflows/qwen-pr-triage.yml |
New orchestrating PR triage workflow with parallel review and tmux-testing stages. |
.github/workflows/qwen-code-pr-review.yml |
Removed the previous standalone PR review workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
actionlint flagged github.event.comment.body in inline script as injection risk. Move all event context fields to env vars.
/triage now delegates PR work to /product-decision, /review, tmux-real-user-testing, and /approval-decision sequentially. This aligns local behavior with the CI pipeline (qwen-pr-triage.yml). Old 3-stage inline logic removed from pr-workflow.md.
tmux-testing now needs review to pass first — no point building and running the app if code review found critical issues. Flow is now: product-decision → review → tmux-testing → approval-decision Each step gates the next. Also updates pr-workflow.md to reflect the serial dependency chain.
Skills now write verdict JSON to /tmp/triage-results/<skill>.json. The workflow reads the file after the action completes and emits the verdict as a job output via $GITHUB_OUTPUT. Fallback: if the file doesn't exist (skill crashed or didn't follow instructions), verdict is inferred from the action's exit code.
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. |
Two fixes: 1. After review runs, check if it posted REQUEST_CHANGES. If so, set verdict=fail and skip tmux-testing (saves runner time). 2. Add environment protection rule (qwen-pr-review-delay) to the review job for auto-triggered PRs. This reuses the existing 10-minute wait timer to debounce rapid consecutive pushes. Manual triggers (@qwen-code /review, workflow_dispatch) skip it.
P1 fixes:
- Add synchronize/reopened/review_requested triggers (parity with old workflow)
- Fix environment empty-string issue — move delay logic to resolve job output
- Fix review verdict detection — filter only by bot login, not authorAssociation
- Add fallback-comment job for pipeline failure notification
P2 fixes:
- Replace || true with proper exit code handling (distinguish timeout vs error)
- Fix qwen-triage.yml to use env vars instead of direct ${{ }} interpolation
P3 fixes:
- Update stale "parallel" comment to reflect serial pipeline
- resolve job: add explicit read-only permissions (least privilege) - tmux-testing: emit verdict=fail/timeout on CLI crash instead of always pass - approval-decision skill: fix TMUX_COMMENT_BODY → read from downloaded artifact file, remove ambiguous var name - product-decision skill: clarify fail is ONLY for template check, direction concerns always escalate to needs_human
…ted runner - review: job timeout 45→90m, inner qwen timeout 35→85m - push→review debounce: comment updated to 1h (env wait timer set to 60 via API) - product-decision: move to ecs-qwen self-hosted runner and call the local qwen CLI instead of qwen-code-action — reuses the runner's preinstalled qwen, no reinstall per run - product-decision: pass CLAUDE_CODE_SRC (from vars.CLAUDE_CODE_SRC_PATH) so the skill can read local Claude Code source for product-direction reasoning - product-decision skill: read local $CLAUDE_CODE_SRC (CHANGELOG + src) with a remote CHANGELOG fallback when the var is unset - remove dead Cache node_modules step from review job (no npm ci ever runs)
…te in AGENTS.md Q6 — readable CI logs without Ink/tmux overhead: - add shared env STREAM_JSON_FMT: a jq program that renders qwen --output-format stream-json events into one-line progress (tool calls, assistant text, final result) instead of raw JSON envelopes - pipe product-decision / review / tmux-testing through `| tee raw.jsonl | jq -Rr "$STREAM_JSON_FMT"`, keeping PIPESTATUS[0] for the qwen exit code and a raw .jsonl for debugging - the non-interactive -p path never mounts Ink, so this avoids the full TUI render cost a tmux capture would incur Q7 — stop PRs tripping the triage template gate after creation: - AGENTS.md "Submitting PRs": instruct agents to read .github/pull_request_template.md and fill it section-for-section via --body-file before opening a PR, and list the required sections, noting product-decision posts CHANGES_REQUESTED if any are missing
- product-decision: clear stale /tmp/triage-results/product-decision.json before the run. /tmp persists on the self-hosted runner (unlike the previous ephemeral ubuntu-latest), so the capture step could otherwise read a verdict left by a previous PR if this run's skill fails to write one. - STREAM_JSON_FMT: wrap the per-line render in try/catch and null-guard subtype/name, so one unexpected event shape can't error out jq mid-stream and blank the rest of the log. - product-decision / review / tmux-testing: fall back to raw passthrough (cat) when jq is absent on the runner, so log output is never silently blanked.
…ch dirs Local review round 2 findings: - product-decision: propagate the qwen exit code at the end of the run step. Previously a timeout/crash left the step green, and when no verdict file was written the capture fallback defaulted to "pass" — silently waving the PR through the gate. Now the job fails and the verdict resolves to fail, matching the original action-based semantics. - jq: add --unbuffered so progress lines stream into the CI log in real time instead of arriving in block-buffered chunks. - Move runner scratch state from /tmp to runner.temp, which is emptied per job and is per runner instance: triage-results verdict dir (via TRIAGE_RESULTS_DIR env, skill falls back to /tmp locally), raw stream-json tee files, and the tmux-results artifact dir. /tmp persists across runs and is shared between runner instances on the same host, risking stale or cross-PR contamination. The approval-decision side stays on /tmp (ephemeral ubuntu-latest VM).
It now runs the local qwen CLI on the same self-hosted runner as the review and tmux-testing stages, so it should use the same credential pair instead of the action-era OPENAI_* secrets.
The old qwen-code-pr-review.yml posted an immediate '<!-- qwen-review-ack -->' comment when a maintainer requested a review, updated in place on re-trigger. That feedback was lost in the pipeline split — a comment trigger gave no signal that the request was accepted. Add a lightweight ack-request job (no needs, fires instantly) mirroring resolve's comment-trigger gate.
The old qwen-code-pr-review.yml accepted @qwen-code /review from PR review threads (pull_request_review_comment) and review summaries (pull_request_review), not just the main conversation. Restore both: on: triggers, resolve gate + PR-number/trigger-type resolution, and the ack-request acknowledgement. Manual triggers still skip the debounce delay.
Port of the old delay job's post-wait re-check: with a 1-hour debounce the PR may be closed, merged, or drafted by the time the review job starts. Skip checkout/review in that case (verdict=skip) and gate tmux-testing and approval-decision off it, instead of burning an 85-minute runner slot on a dead PR.
- Auto-triggered runs (pull_request_target) again require the PR author to be OWNER/MEMBER/COLLABORATOR, as the old workflow did. External/fork PRs run only on explicit maintainer trigger; the gate can be relaxed later. - workflow_dispatch regains the timeout_minutes input (default 90, min 10). The review job timeout and the inner qwen timeout (N-5) derive from it.
ack-request already guards on state == open; resolve didn't, so a /review comment on a closed PR would still launch the pipeline. Align the gates.
…factor # Conflicts: # .github/workflows/qwen-code-pr-review.yml
PR #4962 extends the old qwen-code-pr-review.yml timeouts (review 60->90min, delay comment 10->30min). That file is deleted here — its intent is already implemented in the qwen-pr-triage.yml pipeline (review timeout 90min with a workflow_dispatch override). Resolved the modify/delete conflict by keeping the deletion; merging the branch lets #4962 auto-close as merged when this PR lands.
The qwen-pr-review-delay environment wait timer is set to 30 minutes (per #4962); update the stale 1-hour comments to match.
wenshao
requested changes
Jun 15, 2026
| if: >- | ||
| github.repository == 'QwenLM/qwen-code' && ( | ||
| (github.event_name == 'pull_request_target' && | ||
| github.event.pull_request.draft == false) || |
Collaborator
There was a problem hiding this comment.
[Critical] The pull_request_target branch of the resolve job's if only checks draft == false but is missing two guards that the old qwen-code-pr-review.yml had:
-
state == 'open'— without this,synchronizeorreopenedevents on a merged/closed PR pass the gate.product-decisionruns (up to 15 min on self-hosted) and may post a comment on a closed PR before the review stage's debounce re-check catches it. -
author_association— the old workflow requiredOWNER || MEMBER || COLLABORATORfor auto-triggered reviews. Without this, every non-draft PR from any GitHub user (including first-time external contributors) triggers the full pipeline, consuming runner capacity and model API credits on untrusted content. -
review_requestedfiltering — the old workflow had anauthorize-review-requestjob that checkedrequested_reviewer.login == bot_loginand sender write permission. Now anyreview_requestedevent (even for human reviewers) fires the full pipeline.
Suggested change
| github.event.pull_request.draft == false) || | |
| github.event.pull_request.state == 'open' && | |
| github.event.pull_request.draft == false && | |
| (github.event.pull_request.author_association == 'OWNER' || | |
| github.event.pull_request.author_association == 'MEMBER' || | |
| github.event.pull_request.author_association == 'COLLABORATOR') && | |
| (github.event.action != 'review_requested' || | |
| github.event.requested_reviewer.login == 'qwen-code-ci-bot')) || |
— qwen3.7-max via Qwen Code /review
| timeout-minutes: 15 | ||
| concurrency: | ||
| group: 'qwen-pr-triage-pipeline-${{ needs.resolve.outputs.pr_number }}' | ||
| cancel-in-progress: true |
Collaborator
There was a problem hiding this comment.
[Critical] All four downstream jobs share the concurrency group qwen-pr-triage-pipeline-${pr_number} with cancel-in-progress: true. The old workflow deliberately used different concurrency groups:
# OLD: PR lifecycle events share a PR-scoped group; comment/review events use per-run groups.
group: >-
${{ github.event_name == 'pull_request_target' &&
format('qwen-pr-review-pr-{0}', github.event.pull_request.number) ||
format('qwen-pr-review-run-{0}', github.run_id) }}
cancel-in-progress: "${{ github.event_name == 'pull_request_target' && github.event.action == 'synchronize' }}"With the current setup, a maintainer's @qwen-code /review comment cancels an in-progress review that may have already waited 30 min debounce + spent significant model API budget. Under active PR discussion, the review may never complete.
Scope cancel-in-progress to synchronize events only:
Suggested change
| cancel-in-progress: true | |
| cancel-in-progress: "${{ github.event_name == 'pull_request_target' && github.event.action == 'synchronize' }}" |
— qwen3.7-max via Qwen Code /review
| OPENAI_MODEL: '${{ vars.QWEN_PR_REVIEW_MODEL }}' | ||
| settings_json: |- | ||
| { | ||
| "coreTools": [ |
Collaborator
There was a problem hiding this comment.
[Critical] The approval-decision agent runs with a write-capable PAT (QWEN_CODE_BOT_TOKEN || CI_BOT_PAT) in both GITHUB_TOKEN and GH_TOKEN, while coreTools includes run_shell_command(gh api) — a general-purpose GitHub API client that can call ANY endpoint. Combined with --approval-mode yolo, all tool calls are auto-approved. The agent processes untrusted PR content (title, body, review comments), creating a transitive prompt-injection surface.
Both product-decision and review carefully separate analyze/publish to keep the write PAT away from the agent; approval-decision undoes that protection in the final and most consequential stage.
A crafted PR body could manipulate the agent into calling gh api to dismiss existing reviews, modify labels, or take other write actions.
Suggested fix: Apply the same analyze/publish split: run the agent with a read-only token, have it emit the verdict to a file, and publish in a separate no-agent step with the write PAT. At minimum, narrow run_shell_command(gh api) to specific endpoints the skill actually needs, and add explicit deny rules for gh api -X DELETE and gh api graphql.
— qwen3.7-max via Qwen Code /review
|
|
||
| set +e | ||
| timeout --kill-after=10s "${INNER_TIMEOUT_MIN}m" "${QWEN_CMD[@]}" \ | ||
| --prompt "/review ${REVIEW_URL} --comment" \ |
Collaborator
There was a problem hiding this comment.
[Critical] The old review-pr job validated the qwen result stream event after process exit (~20 lines checking is_error, subtype == "success", and absence of [API Error in the result text). The new "Run review" step only checks exit code and whether the output JSON file exists with valid structure (type == "object" and .event is a string).
If the agent exits 0 but the model aborted mid-analysis (e.g., API connection dropped after writing a partial JSON), the file may carry event: "COMMENT" with an error-laced body or event: "APPROVE" based on incomplete analysis. The publish step submits it as-is. The PR receives a broken-looking review and the workflow shows green.
Suggested fix: After the qwen invocation, inspect the raw stream log for error indicators:
RESULT_LINE="$(grep '"type":"result"' "$RUNNER_TEMP/review-raw.jsonl" | tail -n1 || true)"
if [ -n "$RESULT_LINE" ]; then
RESULT_IS_ERROR="$(printf '%s' "$RESULT_LINE" | jq -r '.is_error // false')"
RESULT_SUBTYPE="$(printf '%s' "$RESULT_LINE" | jq -r '.subtype // ""')"
if [ "$RESULT_IS_ERROR" = "true" ] || [ "$RESULT_SUBTYPE" != "success" ]; then
write_review_unavailable "review agent ended in error state (subtype=${RESULT_SUBTYPE})"
exit 0
fi
fi— qwen3.7-max via Qwen Code /review
| (github.event_name == 'issue_comment' && | ||
| github.event.issue.pull_request && | ||
| github.event.issue.state == 'open' && | ||
| (startsWith(github.event.comment.body, '@qwen-code /triage') || |
Collaborator
There was a problem hiding this comment.
[Suggestion] Trigger matching was loosened from three-way exact match to bare startsWith. The old workflow matched @qwen-code /review with three conditions:
body == '@qwen-code /review' ||
startsWith(body, '@qwen-code /review ') ||
startsWith(body, format('@qwen-code /review{0}', '\n'))The new workflow collapses this to startsWith(body, '@qwen-code /review'), which also matches @qwen-code /reviewers-needed, @qwen-code /reviewing this, etc. The same loosening applies to /triage.
Restore the three-way check for precise command boundary matching:
Suggested change
| (startsWith(github.event.comment.body, '@qwen-code /triage') || | |
| (github.event.comment.body == '@qwen-code /triage' || | |
| startsWith(github.event.comment.body, '@qwen-code /triage ') || | |
| startsWith(github.event.comment.body, format('@qwen-code /triage{0}', '\n')) || | |
| github.event.comment.body == '@qwen-code /review' || | |
| startsWith(github.event.comment.body, '@qwen-code /review ') || | |
| startsWith(github.event.comment.body, format('@qwen-code /review{0}', '\n'))) && |
— qwen3.7-max via Qwen Code /review
| outputs: | ||
| verdict: '${{ steps.capture.outputs.verdict }}' | ||
| steps: | ||
| - name: 'Clean stale review worktrees' |
Collaborator
There was a problem hiding this comment.
[Suggestion] The entire 14-line "Clean stale review worktrees" shell block (git worktree prune, git worktree list, awk pipeline, for-each-ref cleanup, rm -rf) is duplicated verbatim across three jobs (product-decision L248, review L488, tmux-testing L768).
Any fix to the cleanup logic must be applied in three places, and drift between copies is likely over time.
Suggested fix: Extract into a reusable composite action under .github/actions/clean-worktrees/action.yml, or define the script as a workflow-level env var (like STREAM_JSON_FMT) and reference it from each job.
— qwen3.7-max via Qwen Code /review
| # via environment protection rule to debounce rapid pushes. | ||
| # Manual triggers skip the delay. | ||
| if [ "$TRIGGER_TYPE" = "full" ] && [ "$EVENT_NAME" = "pull_request_target" ]; then | ||
| REVIEW_ENV="qwen-pr-review-delay" |
Collaborator
There was a problem hiding this comment.
[Suggestion] The debounce delay environment (qwen-pr-review-delay) is applied to ALL pull_request_target events with trigger_type == "full", but the inline comment says "Auto-triggered PRs (opened/synchronize) get a 30-minute delay." The condition matches review_requested, reopened, and ready_for_review in addition to opened/synchronize.
In the old workflow, only opened and synchronize went through the delay environment; review_requested bypassed it entirely (the old delay-automatic-review job's if gated on action == 'opened' || action == 'synchronize').
This means explicit review requests now wait 30 minutes before starting — a UX regression for maintainers requesting bot review.
Suggested fix: Restrict the delay to opened and synchronize actions:
PRT_ACTION="${{ github.event.action }}"
if [ "$TRIGGER_TYPE" = "full" ] && [ "$EVENT_NAME" = "pull_request_target" ] \
&& { [ "$PRT_ACTION" = "opened" ] || [ "$PRT_ACTION" = "synchronize" ]; }; then
REVIEW_ENV="qwen-pr-review-delay"
else
REVIEW_ENV=""
fi— qwen3.7-max via Qwen Code /review
| gh pr view "$PR_NUMBER" --repo "$REPO" --json number,title,body,author,labels,additions,deletions,changedFiles,baseRefName,headRefName,isCrossRepository,isDraft,url | ||
| ``` | ||
|
|
||
| If draft → exit with verdict `skip`. |
Collaborator
There was a problem hiding this comment.
[Suggestion] Step 1 says "If draft → exit with verdict skip" but Step 7 "Possible verdict values" only lists pass, fail, and needs_human. The skip verdict is undocumented in Step 7.
While the CI workflow's resolve.if already filters out drafts (draft == false), for local invocation the skill could emit verdict=skip, which nothing downstream handles — it would behave like fail (blocking the pipeline) since the review gate only checks verdict == 'pass'.
Suggested fix: Either add skip to the Step 7 verdict values list with a description ("draft PR, no action taken"), or remove the skip branch from Step 1 and have the skill simply exit without writing a verdict file for drafts.
— qwen3.7-max via Qwen Code /review
| fi | ||
|
|
||
| # Check if fork | ||
| IS_FORK=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json isCrossRepository --jq '.isCrossRepository') |
Collaborator
There was a problem hiding this comment.
[Suggestion] The gh pr view call for the fork check (isCrossRepository) has no retry logic, while other steps that call gh pr view (review re-check, publish review) use 3-attempt retry with backoff. The resolve step is the single point of entry for the entire pipeline — a transient GitHub API failure here kills all downstream stages and triggers the fallback comment.
Suggested fix: Add retry with backoff matching the pattern used in other steps:
for attempt in 1 2 3; do
if IS_FORK=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json isCrossRepository --jq '.isCrossRepository'); then
break
fi
if [ "$attempt" -eq 3 ]; then
echo "::error::Failed to check fork status after 3 attempts"
exit 1
fi
sleep $((attempt * 10))
done— qwen3.7-max via Qwen Code /review
- approval-decision Publish: wrap the formal review and comment posts in a retry, and submit the formal review BEFORE the explanatory comments. A transient gh failure no longer fails the step after a misleading comment is already posted; the formal review is load-bearing and fails closed, while comments are best-effort. - Set persist-credentials: false on all four checkouts. On the self-hosted persistent runners the agents read untrusted PR content, so the (read-only) GITHUB_TOKEN should not be left in .git/config where an injected prompt could read it.
wenshao
reviewed
Jun 15, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
2 Critical findings:
-
.github/workflows/qwen-pr-triage.yml:1071— tmux-testing "Run tmux testing" step is missingGH_TOKENandGH_CONFIG_DIRcredential isolation (see inline comment). -
.qwen/skills/triage/SKILL.md:46— Stale marker format: references<!-- qwen-triage stage=N -->but all stages now use<!-- qwen-triage:product -->,<!-- qwen-triage:review -->, etc. The duplicate guard will never match, causing duplicate comments on/triagere-runs. Fix: change to<!-- qwen-triage:* -->.
4 Suggestions posted as inline comments below (BOT_LOGIN empty fallback, tmux exit validation, fallback stage identification, coreTools missing jq).
Additional findings (terminal only):
- Nice to have:
configure_qwen_network()(~60 lines) and worktree cleanup (~12 lines) duplicated 3x across jobs (overlaps with existing comment at line 328). - Nice to have: approval-decision job has no PR state re-check (review and tmux-testing both re-verify PR is open).
- Needs Human Review (low confidence): Stale verdict files in
runner.temp/triage-results/from cancelled runs could be read by the next run's publish step. - Needs Human Review (low confidence): Review fallback file search uses
find | head -1(non-deterministic when multiple files exist).
— qwen3.7-max via Qwen Code /review
| if: "steps.pr_state.outputs.should_test == 'true'" | ||
| id: 'run' | ||
| env: | ||
| OPENAI_API_KEY: '${{ secrets.REVIEW_OPENAI_API_KEY }}' |
Collaborator
There was a problem hiding this comment.
[Critical] tmux-testing "Run tmux testing" step is missing GH_TOKEN and GH_CONFIG_DIR isolation.
The product-decision (line 368+396) and review (line 684+710) jobs both set GH_TOKEN: secrets.QWEN_REVIEW_READ_TOKEN and export GH_CONFIG_DIR="$RUNNER_TEMP/gh-readonly-config" to isolate the agent from ambient credentials on the persistent self-hosted runner. The tmux-testing step omits both.
This is the highest-risk stage — it checks out and executes untrusted PR merge code (refs/pull/N/merge) with --approval-mode yolo on a persistent runner. Without credential isolation, a prompt injection in the PR under test could inherit the runner's ambient gh auth (from ~/.config/gh/hosts.yml if gh was ever authenticated under the runner service user), defeating the analyze/publish split that the other three stages enforce.
Add to the env block:
GH_TOKEN: '${{ secrets.QWEN_REVIEW_READ_TOKEN }}'And add at the start of the run: block (before configure_qwen_network):
export GH_CONFIG_DIR="$RUNNER_TEMP/gh-readonly-config"
mkdir -p "$GH_CONFIG_DIR"— qwen3.7-max via Qwen Code /review
| --paginate \ | ||
| -F per_page=100 \ | ||
| | jq -sr --arg bot "$BOT_LOGIN" \ | ||
| '[.[][] | select(.body | contains("<!-- qwen-triage:product -->")) | select($bot == "" or .user.login == $bot)] | last | .id // empty' |
Collaborator
There was a problem hiding this comment.
[Suggestion] When BOT_LOGIN is empty (transient gh api user failure), the jq filter select($bot == "" or .user.login == $bot) simplifies to select(true) — matching ANY user's comment with the <!-- qwen-triage:product --> marker, not just the bot's. This could overwrite a maintainer's comment.
The approval-decision's publish_marker_comment function (line ~1334) correctly guards with if [ -n "$BOT_LOGIN" ] and skips the dedup query entirely when the login is empty. Align this publish step with that pattern:
EXISTING=""
if [ -n "$BOT_LOGIN" ]; then
EXISTING="$(
gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \
--method GET --paginate -F per_page=100 \
| jq -sr --arg bot "$BOT_LOGIN" \
'[.[][] | select(.body | contains("<!-- qwen-triage:product -->")) | select(.user.login == $bot)] | last | .id // empty'
)" || EXISTING=""
fi— qwen3.7-max via Qwen Code /review
| echo "::warning::Tmux testing exited with code $EXIT_CODE" | ||
| echo "verdict=fail" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "verdict=pass" >> "$GITHUB_OUTPUT" |
Collaborator
There was a problem hiding this comment.
[Suggestion] tmux-testing maps exit code 0 directly to verdict=pass without validating the result stream. The review job (lines ~831-848) performs detailed post-exit validation on exit 0 — checking the type:"result" event for is_error, subtype == "success", and [API Error patterns. tmux-testing skips all of this.
The qwen process can exit 0 even when the skill failed internally (session turn limit, caught API error, incomplete testing). A false verdict=pass would let approval-decision proceed and potentially approve a PR without real tmux validation.
Consider adding result stream validation similar to the review job — at minimum check that a terminal type:"result" event exists and has subtype == "success" before emitting verdict=pass.
— qwen3.7-max via Qwen Code /review
| run: | | ||
| gh pr comment "$PR_NUMBER" \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --body "_Qwen triage pipeline did not complete successfully. See [workflow logs]($RUN_URL)._" |
Collaborator
There was a problem hiding this comment.
[Suggestion] The fallback comment doesn't identify which stage failed. At 3 AM, a maintainer sees "did not complete successfully" and must navigate to the workflow logs to determine the root cause.
The job has access to all upstream results via needs. Include the failed stage name(s):
FAILED_JOBS=""
[[ "${{ needs.product-decision.result }}" == "failure" ]] && FAILED_JOBS+="product-decision "
[[ "${{ needs.review.result }}" == "failure" ]] && FAILED_JOBS+="review "
[[ "${{ needs.tmux-testing.result }}" == "failure" ]] && FAILED_JOBS+="tmux-testing "
[[ "${{ needs.approval-decision.result }}" == "failure" ]] && FAILED_JOBS+="approval-decision "
gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \
--body "_Qwen triage failed at: ${FAILED_JOBS:-unknown stage}. See [workflow logs]($RUN_URL)._"— qwen3.7-max via Qwen Code /review
| "run_shell_command(gh pr view)", | ||
| "run_shell_command(gh api)", | ||
| "run_shell_command(mkdir)", | ||
| "run_shell_command(cat)", |
Collaborator
There was a problem hiding this comment.
[Suggestion] The coreTools allowlist is missing jq, but the approval-decision skill's code examples use piped jq commands in Steps 2 and 5 (comment dedup: gh api ... | jq -sr '...'). The PermissionManager splits compound commands at pipe boundaries and evaluates each sub-command independently — jq would be denied.
The agent can adapt (use --jq flag instead of pipe, or bash pattern matching), but this wastes turns under the 30-turn limit and increases the risk of incorrect dedup or failed stage result gathering.
Suggested change
| "run_shell_command(cat)", | |
| "coreTools": [ | |
| "run_shell_command(gh pr view)", | |
| "run_shell_command(gh api)", | |
| "run_shell_command(jq *)", | |
| "run_shell_command(mkdir)", | |
| "run_shell_command(cat)", | |
| "read_file" | |
| ], |
— qwen3.7-max via Qwen Code /review
DragonnZhang
approved these changes
Jun 15, 2026
DragonnZhang
left a comment
DragonnZhang
left a comment
Collaborator
There was a problem hiding this comment.
Clean CI refactor: splits the monolithic qwen-triage workflow into a 4-job pipeline (classify → product-decision → approval-decision → post-review). Each job has a focused responsibility with explicit needs dependencies. Triage skills updated with clearer instructions. CI green. LGTM ✅ — claude-opus-4-6 via Qwen Code /review
This was referenced Jun 16, 2026
| # Self-hosted persistent runner: keep the GITHUB_TOKEN out of | ||
| # .git/config (the review agent reads untrusted PR content). | ||
| persist-credentials: false | ||
| ref: '${{ github.event.repository.default_branch }}' |
Collaborator
There was a problem hiding this comment.
[Suggestion] The review job checks out ${{ github.event.repository.default_branch }} instead of the PR's actual base branch. For PRs targeting non-default branches (release branches, etc.), the review agent would operate on a different codebase than the PR targets. Product-decision (line 359) and approval-decision (line 1222) both use ${{ github.ref }} which correctly resolves to the PR's base.
Suggested change
| ref: '${{ github.event.repository.default_branch }}' | |
| ref: '${{ github.event.pull_request.base.ref }}' |
— qwen3-235b-a22b via Qwen Code /review
yiliang114
marked this pull request as draft
June 29, 2026 15:29
Collaborator
|
@qwen-code /resolve |
Collaborator
|
Qwen Code did not run conflict resolution for this request. PR #4866 is draft. |
Collaborator
|
@qwen-code /resolve |
Collaborator
|
Qwen Code did not run conflict resolution for this request. PR #4866 is draft. |
Collaborator
|
@qwen-code /resolve |
Collaborator
|
Qwen Code did not run conflict resolution for this request. PR #4866 is draft. |
Collaborator
|
@qwen-code /resolve |
Collaborator
|
Qwen Code did not run conflict resolution for this request. PR #4866 is draft. |
Collaborator
|
@qwen-code /resolve |
Collaborator
|
Qwen Code did not run conflict resolution for this request. PR #4866 is draft. |
Collaborator
Author
|
Closing this PR. The branch has fallen 266 commits behind main and the CI pipeline needs a redesign rather than a rebase. Key learnings that will carry forward:
The replacement work will be tracked in #4786 as part of a broader CI consolidation, using composite actions to keep each workflow under ~200 lines and skills focused on judgment logic rather than CI orchestration. |
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.
What this PR does
Replaces the monolithic
/triageskill + standaloneqwen-code-pr-review.ymlwith a singleqwen-pr-triage.ymlthat orchestrates a staged pipeline, and narrowsqwen-triage.ymlto issues only (qwen-code-pr-review.ymlis deleted).Each stage gates the next — a failed gate stops the pipeline there, and the Actions DAG view shows exactly which stage a PR is sitting at.
New skills:
/product-decision(PR-template + product-direction gate) and/approval-decision(final verdict).The three reasoning stages run on the self-hosted
ecs-qwenrunner using its preinstalledqwenCLI (no per-run install via the action):is_fork == false)product-decisionreads a local Claude Code checkout on the runner (CLAUDE_CODE_SRC←vars.CLAUDE_CODE_SRC_PATH) for direction signals, falling back to the remote CHANGELOG when unset. Its qwen exit code propagates, and a missing verdict JSON fails the job instead of defaulting the verdict to pass.Exactly one stage owns the formal review verdict on every path: template fail → product-decision posts request-changes; review finds blockers → the
/reviewCHANGES_REQUESTED is the verdict (tmux + approval skip); all pass → approval-decision posts the single final verdict. No path produces competing formal reviews.Least privilege: product-decision, review, and approval-decision all run their qwen agent with
QWEN_REVIEW_READ_TOKENand emit files; separate deterministic publish steps holdCI_BOT_PATand post comments/reviews.A crashed or timed-out review, a zero-exit review whose stream result reports an API error, or a run that emits no review JSON is converted into a
review-unavailablegate result:Publish reviewposts or updates a regular PR comment carrying the<!-- qwen-triage:review-unavailable -->marker,Check review verdictemitsfail, and tmux-testing / approval-decision skip. Fatal runner/configuration errors still fail the workflow, so a network blip is never mistaken for a passing review.The bundled
/reviewskill now prewrites that emit-only fallback JSON before launching parallel review agents, then overwrites it with the real review JSON in Step 9 if review completes. This preserves full parallelism while ensuring CI has a deterministic gate result if the controller is interrupted during parallel-agent launch.The four stage jobs share one per-PR concurrency group. A new push (
pull_request_target.synchronize) cancels the whole in-flight pipeline (not just the same stage) and restarts the debounce clock; explicit comment/review triggers queue without cancelling an active run.ECS cross-border network resilience: gh API calls in the review job retry with backoff; checkout low-speed abort/retry handling is not part of this PR.
CI logs:
qwen --output-format stream-jsonis piped through a sharedjq --unbufferedformatter into readable one-line progress (tool calls / narration / result) instead of raw JSON, keeping a raw.jsonland falling back tocatifjqis absent.Runner scratch state (verdict files, raw logs, tmux artifact dir) lives under
runner.temp— emptied per job and per runner instance — instead of shared persistent/tmp.Automatic runs are gated to internal authors (OWNER/MEMBER/COLLABORATOR), as before the split — external/fork PRs run only on an explicit maintainer trigger. The queued-acknowledgement comment, review-thread/review-body triggers, post-debounce PR state re-check, and the
timeout_minutesdispatch input (default 90) are all carried over from the old workflow.AGENTS.md: front-loads the PR-template requirement so agents fill it before opening a PR.Why it's needed
The single-run workflow conflated product judgment, code review, and testing into one opaque pass. Staged gates let each stage block early, run with least privilege, and be triggered/retried independently. Running on the self-hosted runner with the preinstalled CLI avoids repeated installs and lets
product-decisionreason against local source. Rawstream-jsonlogs were unreadable — thejqformatter makes runs debuggable without the Ink TUI cost a tmux capture would add. Front-loading the template stops PRs from being auto-flagged by the new gate right after creation.Reviewer Test Plan
How to verify
workflow_dispatchon an open internal PR → confirm the serial DAGresolve → product-decision → review → tmux-testing → approval-decision(this also works pre-merge:gh workflow run qwen-pr-triage.yml --ref <this branch> -f pr_number=<N>uses the branch's workflow file).product-decisionposts a comment carrying the<!-- qwen-triage:product -->marker; a PR missing template sections getsCHANGES_REQUESTED.▶ tool/💬 text/■ resultlines streaming in real time (not raw JSON).@qwen-code /reviewcomment trigger skipsproduct-decisionand runs review only;@qwen-code /triageruns the full pipeline; both skip the debounce wait and get an immediate "queued" ack comment.qwen-pr-review-delayenvironment, aligned with ci: extend qwen PR review timeout to 90min and queue delay to 30min #4962).Evidence (Before & After)
Not user-visible (CI workflow + skills). Log readability: before = raw
stream-jsonenvelopes; after = one readable line per event. Locally validated: YAML parses, yamllint + actionlint clean (custom runner label aside),act -grenders the expected serial DAG, and theresolvescript was executed directly against the live API for all three trigger paths (workflow_dispatch→ full,@qwen-code /review→ review_only + ack,@qwen-code /triage→ full + ack) with correct outputs.Real runner evidence:
workflow_dispatchcoverage before the latest commits: feat(telemetry): add runtime memory/CPU sampling with OTel metric reporting #4868 (27357866887) ran product-decision + review and requested changes; feat(core): add enter_plan_mode tool and Plan Approval Gate #4853 (27398163036) stopped atneeds_human; feat(telemetry): add runtime memory/CPU sampling with OTel metric reporting #4868 (27404880031) skipped after merge.27498590401) passed checkout, failed product-decision on missing template, and skipped downstream; fix(ci): fail PR review job when the run aborts mid-review #5053 (27498591039) and feat(acp): dedicated agent permission dialog via _meta.toolName (follow-up to #5085) #5105 (27498590774) passed product-decision, reached review, then hit[API Error: terminated]before review JSON output. Fallback comments posted successfully.64a31afc: fix(ci): fail PR review job when the run aborts mid-review #5053 (27498894870) verified merged-PR skip; refactor(ci): split PR triage into 4-job pipeline #4866 (27498933437) verified draft-PR skip; feat(core,cli,sdk): resume an interrupted turn without a synthetic "continue" message #5030 (27498982518) reproduced the missing-review-JSON failure before the review-unavailable gate.55ad9e1: feat(core,cli,sdk): resume an interrupted turn without a synthetic "continue" message #5030 (27499848555) reproduced[API Error: terminated]/ no review JSON, published/updated thereview-unavailablePR comment, emitted review verdictfail, skipped tmux-testing / approval-decision / fallback-comment, and completed successfully; docs(channels): add screenshots to Feishu setup guide #4983 (27499721709) re-verified the full product-decision fail gate on the same branch.e409461d9: merged currentmain, resolved the only conflict by keeping the intended deletion ofqwen-code-pr-review.yml, and re-verified the review-unavailable path on feat(core,cli,sdk): resume an interrupted turn without a synthetic "continue" message #5030 (27501206435). The run timed out review after 5 minutes, wrote the fallback JSON, updated the existingreview-unavailablecomment (4701872011) instead of creating a duplicate, emitted review verdictfail, skipped tmux-testing / approval-decision / fallback-comment, and completed successfully. PR CI (27501191023) passed Classify PR, Lint, CodeQL, Ubuntu tests, macOS tests, Windows tests, and coverage comment.70dc200d4: addressed review follow-ups for trigger boundary matching,review_requestedbot/requester gating, debounce/cancellation scope, review stream-result API-error gating, fork-check retry, documentedskip, and approval-decision's read-only agent + publish split. Local validation:git diff --check, YAML parse,actionlint, and targetedbash -nchecks passed.89e2d9496: tightened the remaining default/product/reviewGITHUB_TOKENpermissions to read-only and made product/approval comment dedup paginate marker lookups. Local validation:git diff --check, YAML parse,actionlint, targetedbash -n, and a marker-dedupjqsmoke check passed. PR CI (27522341719) passed Classify PR, Lint, CodeQL, macOS tests, Ubuntu tests, Windows tests, and coverage comment.12070a30c: required every non-skipproduct-decision verdict to emit its matching product comment body, so the product gate cannot silently continue without the marker/comment input used by downstream stages. Local validation:git diff --check, YAML parse,actionlint, and direct product verdict-capture smoke checks for pass+body, pass-without-body, and skip-without-body passed. PR CI (27525183653) passed Classify PR, Lint, CodeQL, Ubuntu tests, macOS tests, Windows tests, and coverage comment.0610d61a6: hardened the latest local-review findings: provisionalreview-unavailableoutput is replaced by a real.qwen/tmpreview JSON when one exists, tmux PR-state checks now retry with backoff, approval-decision only runs after tmux-testing produced a realpass/fail/timeoutverdict, and the local/bundled skill docs now match that gate behavior. Local validation:git diff --check, YAML parse,actionlint, review fallback-replacement smoke,npm ci(including build/bundle), andnpm run typecheckpassed. Current PR CI (27526661250) is in progress: Classify PR passed; Lint, CodeQL, Ubuntu/macOS/Windows tests were still running at update time.Tested on
Environment (optional)
Self-hosted
ecs-qwenrunners must haveqwenCLI,jq, andtmuxpreinstalled;vars.CLAUDE_CODE_SRC_PATHshould point at a local Claude Code checkout (the skill falls back to the remote CHANGELOG until set). Local validation usedact0.2.88 + direct script execution. Realworkflow_dispatchruns now cover resolve, product-decision pass/fail, checkout, review start, review-unavailable comment publish/update/dedup, fallback-comment skip for handled review-unavailable, and skip guards. The bundled/reviewprewrite fallback takes effect only after the runner's preinstalled qwen is updated to include this branch's skill change. The clean review-pass → tmux-testing → approval-decision path is still not covered because/review --commentcurrently aborts or times out during parallel-agent review before producing a real review JSON.Risk & Scope
product-decisionon self-hosted +--approval-mode yolodrops the action'ssettings_jsontool allowlist (a fork-PR defense-in-depth layer); this matchesreview's existing posture.product-decisionruns undebounced for fast template feedback — rapid pushes re-run it (each cancelled by the next), trading some ECS time for author experience./review --commentparallel-agent/API termination stability; settingvars.CLAUDE_CODE_SRC_PATHis runner configuration, not part of this PR.qwen-code-pr-review.ymlremoved (supersedes ci: extend qwen PR review timeout to 90min and queue delay to 30min #4962, whose content is carried here);qwen-triage.ymlno longer triages PRs (issues only); auto-review waits up to 30 min after a push.Linked Issues
None — CI infrastructure refactor.
中文说明
把原来「单体
/triageskill + 独立qwen-code-pr-review.yml」换成一个qwen-pr-triage.yml，编排成分阶段流水线：resolve → product-decision → review → tmux-testing → approval-decision（串行门控，每一阶段拦住则流水线就停在那里，Actions 的 DAG 视图能直接看出 PR 卡在哪一步）；qwen-triage.yml收窄为只处理 issue，qwen-code-pr-review.yml删除。/product-decision（PR 模板 + 产品方向门）、/approval-decision（最终判定）。ecs-qwenrunner 上、用其预装的qwenCLI（不再每次用 action 安装）：product-decision / review 只通过 API 读 PR，不执行代码；tmux-testing 仅对内部 PR（非 fork）执行代码。product-decision读取 runner 上的本地 Claude Code 源码（CLAUDE_CODE_SRC←vars.CLAUDE_CODE_SRC_PATH）做方向判断，未配置时回退到远程 CHANGELOG；其 qwen 退出码会向上传播，缺 verdict JSON 时也会失败，而不是默认放行。/review的 CHANGES_REQUESTED 即为判定（tmux 与 approval 跳过）；全部通过 → approval-decision 发唯一的最终判定。任何路径都不会出现互相竞争的 formal review。review-unavailable门控结果：Publish review发出或更新带<!-- qwen-triage:review-unavailable -->marker 的普通 PR 评论，Check review verdict输出fail，tmux-testing / approval-decision 跳过。真正的 runner/配置错误仍会让 workflow 失败，所以网络抖动绝不会被误判为 review 通过。/reviewskill 现在会在启动并发 review agents 之前预写这个 emit-only fallback JSON；如果 review 正常完成，Step 9 再用真实 review JSON 覆盖它。这样不降低并发，也能保证 controller 在并发 agent 启动/等待阶段被中断时，CI 仍有确定的门控结果。pull_request_target.synchronize）会取消整条进行中的旧流水线（而不只是同阶段）并重新开始防抖计时；显式评论/review 触发会排队，但不会取消正在运行的流水线。stream-json经共享jq --unbuffered实时渲染成可读单行（工具调用/叙述/结果），保留原始.jsonl，缺jq时回退cat。runner.temp（每个 job 清空、每实例独立），不再用共享持久的/tmp。timeout_minutes手动输入（默认 90）均已保留。AGENTS.md：前置 PR 模板要求，让 agent 建 PR 前就按模板填，避免建完即被 triage 打回。为什么： 分阶段门控可早阻断、最小权限、独立触发重试；用预装 CLI 省安装并能读本地源码；
jq渲染让日志可读又不付 Ink TUI 代价；前置模板避免被新门拦。验证： 本地 yamllint + actionlint 通过；
act -g渲染出预期的串行 DAG；resolve脚本直接对真实 API 跑通三条触发路径（dispatch → full、/review→ review_only + ack、/triage→ full + ack）输出全部正确。真实 runner 上已经用workflow_dispatch跑过多条真实 PR：#4983 验证 product-decision 模板失败门禁；#5053/#5105 验证 product-decision pass 后进入 review，并暴露/review --comment在并行 agent 阶段[API Error: terminated]后不产出 review JSON；#5030 在e409461d9上验证会转成 review-unavailable 普通 PR 评论、更新既有评论不重复创建、verdict fail、tmux/approval/fallback 均跳过；#5053 merged / #4866 draft 验证 skip guard。上一轮完整 PR CI27501191023已通过 Classify PR、Lint、CodeQL、Ubuntu/macOS/Windows tests 与 coverage comment；review follow-up 提交70dc200d4已通过本地git diff --check、YAML parse、actionlint和针对性bash -n；提交89e2d9496进一步收紧默认/product/reviewGITHUB_TOKEN权限，并把 product/approval 评论去重改为分页 marker 查询，本地git diff --check、YAML parse、actionlint、针对性bash -n与 marker-dedupjqsmoke check 均通过，PR CI27522341719已通过 Classify PR、Lint、CodeQL、Ubuntu/macOS/Windows tests 与 coverage comment；提交12070a30c要求所有非skip的 product-decision verdict 同时产出 product comment body，本地git diff --check、YAML parse、actionlint和 verdict capture smoke checks 均通过，PR CI27525183653已通过 Classify PR、Lint、CodeQL、Ubuntu/macOS/Windows tests 与 coverage comment；最新提交0610d61a6修复本轮本地 review 发现的 gate 稳定性问题：真实 review JSON 会覆盖 provisionalreview-unavailable、tmux PR 状态检查带重试、approval-decision 仅在 tmux 产出pass/fail/timeout后运行，并同步本地/bundled skill 文档。本地git diff --check、YAML parse、actionlint、review fallback-replacement smoke、npm ci（含 build/bundle）和npm run typecheck均通过；当前 PR CI27526661250正在运行，更新时 Classify PR 已通过，Lint/CodeQL/Ubuntu/macOS/Windows tests 仍在跑。尚未覆盖干净通过 review 后的 tmux-testing + approval-decision happy path。风险/范围： product-decision 改 yolo 后对 fork 少一层工具白名单（与 review 一致）；product-decision 不防抖（快速模板反馈的取舍，连续 push 时会反复跑、被后续触发取消）；runner 需预装
qwen、jq、tmux，并且 bundled/review预写 fallback 需要 runner 上的预装 qwen 更新后才会在真实 CI 生效。范围外：设CLAUDE_CODE_SRC_PATH（未设时 skill 回退远程 CHANGELOG）、根治/review --comment并行 agent/API termination 稳定性、干净 review pass 后的 tmux + approval happy path。破坏性：删除qwen-code-pr-review.yml（吸收并取代 #4962）；qwen-triage.yml不再处理 PR；push 后自动 review 最长等 30 分钟。