mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-10 17:27:10 +00:00
|
Some checks are pending
The probe used to revert the PR's source to base IN the shared review worktree and restore it in a `finally`. That shared tree is the one every Step 3 review agent reads, and the in-place mutate/restore was the root of two findings on #6790: - a concurrent reader could observe the tree half-reverted to base for the probe's whole duration (Critical), and the later restore cannot un-produce a finding written from the wrong source; - the restore's in-place delete followed a PR-controlled symlink out of the tree and removed an outside file (P0, band-aided with `safeRmWithin`). Both share one cause — mutating a live, shared tree — and one fix retires both. The probe now runs in its OWN disposable worktree, checked out at the PR head as a sibling of the shared one (`.qwen/tmp/review-pr-<n>-probe`) and removed wholesale when it finishes: - the shared tree is never touched, so no reader can see a reverted state; - there is no in-place restore, so the delete that followed a symlink is gone with it — `safeRmWithin` stays only as belt-and-suspenders on the revert-phase delete of added files; - `node_modules` needs no per-tree install: the probe tree is nested under the repo, so `npx vitest` resolves upward to the repo-root `node_modules`, exactly as the shared worktree already does. (Confirmed empirically before relying on it — this is what had the refactor deferred.) Because the shared tree is no longer mutated, the dirty-worktree guard is gone (nothing the caller has uncommitted is ever discarded), and the loud `restoreFailure` / non-zero exit becomes a soft `cleanupFailure` warning: a leftover probe worktree does not corrupt anything and is swept at the next run's `worktree add` and by `cleanup.ts`. Verified by driving the real handler (new `test-efficacy.integration.test.ts`, real git worktrees, a stub vitest bin): verdicts are unchanged (gated/inert), the shared tree is byte-identical before and after, the probe tree is always discarded, and the symlink P0 repro leaves the outside file intact WITHOUT `safeRmWithin` having to refuse — isolation alone protects it. Closes #6832. |
||
|---|---|---|
| .. | ||
| src | ||
| index.ts | ||
| package.json | ||
| test-setup.ts | ||
| tsconfig.json | ||
| vitest.config.ts | ||