From 492544e73a744d7facb130e59bcd4251b280320d Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Wed, 20 May 2026 09:05:48 +0800 Subject: [PATCH 1/2] fix(review): harden SKILL.md against weak-model rule skipping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Weak models often skip parts of the long /review prompt and fall back to familiar defaults — `gh pr checkout` instead of the worktree flow, or running the autofix prompt even when the user passed `--comment` (which means "only post inline comments, don't mutate code"). Three reinforcements, all in SKILL.md (no CLI changes): - Promote the two most commonly violated rules to the top of the "Critical rules" list: worktree is mandatory for PR reviews, and `--comment` skips Step 8 entirely. - Add an inline blockquote at the top of the Step 1 PR branch that names the specific forbidden commands (`gh pr checkout`, `git checkout`, `git switch`, `git pull`, `git reset --hard`). - Add an explicit skip block at the top of Step 8 listing the three conditions that bypass autofix — `--comment`, cross-repo lightweight mode, or no fixable findings — so a weak model doesn't have to infer them from scattered earlier text. --- packages/core/src/skills/bundled/review/SKILL.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/core/src/skills/bundled/review/SKILL.md b/packages/core/src/skills/bundled/review/SKILL.md index b74e33a1c0f..bbe0a213e30 100644 --- a/packages/core/src/skills/bundled/review/SKILL.md +++ b/packages/core/src/skills/bundled/review/SKILL.md @@ -18,8 +18,10 @@ You are an expert code reviewer. Your job is to review code changes and provide **Critical rules (most commonly violated — read these first):** -1. **Match the language of the PR.** If the PR is in English, ALL your output (terminal + PR comments) MUST be in English. If in Chinese, use Chinese. Do NOT switch languages. For **local reviews** (no PR), if the system prompt includes an output language preference, use that language; otherwise follow the user's input language. -2. **Step 9: use Create Review API** with `comments` array for inline comments. Do NOT use `gh api .../pulls/.../comments` to post individual comments. See Step 9 for the JSON format. +1. **For PR reviews, the worktree is MANDATORY.** Your very first action MUST be `qwen review fetch-pr` (Step 1). Do NOT use `gh pr checkout`, `git checkout `, `git switch`, or any other command that touches the user's working tree. After `fetch-pr` returns, ALL subsequent reads, linters, builds, tests, and edits MUST happen inside the `worktreePath` it created. Violating this contaminates the user's local branch state. +2. **If `--comment` was specified, Step 8 (Autofix) is SKIPPED entirely.** `--comment` means the user wants inline PR comments posted, not code mutations. Do not ask "Apply auto-fixes? (y/n)" — go straight from Step 7 to Step 9. +3. **Match the language of the PR.** If the PR is in English, ALL your output (terminal + PR comments) MUST be in English. If in Chinese, use Chinese. Do NOT switch languages. For **local reviews** (no PR), if the system prompt includes an output language preference, use that language; otherwise follow the user's input language. +4. **Step 9: use Create Review API** with `comments` array for inline comments. Do NOT use `gh api .../pulls/.../comments` to post individual comments. See Step 9 for the JSON format. **Design philosophy: Silence is better than noise.** Every comment you make should be worth the reader's time. If you're unsure whether something is a problem, DO NOT MENTION IT. Low-quality feedback causes "cry wolf" fatigue — developers stop reading all AI comments and miss real issues. @@ -44,6 +46,8 @@ Based on the remaining arguments: - If both diffs are empty, inform the user there are no changes to review and stop here — do not proceed to the review agents - **PR number or same-repo URL** (e.g., `123` or a URL whose owner/repo matches the current repo — cross-repo URLs are handled by the lightweight mode above): + + > ⚠️ **MANDATORY worktree flow.** Do NOT use `gh pr checkout`, `git checkout `, `git switch`, `git pull`, `git reset --hard`, or any other command that changes the user's current HEAD or working tree contents. The ONLY entry point is `qwen review fetch-pr` (below) — it isolates the PR into an ephemeral worktree so the user's local state is never touched. After it returns, every subsequent command in Steps 2-8 MUST operate inside the returned `worktreePath` (e.g. `cd ` first, or pass the path as a `--cwd` / explicit argument). - **Run `qwen review fetch-pr`** to set up the working state in one pass — it cleans any stale worktree, fetches the PR HEAD into `qwen-review/pr-`, queries `gh pr view` for metadata, and creates an ephemeral worktree at `.qwen/tmp/review-pr-`: ```bash @@ -442,7 +446,13 @@ If the user responds with "post comments" (or similar intent like "yes post them ## Step 8: Autofix -If there are **Critical** or **Suggestion** findings with clear, unambiguous fixes, offer to auto-apply them. +**Skip this entire step (do not even ask) if ANY of the following is true:** + +- `--comment` was specified in the arguments — the user explicitly asked for inline PR comments, not code edits. Go straight to Step 9. +- The review target is a cross-repo PR running in lightweight mode (no local files to edit). +- There are no Critical or Suggestion findings with concrete, applicable fixes. + +Otherwise, if there are **Critical** or **Suggestion** findings with clear, unambiguous fixes, offer to auto-apply them. 1. Count the number of auto-fixable findings (those with concrete suggested fixes that can be expressed as file edits). 2. If there are fixable findings, ask the user: From 04b30a94b4ebb19ba48e36e0a205440fe7a4a91a Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Wed, 20 May 2026 21:58:38 +0800 Subject: [PATCH 2/2] fix(review): address /review comments on rule scope + Step 8 dedup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the initial harden pass, addressing the inline review comments on PR #4340. Rule #1 (worktree mandatory): - Scope it to **same-repo PR reviews** so cross-repo PRs running in lightweight mode (no matching local remote, no worktree) don't read as a contradiction. - Replace "Your very first action" with "After argument parsing and remote detection, the first command that touches code state" — the literal "very first" was wrong since `--comment` parsing and URL/remote disambiguation legitimately run before `fetch-pr`. - Align the forbidden-command list with the Step 1 blockquote (add `git pull` and `git reset --hard`) so a weak model that only reads the Critical rules section sees the same five commands as a model that reaches the blockquote at the point of use. - Add an explicit "cross-repo PRs use lightweight mode" parenthetical so the same model knows where to look for the alternative path. Step 8 skip block: - Drop the redundant third bullet ("no Critical or Suggestion findings with concrete, applicable fixes") — it was both logically equivalent to the "Otherwise" clause below and used a different qualifier ("concrete, applicable" vs "clear, unambiguous"), risking a weak model treating them as two distinct thresholds. - "ANY of the following" → "EITHER" since only two bullets remain. - Fold the no-findings case into the Otherwise clause as a no-op note. --- packages/core/src/skills/bundled/review/SKILL.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/core/src/skills/bundled/review/SKILL.md b/packages/core/src/skills/bundled/review/SKILL.md index bbe0a213e30..e9a53a6c101 100644 --- a/packages/core/src/skills/bundled/review/SKILL.md +++ b/packages/core/src/skills/bundled/review/SKILL.md @@ -18,7 +18,7 @@ You are an expert code reviewer. Your job is to review code changes and provide **Critical rules (most commonly violated — read these first):** -1. **For PR reviews, the worktree is MANDATORY.** Your very first action MUST be `qwen review fetch-pr` (Step 1). Do NOT use `gh pr checkout`, `git checkout `, `git switch`, or any other command that touches the user's working tree. After `fetch-pr` returns, ALL subsequent reads, linters, builds, tests, and edits MUST happen inside the `worktreePath` it created. Violating this contaminates the user's local branch state. +1. **For same-repo PR reviews (PR number, or URL whose owner/repo matches a local remote), the worktree is MANDATORY.** After argument parsing and remote detection (early in Step 1), the first command that touches code state MUST be `qwen review fetch-pr`. Do NOT use `gh pr checkout`, `git checkout `, `git switch`, `git pull`, `git reset --hard`, or any other command that modifies the user's current HEAD or working tree. After `fetch-pr` returns, ALL subsequent reads, linters, builds, tests, and edits MUST happen inside the `worktreePath` it created. Violating this contaminates the user's local branch state. (Cross-repo PRs with no matching remote use lightweight mode and do NOT create a worktree — see Step 1.) 2. **If `--comment` was specified, Step 8 (Autofix) is SKIPPED entirely.** `--comment` means the user wants inline PR comments posted, not code mutations. Do not ask "Apply auto-fixes? (y/n)" — go straight from Step 7 to Step 9. 3. **Match the language of the PR.** If the PR is in English, ALL your output (terminal + PR comments) MUST be in English. If in Chinese, use Chinese. Do NOT switch languages. For **local reviews** (no PR), if the system prompt includes an output language preference, use that language; otherwise follow the user's input language. 4. **Step 9: use Create Review API** with `comments` array for inline comments. Do NOT use `gh api .../pulls/.../comments` to post individual comments. See Step 9 for the JSON format. @@ -446,13 +446,12 @@ If the user responds with "post comments" (or similar intent like "yes post them ## Step 8: Autofix -**Skip this entire step (do not even ask) if ANY of the following is true:** +**Skip this entire step (do not even ask) if EITHER of the following is true:** - `--comment` was specified in the arguments — the user explicitly asked for inline PR comments, not code edits. Go straight to Step 9. - The review target is a cross-repo PR running in lightweight mode (no local files to edit). -- There are no Critical or Suggestion findings with concrete, applicable fixes. -Otherwise, if there are **Critical** or **Suggestion** findings with clear, unambiguous fixes, offer to auto-apply them. +Otherwise, if there are **Critical** or **Suggestion** findings with clear, unambiguous fixes, offer to auto-apply them. (If there are no such findings, this step is also a no-op — fall through to Step 9.) 1. Count the number of auto-fixable findings (those with concrete suggested fixes that can be expressed as file edits). 2. If there are fixable findings, ask the user: