mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-31 02:06:21 +00:00
fix(ci): clean review worktrees after cancellation (#8474)
* fix(ci): clean review worktrees after cancellation * fix(ci): remove orphaned review worktree directories * fix(tests): sync qwen-resolve-workflow expectations with externalized review timeouts (#8474) * fix(ci): pin review worktree cleanup patterns to paths.ts (#8474) * fix(ci): harden review cleanup sweeps and cover integration_cli (#8474) * fix(ci): extend review cleanup sweep to web_shell_e2e_smoke (#8474) * fix(ci): harden review cleanup git calls * fix(ci): tighten review cleanup comments and test guards (#8474) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ci): pin review cleanup recipe copies byte-identical (#8474) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): guard review worktree removal and pin cleanup invariants (#8474) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
parent
2601d815dd
commit
e34780e24d
5 changed files with 448 additions and 7 deletions
157
.github/workflows/ci.yml
vendored
157
.github/workflows/ci.yml
vendored
|
|
@ -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:
|
||||
|
|
|
|||
62
.github/workflows/qwen-code-pr-review.yml
vendored
62
.github/workflows/qwen-code-pr-review.yml
vendored
|
|
@ -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.
|
||||
|
|
@ -1015,6 +1017,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: |-
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
||||
|
|
|
|||
221
scripts/tests/review-worktree-cleanup-workflow.test.js
Normal file
221
scripts/tests/review-worktree-cleanup-workflow.test.js
Normal file
|
|
@ -0,0 +1,221 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen Team
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { parse } from 'yaml';
|
||||
import {
|
||||
LEASE_PREFIX,
|
||||
REVIEW_TMP_DIR,
|
||||
reviewBranch,
|
||||
worktreePath,
|
||||
} from '../../packages/cli/src/commands/review/lib/paths.js';
|
||||
|
||||
// The cleanup steps in ci.yml and qwen-code-pr-review.yml hard-code the
|
||||
// review-artifact layout owned by paths.ts: worktreePath()/reviewBranch()
|
||||
// and LEASE_PREFIX. Derive the expected patterns from that module so
|
||||
// renaming the layout there fails the build here instead of silently
|
||||
// no-op-ing the sweeps on the shared runners — a suffix rename already
|
||||
// broke a sweeper once (see paths.ts).
|
||||
// npm-cache.yml and qwen-triage.yml also run on the shared pool but are
|
||||
// deliberately not covered here; extending the sweep to them is follow-up
|
||||
// work.
|
||||
const probePr = 12345;
|
||||
const toPosix = (value) => value.replace(/\\/g, '/');
|
||||
const worktreePrefix = toPosix(worktreePath(probePr)).slice(
|
||||
0,
|
||||
-`${probePr}`.length,
|
||||
);
|
||||
const branchFamily = toPosix(reviewBranch(probePr)).slice(
|
||||
0,
|
||||
-`pr-${probePr}`.length,
|
||||
);
|
||||
|
||||
const ciYaml = parse(readFileSync('.github/workflows/ci.yml', 'utf8'));
|
||||
// Every ci.yml job that checks out on the shared self-hosted pool inherits a
|
||||
// possibly dirty workspace. Match the pool itself, not just the output
|
||||
// reference that usually names it: jobs can also hard-code the shared label
|
||||
// array, and a checkout on either form inherits the same leftovers.
|
||||
// Enumerate by pool + checkout instead of job name so the next such job
|
||||
// fails here instead of on the runners.
|
||||
const ciCleanSteps = Object.entries(ciYaml.jobs)
|
||||
.filter(
|
||||
([, job]) =>
|
||||
/ubuntu_runner|ecs-qwen/.test(JSON.stringify(job['runs-on'] ?? '')) &&
|
||||
(job.steps ?? []).some((s) =>
|
||||
String(s.uses ?? '').includes('actions/checkout'),
|
||||
),
|
||||
)
|
||||
.map(([id, job]) => ({
|
||||
id,
|
||||
steps: job.steps,
|
||||
run: job.steps.find((s) => s.name === 'Clean stale .qwen before checkout')
|
||||
?.run,
|
||||
}));
|
||||
const reviewYaml = parse(
|
||||
readFileSync('.github/workflows/qwen-code-pr-review.yml', 'utf8'),
|
||||
);
|
||||
const reviewCleanSteps = reviewYaml.jobs['review-pr'].steps;
|
||||
const reviewCleanIndex = reviewCleanSteps.findIndex(
|
||||
(s) => s.name === 'Clean review worktrees',
|
||||
);
|
||||
const reviewCleanStep = reviewCleanSteps[reviewCleanIndex].run;
|
||||
const agentStateCleanStep = reviewCleanSteps.find(
|
||||
(s) => s.name === 'Clean stale agent state',
|
||||
).run;
|
||||
|
||||
// Comments may name the recipe pieces out of order when explaining them, so
|
||||
// the order and isolation assertions below cover the commands only.
|
||||
const stripComments = (run) =>
|
||||
run
|
||||
.split('\n')
|
||||
.filter((line) => !line.trim().startsWith('#'))
|
||||
.join('\n');
|
||||
|
||||
// The steps run under `bash -e` + pipefail, and a failing for-each-ref or
|
||||
// worktree-list head is exactly the corrupt-leftover state they exist to
|
||||
// tolerate: every piped sweep loop must degrade to a warning via its
|
||||
// trailing `|| true`, never fail the job.
|
||||
function expectPipedLoopsIsolated(code, minLoops) {
|
||||
const loops =
|
||||
code.match(/\|\s*while read -r \w+; do[\s\S]*?\n\s*done(?: \|\| true)?/g) ??
|
||||
[];
|
||||
expect(loops.length).toBeGreaterThanOrEqual(minLoops);
|
||||
for (const loop of loops) {
|
||||
expect(loop.endsWith('done || true')).toBe(true);
|
||||
}
|
||||
}
|
||||
|
||||
// prune (sync registrations) -> force-remove -> prune (drop now-stale
|
||||
// entries) -> delete branches: a branch checked out in a live worktree
|
||||
// cannot be deleted, so worktree removal must precede the branch sweep.
|
||||
function expectCleanupRecipe(run) {
|
||||
expect(run).toContain(`index($0, "/${worktreePrefix}")`);
|
||||
expect(run).toContain('worktree remove --force');
|
||||
expect(run).toContain(`refs/heads/${branchFamily}*`);
|
||||
// The awk filter matches registered paths by substring, but those paths
|
||||
// come from leftover git metadata and are untrusted: the removal loop
|
||||
// must reject `..` traversal and re-anchor to the review prefix first.
|
||||
expect(run).toContain('skipping suspicious review worktree path');
|
||||
expect(run).toContain(`"$GITHUB_WORKSPACE/${worktreePrefix}"*) : ;;`);
|
||||
const code = stripComments(run);
|
||||
const remove = code.indexOf('worktree remove --force');
|
||||
const firstPrune = code.indexOf('worktree prune');
|
||||
expect(firstPrune).toBeGreaterThan(-1);
|
||||
expect(firstPrune).toBeLessThan(remove);
|
||||
expect(code.indexOf('worktree prune', remove)).toBeGreaterThan(remove);
|
||||
expect(code.indexOf(`refs/heads/${branchFamily}*`)).toBeGreaterThan(remove);
|
||||
expectPipedLoopsIsolated(code, 2);
|
||||
}
|
||||
|
||||
function expectHardenedGit(run) {
|
||||
expect(run).toContain(
|
||||
'GIT_SAFE=(git -c core.hooksPath=/dev/null -c core.fsmonitor= -C "$GITHUB_WORKSPACE")',
|
||||
);
|
||||
// Any column, any verb: the review-workflow copies are unindented after
|
||||
// YAML block-scalar stripping, and a bare `git` call would run un-hardened
|
||||
// against leftover untrusted .git config.
|
||||
expect(run).not.toMatch(/^\s*git\s/m);
|
||||
}
|
||||
|
||||
const awkAvailable = spawnSync('awk', ['BEGIN { exit 0 }']).status === 0;
|
||||
|
||||
describe('review worktree cleanup steps', () => {
|
||||
it('keeps every shared-pool ci.yml checkout sweep pinned to paths.ts', () => {
|
||||
expect(ciCleanSteps.map(({ id }) => id)).toEqual(
|
||||
expect.arrayContaining([
|
||||
'test',
|
||||
'web_shell_e2e_smoke',
|
||||
'integration_cli',
|
||||
]),
|
||||
);
|
||||
for (const { id, steps, run } of ciCleanSteps) {
|
||||
expect(
|
||||
run,
|
||||
`job "${id}" checks out on the shared pool and must clean stale .qwen state first`,
|
||||
).toBeDefined();
|
||||
// Position is load-bearing: the sweep must run after the ownership
|
||||
// restore (git refuses root-owned leftovers) and before checkout
|
||||
// (after checkout it no-ops on the fresh tree).
|
||||
const cleanIdx = steps.findIndex(
|
||||
(s) => s.name === 'Clean stale .qwen before checkout',
|
||||
);
|
||||
const checkoutIdx = steps.findIndex((s) =>
|
||||
String(s.uses ?? '').includes('actions/checkout'),
|
||||
);
|
||||
const restoreIdx = steps.findIndex(
|
||||
(s) => s.name === 'Restore workspace ownership',
|
||||
);
|
||||
expect(cleanIdx, id).toBeGreaterThan(restoreIdx);
|
||||
expect(cleanIdx, id).toBeLessThan(checkoutIdx);
|
||||
expectCleanupRecipe(run);
|
||||
expectHardenedGit(run);
|
||||
}
|
||||
// The copies are deliberate: a pre-checkout step cannot trust leftover
|
||||
// workspace scripts, so the recipe stays inline per job. Pin them
|
||||
// byte-identical so a fix to one sweep lands in all of them.
|
||||
const [firstCopy, ...otherCopies] = ciCleanSteps;
|
||||
for (const { id, run } of otherCopies) {
|
||||
expect(run, `job "${id}" sweep drifted from the first copy`).toBe(
|
||||
firstCopy.run,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps the review-job cleanup sweep pinned to paths.ts', () => {
|
||||
// `always()` and the end-of-job position are what make the step fire on
|
||||
// the failure/cancellation paths it exists for: Actions' default
|
||||
// success() condition would skip it once any earlier step fails.
|
||||
expect(reviewCleanSteps[reviewCleanIndex].if).toBe('always()');
|
||||
expect(reviewCleanIndex).toBe(reviewCleanSteps.length - 1);
|
||||
expectCleanupRecipe(reviewCleanStep);
|
||||
expectHardenedGit(reviewCleanStep);
|
||||
// Fallback for worktree directories Git no longer knows about.
|
||||
expect(reviewCleanStep).toContain(`rm -rf ${worktreePrefix}*`);
|
||||
// Leases are session+prompt scoped so a stale one is inert, but the glob
|
||||
// must stay in sync with LEASE_PREFIX or it silently never matches.
|
||||
expect(reviewCleanStep).toContain(
|
||||
`rm -f ${toPosix(REVIEW_TMP_DIR)}/${LEASE_PREFIX}pr-*.json`,
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps the pre-checkout agent-state sweep pinned to paths.ts', () => {
|
||||
// Directories are rm -rf'd first there, so no `worktree remove` to pin.
|
||||
expect(agentStateCleanStep).toContain(`rm -rf ${worktreePrefix}*`);
|
||||
expect(agentStateCleanStep).toContain(`refs/heads/${branchFamily}*`);
|
||||
expectHardenedGit(agentStateCleanStep);
|
||||
expectPipedLoopsIsolated(stripComments(agentStateCleanStep), 1);
|
||||
});
|
||||
|
||||
it('uses one identical worktree filter at every list-driven sweep', () => {
|
||||
const filter = reviewCleanStep.match(/awk '([^']+)'/)?.[1];
|
||||
expect(filter).toBeTruthy();
|
||||
for (const { id, run } of ciCleanSteps) {
|
||||
expect(run, id).toContain(`awk '${filter}'`);
|
||||
}
|
||||
});
|
||||
|
||||
it.skipIf(!awkAvailable)(
|
||||
'filter selects review worktrees only, never the main checkout',
|
||||
() => {
|
||||
const filter = reviewCleanStep.match(/awk '([^']+)'/)?.[1];
|
||||
const main = '/home/runner/work/qwen-code/qwen-code';
|
||||
const review = `${main}/.qwen/tmp/review-pr-42`;
|
||||
const out = spawnSync('awk', [filter], {
|
||||
input: [
|
||||
`worktree ${main}`,
|
||||
`worktree ${review}`,
|
||||
'branch qwen-review/pr-42',
|
||||
'',
|
||||
].join('\n'),
|
||||
encoding: 'utf8',
|
||||
});
|
||||
expect(out.status).toBe(0);
|
||||
expect(out.stdout.trim()).toBe(review);
|
||||
},
|
||||
);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue