Reviewer tasks must fail closed or use scratch clones when target worktree is dirty #233

New Issue

2026-07-06T01:01:29-05:00

jcwalker3 commented

2026-07-06 01:01:29 -05:00

A reviewer session reviewing wiki proof-head refresh PRs #95 and #231 encountered local dirty files unrelated to the review task:

review_proofs.py
tests/test_mcp_server.py
tests/test_review_proofs.py

Instead of stopping or switching to a clean scratch clone/worktree, the session used multiple git stash, stash pop, checkout --, and stash drop operations to manipulate local changes. The final report claims the files were restored, but this violated the reviewer hard rule:

Do not touch another session’s dirty worktree. Use a clean scratch clone/worktree if needed.

This should become an enforced wall because reviewer sessions must never manage, stash, reset, or restore unrelated local work. Reviewer validation should happen in a disposable scratch clone or fail closed with a clear handoff.

Acceptance criteria:

Reviewer workflow detects dirty worktree before checkout, diff, validation, merge, or review.
If dirty files exist outside the PR scope, reviewer must either:
- stop with a terminal report, or
- create/use a clean scratch clone/worktree.
Reviewer helpers must not run git stash, git checkout --, git reset, or git clean against unrelated local changes.
Add tests proving reviewer tasks fail closed or select scratch execution when unrelated dirty files exist.
Add tests proving author-side work can continue using normal branches, but reviewer-side work cannot manipulate another session’s dirty state.
Update reviewer prompt/template text to explicitly forbid stash/reset/checkout cleanup of unrelated local changes.
Add proof output requiring reviewers to report:
- starting worktree path
- whether it was dirty
- whether a scratch clone/worktree was used
- confirmation that no unrelated local files were modified, stashed, reset, or dropped

Related observed commands:

git stash
git stash push -m "reviewer-temp-stash" -- tests/test_mcp_server.py tests/test_review_proofs.py
git checkout -- review_proofs.py tests/test_mcp_server.py tests/test_review_proofs.py
multiple git stash pop
multiple git stash drop

This is not about reverting PR #95 or PR #231. Those docs PRs may be valid. This issue is about preventing reviewer worktree interference in future sessions.

A reviewer session reviewing wiki proof-head refresh PRs #95 and #231 encountered local dirty files unrelated to the review task: * `review_proofs.py` * `tests/test_mcp_server.py` * `tests/test_review_proofs.py` Instead of stopping or switching to a clean scratch clone/worktree, the session used multiple `git stash`, `stash pop`, `checkout --`, and `stash drop` operations to manipulate local changes. The final report claims the files were restored, but this violated the reviewer hard rule: > Do not touch another session’s dirty worktree. Use a clean scratch clone/worktree if needed. This should become an enforced wall because reviewer sessions must never manage, stash, reset, or restore unrelated local work. Reviewer validation should happen in a disposable scratch clone or fail closed with a clear handoff. Acceptance criteria: 1. Reviewer workflow detects dirty worktree before checkout, diff, validation, merge, or review. 2. If dirty files exist outside the PR scope, reviewer must either: * stop with a terminal report, or * create/use a clean scratch clone/worktree. 3. Reviewer helpers must not run `git stash`, `git checkout --`, `git reset`, or `git clean` against unrelated local changes. 4. Add tests proving reviewer tasks fail closed or select scratch execution when unrelated dirty files exist. 5. Add tests proving author-side work can continue using normal branches, but reviewer-side work cannot manipulate another session’s dirty state. 6. Update reviewer prompt/template text to explicitly forbid stash/reset/checkout cleanup of unrelated local changes. 7. Add proof output requiring reviewers to report: * starting worktree path * whether it was dirty * whether a scratch clone/worktree was used * confirmation that no unrelated local files were modified, stashed, reset, or dropped Related observed commands: * `git stash` * `git stash push -m "reviewer-temp-stash" -- tests/test_mcp_server.py tests/test_review_proofs.py` * `git checkout -- review_proofs.py tests/test_mcp_server.py tests/test_review_proofs.py` * multiple `git stash pop` * multiple `git stash drop` This is not about reverting PR #95 or PR #231. Those docs PRs may be valid. This issue is about preventing reviewer worktree interference in future sessions.

jcwalker3 added the status:in-progress label 2026-07-06 01:02:32 -05:00

jcwalker3 referenced this issue from a commit

2026-07-06 01:04:40 -05:00

feat(reviewer): fail closed on dirty worktrees and forbidden git cleanup (#233)

jcwalker3 commented

2026-07-06 01:04:44 -05:00

Claimed and implementation started.

Branch: feat/issue-233-reviewer-worktree-gate
Worktree: branches/feat-issue-233-reviewer-worktree-gate
Scope: reviewer_worktree.py, review_proofs.py integration, tests, review-pr.md template

Opening PR shortly.

Claimed and implementation started. - Branch: `feat/issue-233-reviewer-worktree-gate` - Worktree: `branches/feat-issue-233-reviewer-worktree-gate` - Scope: `reviewer_worktree.py`, `review_proofs.py` integration, tests, `review-pr.md` template Opening PR shortly.

jcwalker3 referenced a pull request that will close this issue

2026-07-06 01:04:51 -05:00

feat(reviewer): fail closed on dirty worktrees and forbidden git cleanup (closes #233) #234

sysadmin referenced this issue

2026-07-06 01:06:22 -05:00

feat(reviewer): fail closed on dirty worktrees and forbidden git cleanup (closes #233) #234

sysadmin closed this issue

2026-07-06 01:06:24 -05:00

sysadmin referenced this issue from a commit

2026-07-06 01:06:25 -05:00

Merge pull request 'feat(reviewer): fail closed on dirty worktrees and forbidden git cleanup (closes #233)' (#234) from feat/issue-233-reviewer-worktree-gate into master

sysadmin removed the status:in-progress label 2026-07-06 01:06:26 -05:00