1. Pure integer PR numbers default to origin (no URL-based remote
matching available). URL-based PRs use the matched remote.
2. Autofix push uses matched remote instead of hardcoded origin.
Push failure is expected for fork PRs where user lacks push
access — handled gracefully with informative message.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Cross-repo lightweight mode: no longer skips Step 11 entirely.
Worktree removal is skipped (none created) but temp files are
still cleaned up.
2. Step 2 base-branch fetch: use the matched remote from Step 1
(e.g., upstream for forks) instead of hardcoded origin.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously compared URL owner/repo against gh repo view (current repo
only). This caused fork-based workflows to be wrongly classified as
cross-repo (e.g., local clone of wenshao/jdk reviewing openjdk/jdk PR).
Now checks all git remotes (git remote -v) for a match. If any remote
points to the URL's repo, proceeds with full worktree mode using that
remote for fetch. Only falls back to lightweight mode if NO remote
matches.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Step 9 hardcoded gh repo view to get owner/repo, which returns the
current repo — wrong for cross-repo reviews. Now explicitly branches:
same-repo uses gh repo view, cross-repo uses URL-extracted owner/repo.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- git branch -D: add 2>/dev/null || true to both cleanup sites
(Step 1 stale cleanup + Step 11) to prevent abort if ref missing
- Cross-repo doc: clarify Agents 1-4 only (Agent 5 build/test
requires local codebase, not available in cross-repo mode)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Line 22: "Step 2" → "Step 4" (agents are Step 4, rules are Step 2)
2. PR section: clarify "same-repo URL" to avoid ambiguity with
cross-repo lightweight path
3. Cross-file analysis: add "same-repo only" qualifier (no local
files in cross-repo mode for grep_search)
4. Cross-repo lightweight mode: add existing PR comment fetching
to avoid duplicating human feedback
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Strengthen the parallel instruction from "five parallel review agents"
to explicitly requiring all 5 task tool calls in one response. Without
this, the LLM may serialize agents (wait for each to finish), losing
the wall-clock time benefit of parallel execution.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SKILL.md:
- Step 9 must use owner/repo from URL (not gh repo view) for cross-repo
- Step 2 (project rules) skipped in cross-repo mode (no local files)
User doc: add Cross-repo PR Review section with same-repo vs cross-repo
capability comparison table.
DESIGN.md: add "Why cross-repo uses lightweight mode" section explaining
CLI tools are inherently repo-local and our approach is best available.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of rejecting cross-repo PR URLs, run in lightweight mode:
use gh pr diff <url> to get diff directly (no worktree needed),
skip deterministic analysis and build/test, and review diff text
only with LLM agents.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a PR URL points to a different repo (e.g., other-org/other-repo),
the review would silently operate on the wrong PR in the current repo.
Now verifies URL owner/repo matches current repo before proceeding.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If autofix pushes a new commit in Step 3.5, the PR HEAD changes.
Step 4's inline comments would then reference the autofix commit
where line numbers may have shifted, causing comments on wrong lines.
Fix: capture headRefOid in Step 1 (before autofix) and reuse in
Step 4. Also fix stale Step 5 comment about worktree/commit SHA.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reverse audit agent already has full context (all confirmed findings +
entire diff), so its findings don't need a second opinion. This brings
the actual LLM call count to 7 (5 review + 1 verify + 1 reverse),
matching the documented claim.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If a previous review was interrupted (Ctrl+C, crash), stale worktree
and local ref would block the next review. Now Step 1 checks for and
cleans up stale .qwen/tmp/review-pr-<N> worktree and qwen-review/pr-<N>
ref before creating new ones.
Step 5 also cleans up the local ref alongside the worktree.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mermaid only renders on GitHub; shows as raw code on Nextra,
Docusaurus, VS Code preview, and offline viewing. Plain-text
ASCII diagram is universally compatible and includes LLM call
cost annotations on each stage.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Step 4.5: use absolute paths for reports/cache in worktree mode
(relative paths would land in worktree and be deleted)
- Step 1: fetch into qwen-review/pr-<N> ref to avoid clobbering
existing local branches
- Step 2.6: reverse audit findings use batch verification (not
one-per-finding), consistent with Step 2.5
- Doc: clarify reverse audit findings are also batch-verified
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add Token Efficiency section showing fixed 7 LLM calls breakdown
- Fix follow-up table: "fix these issues" is local-only (worktree
cleaned up after PR review)
- Update PR description with worktree, batch verification, cross-model
review, PR comment dedup, and expanded test plan
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously, each finding got its own independent verification agent
(N findings = N LLM calls). Now a single verification agent receives
all findings at once and verifies them in one pass.
Token cost: 6+N variable calls → 7 fixed calls (5 review + 1 verify + 1 reverse audit)
Quality: minimal impact — batch verification has fuller cross-finding context
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add model attribution to no-findings LGTM path
- Handle empty string from getModel() with .trim() || 'unknown'
- Add tests for {{model}} with args and empty model ID
- Fix doc contradiction: PR autofix pushes automatically from worktree
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Remove gh pr checkout --detach (modifies working tree, defeats
worktree purpose). Use git fetch only.
2. Add dependency installation step (npm ci etc.) in worktree —
without it, all TS/JS linting/building fails.
3. Cache and reports written to main project dir, not worktree
(would be deleted in Step 5).
4. "fix these issues" tip only for local reviews — worktree is
cleaned up after PR review, so interactive fixing not possible.
5. Autofix push uses explicit remote branch name from Step 1.
6. Move incremental check before dependency install to avoid
wasting time when no new changes.
7. Fix Step 3 reference: "from Steps 2.5 and 2.6" (includes
reverse audit findings).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the stash + checkout + restore flow with an isolated git
worktree for PR reviews. This eliminates:
- Stash orphan risks (multiple early exit paths)
- Wrong-branch risks (Step 5 restore failures)
- Build cache pollution (worktree has its own state)
- All stash-related error handling complexity
New flow:
- Step 1: git worktree add .qwen/tmp/review-pr-<number>
- All agents operate in the worktree directory
- Autofix commits and pushes from the worktree
- Step 5: git worktree remove (--force for dirty worktrees)
User's working tree is never modified during PR reviews.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
run_shell_command expects timeout in milliseconds. Without explicit
ms values, implementations may pass 120/60 and time out immediately.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
For PR reviews, fetch existing inline and general comments via gh api
before launching agents. A summary of already-discussed issues is
passed to agents so they don't re-report problems that humans or other
tools have already flagged.
Added to Exclusion Criteria: "Issues already discussed in existing
PR comments."
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- commands.md: renumber 1.6→1.7→1.8→1.9 after inserting 1.5 Built-in Skills
- SKILL.md: promote Reverse audit from ### to ## Step 2.6 for consistent
step hierarchy
- _meta.ts: add code-review to Features navigation sidebar
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The incremental review cache now stores modelId alongside commitSha.
When the same PR is re-reviewed with a different model:
- Cache detects model change → runs full review (not skipped)
- Informs user: "Previous review used X. Running full review with Y
for a second opinion."
Same SHA + same model still skips as before.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Step 2.6: after all findings are verified and aggregated, a single
reverse audit agent reviews the diff with full knowledge of what was
already found, specifically looking for important issues that all
previous agents missed.
- Only reports Critical/Suggestion level gaps (not Nice to have)
- Findings go through the same verification as other agents
- Single agent call — minimal cost overhead
- If nothing is found, initial review had strong coverage
This formalizes the "multi-round undirected audit" pattern that proved
effective during the development of this PR (14 rounds, 40+ issues).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add comprehensive user documentation for the /review command covering:
- Quick start examples for all modes (local, PR, file, --comment)
- Pipeline overview with all steps explained
- Review agents table (5 agents + their focus areas)
- Deterministic analysis (supported languages and tools)
- Severity levels and PR comment filtering rules
- Autofix workflow
- PR inline comments (what gets posted vs terminal-only)
- Follow-up actions (fix/post comments/commit)
- Project review rules (.qwen/review-rules.md etc.)
- Incremental review and caching
- Review report persistence
- Cross-file impact analysis
- Design philosophy
Also add /review and /simplify to the commands reference page
under a new "Built-in Skills" section with link to full docs.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After review with unfixed findings (autofix declined/partial/N/A),
suggest "type fix these issues" so the LLM can interactively fix
each finding using the edit tool without re-running the review.
Follow-up tips now cover the complete post-review flow:
- Unfixed findings → "fix these issues"
- PR with findings → "post comments"
- Local all clear → "commit"
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After local review with no critical issues, suggest "type commit to
commit your changes" — the follow-up system picks this up as ghost
text so users can Tab to commit.
PR reviews keep the existing "post comments" tip.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Comment template: replace **[{severity}]** with {prefix} placeholder
so auto-fixed prefix is not dropped
- Agent 5: run exactly one build + one test command using precedence
order to avoid duplicates (e.g., Makefile wrapping npm)
- Clarify timeout as 120000ms for run_shell_command
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Changed the tip from "/review <number> --comment" (which re-runs the
full review) to "post comments" (which reuses existing findings in
the same conversation and jumps directly to Step 4).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The tip placeholder <number> was being output literally, causing the
follow-up suggestion system to generate commands with wrong PR IDs.
Now explicitly instructs the LLM to substitute the real PR number.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace ambiguous {prefix} placeholder with concrete examples showing
the full Markdown bold + severity tag format for normal and auto-fixed
findings.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Low-confidence findings now appear only in the terminal "Needs Human
Review" section and are never posted as PR inline comments. This
resolves the contradiction between "silence > noise" and posting
uncertain findings on PRs.
Also clarified that "confirmed (low confidence)" is for issues likely
real but needing human judgment, not vague suspicions (those should
be rejected).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deleted/renamed files in the diff would cause per-file linters to fail
on non-existent paths, producing false deterministic failures. Now uses
--diff-filter=d to exclude deletions from the changed files list.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rename "should use unknown when model is not available" to
"when getModel returns undefined" — the mock config does define
getModel, it just returns undefined.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Copilot-instructions.md precedence: prefer .github/ path, do not
load both when both exist
- Simplify getModel() call: remove unnecessary typeof guard since
Config already defines getModel()
- Fix TS2352 type error in test: use proper mock cast pattern
- Add getModel to base mockConfig for test consistency
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add {{model}} template variable support in BundledSkillLoader. When a
skill body contains {{model}}, it is replaced with the runtime model ID
from config.getModel(). Only skills that use the variable are affected.
The /review skill now appends a model attribution footer to PR review
summaries: "Reviewed by {model} via Qwen Code /review"
This enables cross-model review workflows (e.g., develop with model A,
review with model B) with accurate attribution in PR comments.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Step 1.5 said "the diff output" (singular) but local reviews produce
two diffs (git diff + git diff --staged). Changed files list now
explicitly takes the union of both.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When deduplication merges findings with different severities (e.g.,
a Critical typecheck error with a Suggestion from LLM review), the
merged finding now uses the highest severity. Deterministic severity
is treated as authoritative and cannot be downgraded.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When PR checkout fails or incremental review finds no new changes,
restore the environment (checkout original branch, pop stash) before
stopping. Previously these early exits left the stash orphaned.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
