fix(ci): clean review worktrees after cancellation

.github/workflows/ci.yml

@@ -181,6 +181,51 @@ jobs:
             chmod -R u+w "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || true
             rm -rf "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || sudo -n rm -rf "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || echo "::warning::leaked .qwen; runner needs manual cleanup"
           fi
+          # Interrupted reviews leave worktree registrations under .qwen/tmp/
+          # and qwen-review/* branches behind. prune drops registrations whose
+          # directories the rm above removed; worktree remove --force then
+          # clears any still-registered leftover directory (--force tolerates
+          # dirty contents), since a branch checked out in a live worktree
+          # cannot be deleted. If removal still fails, the registration
+          # survives and the branch delete below warns. The sweep deletes all
+          # review artifacts, not just the current PR's: safe because a runner
+          # executes one job at a time. Kept inline rather than a shared
+          # script: this runs pre-checkout on shared runners, where leftover
+          # workspace files are untrusted.
+          if [ -e "$GITHUB_WORKSPACE/.git" ]; then
+            GIT_SAFE=(git -c core.hooksPath=/dev/null -c core.fsmonitor= -C "$GITHUB_WORKSPACE")
+            "${GIT_SAFE[@]}" worktree prune -v || true
+            "${GIT_SAFE[@]}" worktree list --porcelain \
+              | awk '$1 == "worktree" && index($0, "/.qwen/tmp/review-pr-") > 0 { sub(/^worktree /, ""); print }' \
+              | while read -r worktree; do
+                  [ -n "$worktree" ] || continue
+                  # Registered paths come from leftover git metadata and are
+                  # untrusted: the awk filter above matched by substring, so reject
+                  # `..` traversal and re-anchor to the review prefix before the
+                  # destructive remove.
+                  case "$worktree" in
+                    */../*|../*|*/..)
+                      echo "::warning::skipping suspicious review worktree path: $worktree"
+                      continue
+                      ;;
+                    "$GITHUB_WORKSPACE/.qwen/tmp/review-pr-"*) : ;;
+                    *)
+                      echo "::warning::skipping unexpected review worktree path: $worktree"
+                      continue
+                      ;;
+                  esac
+                  "${GIT_SAFE[@]}" worktree remove --force "$worktree" ||
+                    echo "::warning::could not remove review worktree: $worktree"
+                done || true
+            "${GIT_SAFE[@]}" worktree prune -v || true
+            "${GIT_SAFE[@]}" for-each-ref --format='%(refname:short)' 'refs/heads/qwen-review/*' \
+              | while read -r stale_ref; do
+                  if [ -n "$stale_ref" ]; then
+                    "${GIT_SAFE[@]}" branch -D "$stale_ref" ||
+                      echo "::warning::could not remove review branch: $stale_ref"
+                  fi
+                done || true
+          fi
 
       # On PRs, check out refs/pull/N/head (the immutable PR head, published the
       # instant the branch is pushed) instead of github.ref. github.ref is the
@@ -523,6 +568,62 @@ jobs:
           fi
           chmod -R u+rwX "$GITHUB_WORKSPACE" 2>/dev/null || sudo -n chmod -R u+rwX "$GITHUB_WORKSPACE" || echo "::warning::could not restore workspace write permissions; checkout may fail on leftover read-only files"
 
+      # Same pre-checkout recovery as the test job: this job lands on the
+      # same reused pool, so leftover review worktrees and branches from an
+      # interrupted review would break this checkout too.
+      - name: 'Clean stale .qwen before checkout'
+        run: |-
+          set -uo pipefail
+          if [ -d "$GITHUB_WORKSPACE/.qwen" ] && [ ! -L "$GITHUB_WORKSPACE/.qwen" ]; then
+            chmod -R u+w "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || true
+            rm -rf "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || sudo -n rm -rf "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || echo "::warning::leaked .qwen; runner needs manual cleanup"
+          fi
+          # Interrupted reviews leave worktree registrations under .qwen/tmp/
+          # and qwen-review/* branches behind. prune drops registrations whose
+          # directories the rm above removed; worktree remove --force then
+          # clears any still-registered leftover directory (--force tolerates
+          # dirty contents), since a branch checked out in a live worktree
+          # cannot be deleted. If removal still fails, the registration
+          # survives and the branch delete below warns. The sweep deletes all
+          # review artifacts, not just the current PR's: safe because a runner
+          # executes one job at a time. Kept inline rather than a shared
+          # script: this runs pre-checkout on shared runners, where leftover
+          # workspace files are untrusted.
+          if [ -e "$GITHUB_WORKSPACE/.git" ]; then
+            GIT_SAFE=(git -c core.hooksPath=/dev/null -c core.fsmonitor= -C "$GITHUB_WORKSPACE")
+            "${GIT_SAFE[@]}" worktree prune -v || true
+            "${GIT_SAFE[@]}" worktree list --porcelain \
+              | awk '$1 == "worktree" && index($0, "/.qwen/tmp/review-pr-") > 0 { sub(/^worktree /, ""); print }' \
+              | while read -r worktree; do
+                  [ -n "$worktree" ] || continue
+                  # Registered paths come from leftover git metadata and are
+                  # untrusted: the awk filter above matched by substring, so reject
+                  # `..` traversal and re-anchor to the review prefix before the
+                  # destructive remove.
+                  case "$worktree" in
+                    */../*|../*|*/..)
+                      echo "::warning::skipping suspicious review worktree path: $worktree"
+                      continue
+                      ;;
+                    "$GITHUB_WORKSPACE/.qwen/tmp/review-pr-"*) : ;;
+                    *)
+                      echo "::warning::skipping unexpected review worktree path: $worktree"
+                      continue
+                      ;;
+                  esac
+                  "${GIT_SAFE[@]}" worktree remove --force "$worktree" ||
+                    echo "::warning::could not remove review worktree: $worktree"
+                done || true
+            "${GIT_SAFE[@]}" worktree prune -v || true
+            "${GIT_SAFE[@]}" for-each-ref --format='%(refname:short)' 'refs/heads/qwen-review/*' \
+              | while read -r stale_ref; do
+                  if [ -n "$stale_ref" ]; then
+                    "${GIT_SAFE[@]}" branch -D "$stale_ref" ||
+                      echo "::warning::could not remove review branch: $stale_ref"
+                  fi
+                done || true
+          fi
+
       - name: 'Checkout'
         uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2
         with:
@@ -815,6 +916,62 @@ jobs:
           fi
           chmod -R u+rwX "$GITHUB_WORKSPACE" 2>/dev/null || sudo -n chmod -R u+rwX "$GITHUB_WORKSPACE" || echo "::warning::could not restore workspace write permissions; checkout may fail on leftover read-only files"
 
+      # Same pre-checkout recovery as the test job: this job lands on the
+      # same reused pool, so leftover review worktrees and branches from an
+      # interrupted review would break this checkout too.
+      - name: 'Clean stale .qwen before checkout'
+        run: |-
+          set -uo pipefail
+          if [ -d "$GITHUB_WORKSPACE/.qwen" ] && [ ! -L "$GITHUB_WORKSPACE/.qwen" ]; then
+            chmod -R u+w "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || true
+            rm -rf "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || sudo -n rm -rf "$GITHUB_WORKSPACE/.qwen" 2>/dev/null || echo "::warning::leaked .qwen; runner needs manual cleanup"
+          fi
+          # Interrupted reviews leave worktree registrations under .qwen/tmp/
+          # and qwen-review/* branches behind. prune drops registrations whose
+          # directories the rm above removed; worktree remove --force then
+          # clears any still-registered leftover directory (--force tolerates
+          # dirty contents), since a branch checked out in a live worktree
+          # cannot be deleted. If removal still fails, the registration
+          # survives and the branch delete below warns. The sweep deletes all
+          # review artifacts, not just the current PR's: safe because a runner
+          # executes one job at a time. Kept inline rather than a shared
+          # script: this runs pre-checkout on shared runners, where leftover
+          # workspace files are untrusted.
+          if [ -e "$GITHUB_WORKSPACE/.git" ]; then
+            GIT_SAFE=(git -c core.hooksPath=/dev/null -c core.fsmonitor= -C "$GITHUB_WORKSPACE")
+            "${GIT_SAFE[@]}" worktree prune -v || true
+            "${GIT_SAFE[@]}" worktree list --porcelain \
+              | awk '$1 == "worktree" && index($0, "/.qwen/tmp/review-pr-") > 0 { sub(/^worktree /, ""); print }' \
+              | while read -r worktree; do
+                  [ -n "$worktree" ] || continue
+                  # Registered paths come from leftover git metadata and are
+                  # untrusted: the awk filter above matched by substring, so reject
+                  # `..` traversal and re-anchor to the review prefix before the
+                  # destructive remove.
+                  case "$worktree" in
+                    */../*|../*|*/..)
+                      echo "::warning::skipping suspicious review worktree path: $worktree"
+                      continue
+                      ;;
+                    "$GITHUB_WORKSPACE/.qwen/tmp/review-pr-"*) : ;;
+                    *)
+                      echo "::warning::skipping unexpected review worktree path: $worktree"
+                      continue
+                      ;;
+                  esac
+                  "${GIT_SAFE[@]}" worktree remove --force "$worktree" ||
+                    echo "::warning::could not remove review worktree: $worktree"
+                done || true
+            "${GIT_SAFE[@]}" worktree prune -v || true
+            "${GIT_SAFE[@]}" for-each-ref --format='%(refname:short)' 'refs/heads/qwen-review/*' \
+              | while read -r stale_ref; do
+                  if [ -n "$stale_ref" ]; then
+                    "${GIT_SAFE[@]}" branch -D "$stale_ref" ||
+                      echo "::warning::could not remove review branch: $stale_ref"
+                  fi
+                done || true
+          fi
+
       - name: 'Checkout'
         uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3
         with:

.github/workflows/qwen-code-pr-review.yml

@@ -397,15 +397,17 @@ jobs:
             echo "no prior workspace; nothing to clean"
             exit 0
           fi
+          GIT_SAFE=(git -c core.hooksPath=/dev/null -c core.fsmonitor= -C "$GITHUB_WORKSPACE")
           rm -rf .qwen/tmp/review-pr-* 2>/dev/null || true
-          git worktree prune -v || true
-          git for-each-ref --format='%(refname:short)' 'refs/heads/qwen-review/*' \
+          "${GIT_SAFE[@]}" worktree prune -v || true
+          "${GIT_SAFE[@]}" for-each-ref --format='%(refname:short)' 'refs/heads/qwen-review/*' \
             | while read -r stale_ref; do
                 if [ -n "$stale_ref" ]; then
-                  git branch -D "$stale_ref" || true
+                  "${GIT_SAFE[@]}" branch -D "$stale_ref" ||
+                    echo "::warning::could not remove review branch: $stale_ref"
                 fi
-              done
-          git worktree prune -v || true
+              done || true
+          "${GIT_SAFE[@]}" worktree prune -v || true
           echo "stale agent state cleaned"
 
       # SECURITY: checkout trusted base code; /review fetches PR diff context.
@@ -989,6 +991,56 @@ jobs:
             --repo "$GITHUB_REPOSITORY" \
             --body "$body"
 
+      # A cancelled or timed-out review may not reach the CLI's process cleanup.
+      # Remove both the worktree directories and Git's worktree registrations so
+      # the next job on this reused runner can delete qwen-review/* branches.
+      # The sweep deletes all review artifacts, not just this PR's: safe because
+      # a runner executes one job at a time.
+      - name: 'Clean review worktrees'
+        if: 'always()'
+        timeout-minutes: 5
+        run: |-
+          set -uo pipefail
+          if [ ! -e .git ]; then
+            echo "no Git checkout; nothing to clean"
+            exit 0
+          fi
+
+          GIT_SAFE=(git -c core.hooksPath=/dev/null -c core.fsmonitor= -C "$GITHUB_WORKSPACE")
+          "${GIT_SAFE[@]}" worktree prune -v || true
+          "${GIT_SAFE[@]}" worktree list --porcelain \
+            | awk '$1 == "worktree" && index($0, "/.qwen/tmp/review-pr-") > 0 { sub(/^worktree /, ""); print }' \
+            | while read -r worktree; do
+                [ -n "$worktree" ] || continue
+                # Registered paths come from leftover git metadata and are
+                # untrusted: the awk filter above matched by substring, so reject
+                # `..` traversal and re-anchor to the review prefix before the
+                # destructive remove.
+                case "$worktree" in
+                  */../*|../*|*/..)
+                    echo "::warning::skipping suspicious review worktree path: $worktree"
+                    continue
+                    ;;
+                  "$GITHUB_WORKSPACE/.qwen/tmp/review-pr-"*) : ;;
+                  *)
+                    echo "::warning::skipping unexpected review worktree path: $worktree"
+                    continue
+                    ;;
+                esac
+                "${GIT_SAFE[@]}" worktree remove --force "$worktree" ||
+                  echo "::warning::could not remove review worktree: $worktree"
+              done || true
+          rm -rf .qwen/tmp/review-pr-* 2>/dev/null || true
+          "${GIT_SAFE[@]}" worktree prune -v || true
+          "${GIT_SAFE[@]}" for-each-ref --format='%(refname:short)' 'refs/heads/qwen-review/*' \
+            | while read -r review_ref; do
+                [ -n "$review_ref" ] || continue
+                "${GIT_SAFE[@]}" branch -D "$review_ref" ||
+                  echo "::warning::could not remove review branch: $review_ref"
+              done || true
+          rm -f .qwen/tmp/qwen-review-lease-pr-*.json 2>/dev/null || true
+          echo "review worktrees cleaned"
+
   resolve-pr:
     needs: ['authorize']
     if: |-

packages/cli/src/commands/review/lib/paths.ts

@@ -15,6 +15,14 @@ export const REVIEW_TMP_DIR = join('.qwen', 'tmp');
 export const REVIEWS_DIR = join('.qwen', 'reviews');
 export const REVIEW_CACHE_DIR = join('.qwen', 'review-cache');
 
+/**
+ * Filename prefix for review-worktree lease files under `REVIEW_TMP_DIR`.
+ * Lives here, not in `review-worktree-lease.ts`, because the review
+ * workflow's cleanup sweep deletes leases by glob — the sweep pattern and
+ * the lease writer must share one definition (the cleanup spec pins both).
+ */
+export const LEASE_PREFIX = 'qwen-review-lease-';
+
 /**
  * Where the skill tees `qwen review parse-args`'s verdict (SKILL Step 0). A fixed,
  * conventional name so a capture command can read back the effort the parser

packages/cli/src/services/review-worktree-lease.ts

@@ -12,9 +12,12 @@ import {
 } from 'node:fs';
 import { basename, isAbsolute, join, relative, resolve } from 'node:path';
 import { createDebugLogger } from '@qwen-code/qwen-code-core';
-import { REVIEW_TMP_DIR, reviewBranch } from '../commands/review/lib/paths.js';
+import {
+  LEASE_PREFIX,
+  REVIEW_TMP_DIR,
+  reviewBranch,
+} from '../commands/review/lib/paths.js';
 
-const LEASE_PREFIX = 'qwen-review-lease-';
 const GIT_TIMEOUT_MS = 120_000;
 const debugLogger = createDebugLogger('REVIEW_WORKTREE_LEASE');