From e57663ac4cf98f68ec93db9fb0e2d528540ca5d3 Mon Sep 17 00:00:00 2001 From: Godopu Date: Mon, 14 Sep 2026 20:54:53 +0000 Subject: [PATCH] feat(issue-21): item 5 review identity - per-round snapshot and snapshot diff, detached review worktree (--review-isolation), stale-round re-review, identity records with evidence, pre-promotion re-check; remote reviewers refused or diff-only (--remote-review) Active with --worktree/--base only; without them prompts, flow and promotion are unchanged (a loop_id is recorded in the checkpoint either way). - Every review round is pinned (B3): the source's HEAD and its working state as a tree built through a temporary index (the real index/status/refs are untouched); a dirty state becomes a ref-less snapshot commit on HEAD (committed as mam-loop, not as the user). An untracked nested repository cannot be pinned and is refused. identity = (loop_id, base, head, tree). - Review isolation (B4): a detached review worktree of the snapshot under TMPDIR (default), or `--review-isolation env` (the source root with env and temp-dir instructions). Failing to open the tree refuses the round - the isolation is never lowered silently. Trees are removed after the round and on any exit (run() wraps the body; release_guard too). - The reviewers' diff comes from the snapshot (B13): diff_collect.sh takes an optional head commit for a commit-to-commit diff; its two-argument form is byte-identical. - Stale rounds (B7/B8): after the round the source is measured again; if it changed, the round's approvals are discarded and the next iteration re-reviews without a correction job (last iteration: rc 1). Only the last non-stale round's PASSes are promoted. - Records (B9/B15): review_rounds in the checkpoint with per-reviewer transport/evidence, and report-.identity.json next to each promoted report: evidence review-tree | source-tree | diff-only. - Pre-promotion re-check (B10): a source changed after approval is not promoted (rc 1). - Remote reviewers (B14, Creator challenge 91c81095): they cannot reach this host's review tree. Checked before anything is delegated: refused by default; `--remote-review diff-only` admits them with the snapshot SHAs and diff as their only evidence (no local path in their prompt), and refuses a round whose diff is truncated or unavailable. - run_loop.sh accepts both options (usage identical to the runner's); the legacy shell loop refuses them before the lock. Tests tests/test_issue21_loop_b.py L1-L16 (22). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G --- .agents/skills/lib_py/loop/runner.py | 334 ++++++++++++++- .../scripts/diff_collect.sh | 23 +- .../multi-agent-mux-loop/scripts/run_loop.sh | 11 +- tests/test_issue21_loop_b.py | 397 ++++++++++++++++++ 4 files changed, 748 insertions(+), 17 deletions(-) create mode 100644 tests/test_issue21_loop_b.py diff --git a/.agents/skills/lib_py/loop/runner.py b/.agents/skills/lib_py/loop/runner.py index 94e0b28..1874fdf 100644 --- a/.agents/skills/lib_py/loop/runner.py +++ b/.agents/skills/lib_py/loop/runner.py @@ -13,12 +13,14 @@ import glob import json import os import re +import secrets import shlex import shutil import signal import sqlite3 import subprocess import sys +import tempfile import time from pathlib import Path from typing import Any, Dict, List, Optional, Set, Tuple @@ -73,6 +75,14 @@ class LoopRunner: self.base_sha = "" self.base_source = "head" self.review_only = False + # Issue #21 item 5: review identity (active with --worktree/--base) + self.loop_id = "loop-" + time.strftime("%Y%m%dT%H%M%SZ", time.gmtime()) + "-" + secrets.token_hex(3) + self.review_isolation = "worktree" + self.remote_review = "refuse" + self._remote_recipients: Set[str] = set() + self._review_paths: List[str] = [] + self._rounds: List[Dict[str, Any]] = [] + self._accepted_round: Optional[Dict[str, Any]] = None self.created_jobs: List[str] = [] self.failed_submit_jobs: Set[str] = set() self.current_plan = "" @@ -111,6 +121,8 @@ class LoopRunner: " --worktree Source worktree for diff/base/head (control plane stays here)", " --base Pin the review diff base (must be an ancestor of the source HEAD)", " --review-only Skip implementation; review ..HEAD (requires --base)", + " --review-isolation Review tree with --worktree/--base: worktree (default) or env", + " --remote-review Remote reviewers with --worktree/--base: refuse (default) or diff-only", ] print("\n".join(lines)) @@ -195,6 +207,19 @@ class LoopRunner: elif arg == "--review-only": self.review_only = True i += 1 + elif arg in ("--review-isolation", "--remote-review"): + if i + 1 >= len(args): + self.print_usage() + sys.exit(1) + allowed = ("worktree", "env") if arg == "--review-isolation" else ("refuse", "diff-only") + if args[i + 1] not in allowed: + print(f"ERROR: {arg} must be one of: {', '.join(allowed)}.", file=sys.stderr) + sys.exit(1) + if arg == "--review-isolation": + self.review_isolation = args[i + 1] + else: + self.remote_review = args[i + 1] + i += 2 else: print(f"Unknown option: {arg}") self.print_usage() @@ -234,6 +259,8 @@ class LoopRunner: if self.base_ref is not None: self.base_sha = self._resolve_base(self.base_ref) self.base_source = "cli" + if not self._context_active() and any(a in self.raw_argv for a in ("--review-isolation", "--remote-review")): + log_warn("--review-isolation/--remote-review only apply with --worktree or --base; ignored.") @staticmethod def _git(cwd: Path, *args: str) -> subprocess.CompletedProcess: @@ -490,21 +517,26 @@ class LoopRunner: return matches[0] return None - def collect_changes_diff(self) -> str: - """Collect git diff since base commit.""" + def collect_changes_diff(self, head_commit: Optional[str] = None) -> str: + """Collect git diff since base commit - of the live source tree, or, + with head_commit (review identity, B13), of base..head_commit.""" # The collector is the control root's copy (never the worktree's # unreviewed one); the tree it diffs is the source root (Issue #21). diff_script = self.workspace_root / ".agents" / "skills" / "multi-agent-mux-loop" / "scripts" / "diff_collect.sh" src = str(self.worktree_root) if diff_script.exists(): cmd = (f"source {shlex.quote(str(diff_script))} && " - f"mam_collect_changes_diff {shlex.quote(src)} {shlex.quote(self.base_commit)}") + f"mam_collect_changes_diff {shlex.quote(src)} {shlex.quote(self.base_commit)}" + + (f" {shlex.quote(head_commit)}" if head_commit else "")) res = subprocess.run(["bash", "-c", cmd], capture_output=True, text=True, cwd=src) if res.returncode == 0: return res.stdout # Fallback to direct git diff try: - cmd = ["git", "diff", self.base_commit] if self.base_commit else ["git", "diff", "HEAD"] + if head_commit: + cmd = ["git", "diff", self.base_commit, head_commit] + else: + cmd = ["git", "diff", self.base_commit] if self.base_commit else ["git", "diff", "HEAD"] res = subprocess.run(cmd, capture_output=True, text=True, cwd=src) return res.stdout except Exception: @@ -518,7 +550,7 @@ class LoopRunner: def _context_active(self) -> bool: return self.worktree_given or self.base_ref is not None - def _worktree_context(self) -> str: + def _worktree_context(self, remote: bool = False) -> str: """The source-root block put in front of every delegated prompt (D6); empty without --worktree/--base, so prompts stay byte-identical.""" if not self._context_active(): @@ -535,17 +567,23 @@ class LoopRunner: count = r.stdout.strip() st = self._git(src, "status", "--porcelain") dirty = "알 수 없음" if st.returncode != 0 else ("있음" if st.stdout.strip() else "없음") + # a remote reviewer (diff-only) cannot reach this host's paths (B6) + src_line = ("- 소스 경로: (오케스트레이터 호스트 전용 — 원격 리뷰어는 접근할 수 없음)\n" if remote + else f"- 소스 경로: {src}\n") + tail = ("이 잡의 검증 대상은 아래 SHA 와 diff 다.\n\n" if remote else + "이 잡의 코드 작업·검증 대상은 위 소스 경로다. 브리프와 리포트는 브리프에 적힌 절대경로를 그대로 쓴다.\n\n") return ("[워크트리 컨텍스트]\n" - f"- 소스 경로: {src}\n" - f"- 브랜치: {branch}\n" + + src_line + + f"- 브랜치: {branch}\n" f"- base: {base or '(Phase 2 시작 시 HEAD 로 정해짐)'} (출처: {origin})\n" f"- head: {head} (base..head 커밋: {count}개, 미커밋 변경: {dirty})\n" f"- 제어 평면(잡 브리프·리포트·레지스트리): {self.workspace_root}\n" - "이 잡의 코드 작업·검증 대상은 위 소스 경로다. 브리프와 리포트는 브리프에 적힌 절대경로를 그대로 쓴다.\n\n") + + tail) - def _dispatch(self, agent_session: str, agent_type: str, role: str, prompt: str) -> Optional[str]: + def _dispatch(self, agent_session: str, agent_type: str, role: str, prompt: str, + remote: bool = False) -> Optional[str]: """delegate_job with the source-root context block in front of the prompt.""" - return self.delegate_job(agent_session, agent_type, role, self._worktree_context() + prompt) + return self.delegate_job(agent_session, agent_type, role, self._worktree_context(remote) + prompt) def _change_set_state(self) -> str: """'empty' | 'present' | 'unknown' for the source root against base, @@ -580,6 +618,174 @@ class LoopRunner: return False return True + # ------------------------------------------------------------------ + # Issue #21 item 5: review identity + # ------------------------------------------------------------------ + def _is_remote(self, name: str) -> bool: + return self.session_label(name).startswith("remote:") + + def _verdict_recipients(self, state: Dict[str, Any]) -> List[str]: + """Who receives a verdict job - the same rule Phase 3 applies; the + creator when there is no reviewer (self-review).""" + if self.all_reviewers: + names = self.resolve_all_reviewers(state) + elif self.reviewer_list: + names = [r.strip() for r in self.reviewer_list.split(",") if r.strip()] + else: + names = [] + return names or [self.creator] + + def _check_remote_recipients(self, state: Dict[str, Any]) -> bool: + """B14: a verdict recipient must be able to reach the evidence recorded + for it. Remote reviewers cannot reach this host's review tree: refused + by default, admitted with the snapshot diff as their only evidence + under --remote-review diff-only. Checked before any delegation.""" + remote = [n for n in self._verdict_recipients(state) if self._is_remote(n)] + self._remote_recipients = set(remote) + if remote and self.remote_review != "diff-only": + log_error(f"identity review needs the review tree on this host; remote reviewer(s) {' '.join(remote)} " + "cannot verify it. Use local reviewers, or pass --remote-review diff-only") + return False + if remote: + log_warn(f"remote reviewer(s) {' '.join(remote)} admitted with the snapshot diff as their only " + "evidence (--remote-review diff-only)") + return True + + def _worktree_tree(self) -> Optional[str]: + """The source worktree's state as a tree, through a temporary index + (the real index, status and refs are untouched); None on failure.""" + fd, idx = tempfile.mkstemp(prefix="mam-review-index-") + os.close(fd) + os.unlink(idx) + env = dict(os.environ, GIT_INDEX_FILE=idx) + try: + for step in (("read-tree", "HEAD"), ("add", "-A"), ("write-tree",)): + r = subprocess.run(["git", "-C", str(self.worktree_root), *step], + capture_output=True, text=True, env=env) + if r.returncode != 0: + log_error(f"review snapshot: git {step[0]} failed in {self.worktree_root}: {r.stderr.strip()}") + return None + return r.stdout.strip() or None + finally: + for p in (idx, idx + ".lock"): + try: + os.unlink(p) + except OSError: + pass + + def _gitlinks(self, treeish: str) -> Optional[Set[str]]: + r = self._git(self.worktree_root, "ls-tree", "-r", "-z", treeish) + if r.returncode != 0: + return None + links = set() + for ent in r.stdout.split("\0"): + meta, _, path = ent.partition("\t") + if meta.split(" ")[0] == "160000": + links.add(path) + return links + + def _identity_now(self) -> Tuple[str, str]: + return (self._source_head(), self._worktree_tree() or "") + + def _snapshot(self, n: int) -> Optional[Dict[str, Any]]: + """B3: pin the source state of review round n. Uncommitted and + untracked (not ignored) files go into a ref-less snapshot commit on + top of HEAD. An untracked nested repository cannot be pinned (only a + gitlink would be) and is refused.""" + head = self._source_head() + tree = self._worktree_tree() if head else None + if not head or not tree: + log_error(f"cannot snapshot {self.worktree_root}; refusing to request reviews") + return None + new_links = self._gitlinks(tree) + old_links = self._gitlinks(head) + if new_links is None or old_links is None: + log_error("cannot list the snapshot's entries; refusing to request reviews") + return None + untracked_repos = sorted(new_links - old_links) + if untracked_repos: + log_error(f"untracked nested git repository {' '.join(untracked_repos)} cannot be pinned into the review " + "snapshot; commit it, ignore it or move it out") + return None + dirty = tree != self._git(self.worktree_root, "rev-parse", "HEAD^{tree}").stdout.strip() + commit = head + if dirty: + ident = {"GIT_AUTHOR_NAME": "mam-loop", "GIT_AUTHOR_EMAIL": "mam-loop@localhost", + "GIT_COMMITTER_NAME": "mam-loop", "GIT_COMMITTER_EMAIL": "mam-loop@localhost"} + c = subprocess.run(["git", "-C", str(self.worktree_root), "commit-tree", tree, "-p", head, + "-m", f"mam review snapshot {self.loop_id} #{n}"], + capture_output=True, text=True, env=dict(os.environ, **ident)) + if c.returncode != 0 or not c.stdout.strip(): + log_error(f"cannot commit the review snapshot: {c.stderr.strip()}") + return None + commit = c.stdout.strip() + return {"head": head, "tree": tree, "dirty": dirty, "review_commit": commit} + + def _open_review_tree(self, snap: Dict[str, Any], n: int) -> Optional[str]: + path = Path(tempfile.gettempdir()) / "mam-review" / self.loop_id / str(n) + path.parent.mkdir(parents=True, exist_ok=True) + r = self._git(self.worktree_root, "worktree", "add", "--detach", str(path), snap["review_commit"]) + if r.returncode != 0: + log_error(f"cannot open the review worktree {path}: {r.stderr.strip()}; refusing to request reviews " + "(the isolation is not lowered silently)") + return None + self._review_paths.append(str(path)) + return str(path) + + def _close_review_tree(self, path: Optional[str]) -> None: + if not path or path not in self._review_paths: + return + self._git(self.worktree_root, "worktree", "remove", "--force", path) + self._git(self.worktree_root, "worktree", "prune") + if os.path.isdir(path): + shutil.rmtree(path, ignore_errors=True) + self._review_paths.remove(path) + + def _close_all_review_trees(self) -> None: + for p in list(self._review_paths): + self._close_review_tree(p) + + def _evidence(self, name: str) -> Dict[str, str]: + if name in self._remote_recipients: + return {"transport": "remote", "evidence": "diff-only", "isolation": "none (remote)"} + if self.review_isolation == "worktree": + return {"transport": "herdr", "evidence": "review-tree", "isolation": "worktree"} + return {"transport": "herdr", "evidence": "source-tree", "isolation": "env"} + + def _identity_block(self, snap: Dict[str, Any], path: Optional[str], remote: bool) -> str: + lines = ["[리뷰 identity]", + f"- loop_id: {self.loop_id}", + f"- base: {self.base_commit}", + f"- head: {snap['head']}", + f"- tree: {snap['tree']}"] + if snap["dirty"]: + lines.append(f"- 미커밋 변경 포함 스냅샷 커밋: {snap['review_commit']}") + if remote: + lines.append("당신의 호스트에는 이 트리가 없다. 자기 체크아웃이나 다른 커밋에서 실행한 결과를 판정 근거로 " + "삼지 말 것. 근거는 아래 diff 뿐이며, 리포트에 'diff 만으로 검토' 를 밝힐 것.") + else: + py = self.workspace_root / ".venv" / "bin" / "python" + py_cmd = str(py) if py.exists() else "python3" + lines.append(f"- 검증 경로: {path}") + lines.append("이 경로에서만 검증하고 커밋·브랜치 변경은 하지 말 것. 테스트는 " + f"`PYTHONDONTWRITEBYTECODE=1 {py_cmd} -m pytest -p no:cacheprovider " + "--basetemp=<검증 경로 밖의 임시 디렉터리>` 로 실행한다.") + return "\n".join(lines) + "\n\n" + + def _record_round(self, n: int, snap: Dict[str, Any], path: Optional[str], jobs: Dict[str, str], + verdicts: Dict[str, str], stale: bool) -> None: + self._rounds.append({ + "iteration": n, "base_sha": self.base_commit, "head_sha": snap["head"], "tree_sha": snap["tree"], + "review_commit": snap["review_commit"], "isolation": self.review_isolation, + "review_path": path or "", "jobs": jobs, "verdicts": verdicts, + "reviewers": {r: self._evidence(r) for r in jobs}, "stale": stale, + }) + self._record_checkpoint(iteration=n) + + def _round_is_stale(self, snap: Dict[str, Any]) -> bool: + """B7: the source changed while the round was being reviewed.""" + return self._identity_now() != (snap["head"], snap["tree"]) + def _record_checkpoint(self, iteration: int = 1, review_status: Optional[Dict[str, str]] = None) -> None: data = { "iteration": iteration, @@ -592,12 +798,16 @@ class LoopRunner: "review_only": self.review_only, "worktree_root": str(self.worktree_root), "head_commit": self._source_head() if self._context_active() else "", + "loop_id": self.loop_id, "timestamp": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()), } + if self._context_active(): + data["review_rounds"] = self._rounds checkpoint.save_checkpoint(self.checkpoint_file, data) def release_guard(self) -> None: """Release MAM_LOOP_MARKER and clean up freeze dir if owned.""" + self._close_all_review_trees() marker = os.environ.get("MAM_LOOP_MARKER") or str(self.mam_dir / "loop-guard-active") if os.path.isfile(marker): try: @@ -614,6 +824,12 @@ class LoopRunner: shutil.rmtree(freeze_dir, ignore_errors=True) def run(self) -> int: + try: + return self._run_body() + finally: + self._close_all_review_trees() # a return or exit mid-round leaves no review tree + + def _run_body(self) -> int: self.parse_args() atexit.register(self.release_guard) @@ -630,6 +846,7 @@ class LoopRunner: signal.signal(signal.SIGTERM, _handle_signal) log_info("Initializing multi-agent-mux-loop controller...") + log_info(f"Loop id: {self.loop_id}") log_info(f"Target Agent: {self.creator}") log_info(f"Task Goal: {self.task}") @@ -672,6 +889,11 @@ class LoopRunner: log_info(f"Resolved Planner session: {planner_session}") + # Issue #21 item 5 (B14): every verdict recipient must reach the + # evidence recorded for it - checked before anything is delegated. + if self._context_active() and not self._check_remote_recipients(state): + return 1 + # ======================================================================= # PHASE 1: PLANNING & DISCUSSIONS # ======================================================================= @@ -780,6 +1002,7 @@ class LoopRunner: rebut_total_budget = self.max_rebut * self.max_loop final_passed_reviewers: Dict[str, Tuple[str, Path]] = {} + identity = self._context_active() for loop_count in range(1, self.max_loop + 1): log_info(f"Review Loop Iteration {loop_count}/{self.max_loop}...") @@ -788,16 +1011,47 @@ class LoopRunner: if not self._change_set_allows_review(): return 1 + # Issue #21 item 5: pin this round (B3), open its review tree (B4) + snap: Optional[Dict[str, Any]] = None + review_path: Optional[str] = None + if identity: + final_passed_reviewers.clear() # B8: only this round's approvals can be promoted + snap = self._snapshot(loop_count) + if snap is None: + return 1 + if self.review_isolation == "worktree": + review_path = self._open_review_tree(snap, loop_count) + if review_path is None: + return 1 + else: + review_path = str(self.worktree_root) + if not reviewers: # Creator Self-Review log_warn("No reviewers specified. Conducting Creator Self-Review...") sr_prompt = "작업 완료 상태에 대해 스스로 검증(Self-Review)하여 결함이 없음을 확인하고 종결해주세요. 리뷰 리포트 마지막에 단독 행으로 반드시 '[VERDICT: PASS]' 혹은 '[VERDICT: NOT PASS]' 태그를 명시해주세요." - sr_jid = self._dispatch(self.creator, creator_type, "Reviewer", sr_prompt) + sr_remote = self.creator in self._remote_recipients + if identity: + sr_prompt = self._identity_block(snap, review_path, sr_remote) + sr_prompt + sr_jid = self._dispatch(self.creator, creator_type, "Reviewer", sr_prompt, remote=sr_remote) if not sr_jid: log_error("Failed to register Self-Review job.") return 1 self.wait_for_job(sr_jid) sr_report = self.find_report(sr_jid) + if identity: + if self.review_isolation == "worktree": + self._close_review_tree(review_path) + sr_stale = self._round_is_stale(snap) + sr_verdict = "PASS" if (sr_report and parser.has_verdict(sr_report, "PASS")) else "NOT PASS" + self._record_round(loop_count, snap, review_path, {self.creator: sr_jid}, + {self.creator: sr_verdict}, sr_stale) + if sr_stale: + log_warn("source changed during review; the self-review is discarded, re-reviewing") + if loop_count == self.max_loop: + log_error("the source changed during review in the last iteration; no verdict to accept") + return 1 + continue if sr_report and parser.has_verdict(sr_report, "PASS") and not parser.has_verdict(sr_report, "NOT PASS"): log_success("Self-Review PASS.") break @@ -809,20 +1063,32 @@ class LoopRunner: continue log_info(f"Active reviewers: {' '.join(reviewers)}") - changes_diff = self.collect_changes_diff() + # B13: in identity mode the reviewers' diff is the snapshot's, so the + # evidence they get is exactly what the verdict is bound to. + changes_diff = self.collect_changes_diff(snap["review_commit"] if identity else None) + diff_only = [r for r in reviewers if r in self._remote_recipients] + if identity and diff_only and ("!!! DIFF TRUNCATED !!!" in changes_diff + or "!!! CHANGE SET UNAVAILABLE !!!" in changes_diff): + log_error(f"the snapshot diff is truncated/unavailable; diff-only remote reviewer(s) " + f"{' '.join(diff_only)} would have no evidence") + return 1 review_jobs: List[Tuple[str, str]] = [] # (rev, job_id) for rev in reviewers: log_info(f"Requesting code review from Reviewer '{rev}'...") r_type = self.resolve_agent_type(rev, state) r_prompt = f"다음 구현 사항(작업 목표: {self.task}) 및 누적 변경분(git diff)에 대해 린트, 동작성, 유실 등의 관점에서 교차 코드 리뷰를 수행해주세요. 확인 후 최종 Verdict로 '[VERDICT: PASS]' 혹은 '[VERDICT: NOT PASS]' 태그를 리뷰 리포트 마지막에 단독 행으로 명시적으로 작성해주세요. 만약 단순 버그 수정으로는 부족하고 설계 변경/재작업 수준의 재계획이 필요하다고 판단되면, 리포트 아무 곳에나 단독 행으로 '[ESCALATE: PLANNER]' 태그도 함께 남겨주세요. 변경분:\n{changes_diff}" - r_jid = self._dispatch(rev, r_type, "Reviewer", r_prompt) + r_remote = rev in self._remote_recipients + if identity: + r_prompt = self._identity_block(snap, review_path, r_remote) + r_prompt + r_jid = self._dispatch(rev, r_type, "Reviewer", r_prompt, remote=r_remote) if not r_jid: log_error(f"Failed to register review job for '{rev}'.") return 1 review_jobs.append((rev, r_jid)) all_passed = True + round_verdicts: Dict[str, str] = {} fail_revs: List[str] = [] fail_reports: List[Path] = [] feedback_parts: List[str] = [] @@ -830,12 +1096,14 @@ class LoopRunner: for rev, jid in review_jobs: if not self.wait_for_job(jid): log_warn(f"Reviewer '{rev}' job crashed.") + round_verdicts[rev] = "crashed" all_passed = False continue r_file = self.find_report(jid) if not r_file or not r_file.exists(): log_warn(f"Reviewer '{rev}' report not found. Counting as NOT PASS.") + round_verdicts[rev] = "missing" all_passed = False continue @@ -844,15 +1112,34 @@ class LoopRunner: if verdict != "PASS": log_warn(f"Reviewer '{rev}': NOT PASS") + round_verdicts[rev] = "NOT PASS" all_passed = False fail_revs.append(rev) fail_reports.append(r_file) feedback_parts.append(f"\n--- Reviewer ({rev}) Feedback ---\n{content}") else: log_success(f"Reviewer '{rev}': PASS") + round_verdicts[rev] = "PASS" final_passed_reviewers[rev] = (jid, r_file) + if identity: + # B7: judge the round against the source as it is now + if self.review_isolation == "worktree": + self._close_review_tree(review_path) + stale = self._round_is_stale(snap) + self._record_round(loop_count, snap, review_path, dict(review_jobs), round_verdicts, stale) + if stale: + final_passed_reviewers.clear() + log_warn(f"source changed during review (head {snap['head'][:12]}, tree {snap['tree'][:12]}); " + "approvals discarded, re-reviewing") + if loop_count == self.max_loop: + log_error("the source changed during review in the last iteration; no approval to accept") + return 1 + continue + if all_passed: + if identity: + self._accepted_round = self._rounds[-1] log_success("All reviewers issued [VERDICT: PASS]. Loop completed successfully.") break @@ -910,7 +1197,8 @@ class LoopRunner: log_info(f"Re-adjudication: returning rebuttal to Reviewer '{rt}'...") rt_type = self.resolve_agent_type(rt, state) adj_prompt = f"작업자(Creator)가 귀하의 리뷰 지적 중 일부를 타당하지 않다고 판단하여 반론을 제기했습니다. 반론을 검토하고 재심(re-adjudication)하십시오. 반론이 타당하여 귀하의 지적을 철회한다면 리포트 마지막에 단독 행으로 '[ADJUDICATION: SUSTAINED]' 를, 반론이 타당하지 않아 지적을 유지한다면 '[ADJUDICATION: OVERRULED]' 를 명시하십시오. 반론 내용:\n{rebuttal_text}" - adj_jid = self._dispatch(rt, rt_type, "Reviewer", adj_prompt) + adj_jid = self._dispatch(rt, rt_type, "Reviewer", adj_prompt, + remote=rt in self._remote_recipients) if not adj_jid or not self.wait_for_job(adj_jid): log_warn(f"Re-adjudication job for '{rt}' failed; objection stands.") overruled_revs.append(rt) @@ -956,6 +1244,13 @@ class LoopRunner: tmp_plan.write_text(self.current_plan, encoding="utf-8") tmp_plan.replace(plan_dest_dir / f"plan-{self.plan_job_id}.md") + if final_passed_reviewers and identity and self._accepted_round: + # B10: an approval is promoted only for the source it approved + acc = self._accepted_round + if self._identity_now() != (acc["head_sha"], acc["tree_sha"]): + log_error("source changed after approval; not promoting") + return 1 + if final_passed_reviewers: log_info("Promoting final review reports to durable location...") for rev, (jid, rep_file) in final_passed_reviewers.items(): @@ -965,6 +1260,17 @@ class LoopRunner: tmp_dest = dest_dir / f"report-{jid}.md.tmp" shutil.copyfile(rep_file, tmp_dest) tmp_dest.replace(dest_file) + if identity and self._accepted_round: + acc = self._accepted_round + ev = self._evidence(rev) + sidecar = {"loop_id": self.loop_id, "base_sha": acc["base_sha"], "head_sha": acc["head_sha"], + "tree_sha": acc["tree_sha"], "review_commit": acc["review_commit"], + "reviewer": rev, "job_id": jid, "verdict": "PASS", **ev, + "verified_at": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())} + tmp_side = dest_dir / f"report-{jid}.identity.json.tmp" + tmp_side.write_text(json.dumps(sidecar, indent=2), encoding="utf-8") + tmp_side.replace(dest_dir / f"report-{jid}.identity.json") + log_info(f" {rev}: report-{jid}.md (evidence: {ev['evidence']})") # Cleanup if self.cleanup: diff --git a/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh b/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh index 3ba1fd4..7c75d76 100755 --- a/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh +++ b/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh @@ -55,6 +55,11 @@ _mam_untracked_diff() { mam_collect_changes_diff() { local repo_root="${1:-}" local base_commit="${2:-}" + # Optional third argument (Issue #21 item 5): a commit - the review + # snapshot - to diff against base, commit to commit. The snapshot already + # holds the untracked files, so no untracked scan; nothing falls back to + # the live tree. + local head_commit="${3:-}" if [ -z "$repo_root" ]; then printf '%s\nreason: repo_root parameter is required\n' "$MAM_DIFF_UNAVAILABLE_MARKER" @@ -70,6 +75,12 @@ mam_collect_changes_diff() { raw_diff=$( cd -P "$repo_root" 2>/dev/null || exit 3 git rev-parse --git-dir >/dev/null 2>&1 || exit 4 + if [ -n "$head_commit" ]; then + git cat-file -e "${base_commit}^{commit}" 2>/dev/null || exit 6 + git cat-file -e "${head_commit}^{commit}" 2>/dev/null || exit 6 + git diff "$base_commit" "$head_commit" || exit 5 + exit 0 + fi if [ -n "$base_commit" ] && git cat-file -e "${base_commit}^{commit}" 2>/dev/null; then git diff "$base_commit" || exit 5 else @@ -86,6 +97,8 @@ mam_collect_changes_diff() { reason="not a git repository: $repo_root" elif [ "$rc" -eq 5 ]; then reason="git diff command failed" + elif [ "$rc" -eq 6 ]; then + reason="base '$base_commit' or head '$head_commit' is not a commit" fi printf '%s\nreason: %s\n' "$MAM_DIFF_UNAVAILABLE_MARKER" "$reason" return 2 @@ -104,6 +117,10 @@ mam_collect_changes_diff() { local stat_summary="" stat_summary=$( cd -P "$repo_root" 2>/dev/null || exit 0 + if [ -n "$head_commit" ]; then + git diff "$base_commit" "$head_commit" --stat 2>/dev/null || true + exit 0 + fi if [ -n "$base_commit" ] && git cat-file -e "${base_commit}^{commit}" 2>/dev/null; then git diff "$base_commit" --stat 2>/dev/null || true else @@ -117,7 +134,11 @@ mam_collect_changes_diff() { printf 'The change set is %s bytes / %s lines, over the review limit (%s bytes / %s lines).\n' \ "$byte_cnt" "$line_cnt" "$MAM_DIFF_MAX_BYTES" "$MAM_DIFF_MAX_LINES" printf 'Only the file-level summary is shown below. You have NOT been shown the\n' - printf 'full change set -- inspect the working tree directly before voting.\n\n' + if [ -n "$head_commit" ]; then + printf 'full change set -- inspect the review snapshot (%s) directly before voting.\n\n' "$head_commit" + else + printf 'full change set -- inspect the working tree directly before voting.\n\n' + fi printf '%s\n\n' "$stat_summary" printf '%s\n' "$MAM_DIFF_TRUNCATED_MARKER" return 0 diff --git a/.agents/skills/multi-agent-mux-loop/scripts/run_loop.sh b/.agents/skills/multi-agent-mux-loop/scripts/run_loop.sh index f83b853..da0cbdf 100644 --- a/.agents/skills/multi-agent-mux-loop/scripts/run_loop.sh +++ b/.agents/skills/multi-agent-mux-loop/scripts/run_loop.sh @@ -35,6 +35,8 @@ PLANNER_SESSION_OVERRIDE="" LOOP_WORKTREE="" LOOP_BASE="" LOOP_REVIEW_ONLY=0 +LOOP_REVIEW_ISOLATION="" +LOOP_REMOTE_REVIEW="" # Print usage instructions usage() { @@ -53,6 +55,8 @@ usage() { echo " --worktree Source worktree for diff/base/head (control plane stays here)" echo " --base Pin the review diff base (must be an ancestor of the source HEAD)" echo " --review-only Skip implementation; review ..HEAD (requires --base)" + echo " --review-isolation Review tree with --worktree/--base: worktree (default) or env" + echo " --remote-review Remote reviewers with --worktree/--base: refuse (default) or diff-only" exit 1 } @@ -92,6 +96,8 @@ while [[ "$#" -gt 0 ]]; do --worktree) LOOP_WORKTREE="$2"; shift 2 ;; --base) LOOP_BASE="$2"; shift 2 ;; --review-only) LOOP_REVIEW_ONLY=1; shift ;; + --review-isolation) LOOP_REVIEW_ISOLATION="$2"; shift 2 ;; + --remote-review) LOOP_REMOTE_REVIEW="$2"; shift 2 ;; -h|--help) usage ;; *) echo "Unknown option: $1"; usage ;; esac @@ -117,8 +123,9 @@ fi # diffs the checkout it runs in; the worktree/base options are Python-only. # Checked before the freeze and the loop lock (log_* are not defined yet). if [ "${MAM_LOOP_PY:-1}" = "0" ] && { [ -n "$LOOP_WORKTREE" ] || [ -n "$LOOP_BASE" ] \ - || [ "$LOOP_REVIEW_ONLY" = "1" ] || [ -n "${MAM_LOOP_WORKTREE:-}" ]; }; then - echo "ERROR: --worktree/--base/--review-only require the Python loop (MAM_LOOP_PY=1)" >&2 + || [ "$LOOP_REVIEW_ONLY" = "1" ] || [ -n "${MAM_LOOP_WORKTREE:-}" ] \ + || [ -n "$LOOP_REVIEW_ISOLATION" ] || [ -n "$LOOP_REMOTE_REVIEW" ]; }; then + echo "ERROR: --worktree/--base/--review-only/--review-isolation/--remote-review require the Python loop (MAM_LOOP_PY=1)" >&2 exit 1 fi diff --git a/tests/test_issue21_loop_b.py b/tests/test_issue21_loop_b.py new file mode 100644 index 0000000..f7742f2 --- /dev/null +++ b/tests/test_issue21_loop_b.py @@ -0,0 +1,397 @@ +"""Issue #21 phase 2, loop B, item 5: review identity. + +With --worktree/--base every review round is pinned to a snapshot of the +source (loop_id, base, head, tree), reviewed in a detached review worktree (or +env isolation), judged stale when the source changes during the round, and +re-checked before promotion. Remote reviewers cannot reach this host's review +tree: refused by default, admitted diff-only on request, recorded as such. +Without the options nothing changes. All repositories live under tmp_path. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from test_issue21_worktree_prereqs import GIT_ID, RUN_LOOP_SH, SKILLS, _git, wt_repo # noqa: F401 + +DIFF_SH = SKILLS / "multi-agent-mux-loop" / "scripts" / "diff_collect.sh" +REMOTE_ROW = {"name": "reviewer-far-01", "agent": "claude", "role": "reviewer", "status": "running", + "transport": "remote"} +LOCAL_ROWS = [{"name": "creator-claude-01", "agent": "claude", "role": "creator", "status": "running"}, + {"name": "reviewer-claude-01", "agent": "claude", "role": "reviewer", "status": "running"}] + + +def _state(monkeypatch, rows): + monkeypatch.setenv("MAM_STATE_JSON", json.dumps({"herdr_sessions": rows})) + + +def _drive(monkeypatch, tmp_path, argv, on_review=None, reviewers="reviewer-claude-01", max_loop=1): + """run() with delegation stubbed: every job completes, every report is a + PASS; returns (rc, [(session, role, prompt)], runner).""" + from lib_py.loop import runner as lr + monkeypatch.setattr(lr.signal, "signal", lambda *a, **k: None) + args = ["--creator", "creator-claude-01", "--task", "t21", "--max-loop", str(max_loop), *argv] + if reviewers: + args += ["--reviewer", reviewers] + r = lr.LoopRunner(args) + calls = [] + report = tmp_path / "pass-report.md" + report.write_text("looks fine\n\n[VERDICT: PASS]\n") + + def fake_delegate(session, agent_type, role, prompt): + calls.append((session, role, prompt)) + if role == "Reviewer" and on_review: + on_review(r, len(calls), prompt) + return f"j{len(calls)}" + + monkeypatch.setattr(r, "delegate_job", fake_delegate) + monkeypatch.setattr(r, "wait_for_job", lambda *a, **k: True) + monkeypatch.setattr(r, "find_report", lambda jid: report) + return r.run(), calls, r + + +def _reviews(calls): + return [(s, p) for s, role, p in calls if role == "Reviewer"] + + +def _impl(wt: Path) -> str: + """Commit an implementation in the worktree; return the pre-implementation sha.""" + before = _git(wt, "rev-parse", "HEAD") + (wt / "impl.txt").write_text("impl line\n") + _git(wt, "add", "impl.txt") + _git(wt, "commit", "-q", "-m", "impl") + return before + + +def _worktree_count(main: Path) -> int: + return _git(main, "worktree", "list", "--porcelain").count("\nworktree ") + 1 + + +def _sidecars(main: Path) -> list: + return sorted((main / ".agents" / "reports").rglob("*.identity.json")) + + +# --------------------------------------------------------------------------- + +def test_l1_without_options_nothing_changes(wt_repo, monkeypatch, tmp_path): + main, _ = wt_repo + (main / "a.txt").write_text("a\nchanged in main\n") + n = _worktree_count(main) + rc, calls, r = _drive(monkeypatch, tmp_path, []) + assert rc == 0 + assert all("[리뷰 identity]" not in p and "[워크트리 컨텍스트]" not in p for _, _, p in calls) + assert _worktree_count(main) == n + assert r.loop_id.startswith("loop-") and r._rounds == [] + assert _sidecars(main) == [] + + +def test_l2_snapshot_pins_uncommitted_and_untracked_not_ignored(wt_repo): + main, wt = wt_repo + (wt / ".gitignore").write_text("*.log\n") + _git(wt, "add", ".gitignore") + _git(wt, "commit", "-q", "-m", "ignore logs") + (wt / "a.txt").write_text("a\nmodified\n") + (wt / "new.txt").write_text("untracked\n") + (wt / "x.log").write_text("ignored\n") + status0 = _git(wt, "status", "--porcelain") + refs0 = _git(wt, "for-each-ref") + from lib_py.loop.runner import LoopRunner + r = LoopRunner(["--creator", "creator-claude-01", "--task", "t", "--worktree", str(wt)]) + r.parse_args() + r.base_commit = _git(wt, "rev-parse", "HEAD") + snap = r._snapshot(1) + assert snap and snap["dirty"] and snap["review_commit"] != snap["head"] + assert r._worktree_tree() == snap["tree"], "the snapshot tree is stable for an unchanged tree" + path = Path(r._open_review_tree(snap, 1)) + try: + assert (path / "new.txt").read_text() == "untracked\n" + assert "modified" in (path / "a.txt").read_text() + assert not (path / "x.log").exists(), "ignored files stay out of the snapshot" + assert _git(path, "rev-parse", "HEAD") == snap["review_commit"] + finally: + r._close_review_tree(str(path)) + assert not path.exists() + assert _git(wt, "status", "--porcelain") == status0 and _git(wt, "for-each-ref") == refs0 + + +def test_l3_l5_stale_round_is_re_reviewed_and_the_sidecar_matches(wt_repo, monkeypatch, tmp_path): + main, wt = wt_repo + before = _impl(wt) + seen = {} + + def creator_commits_during_first_review(r, n, prompt): + if "h1" not in seen: + seen["h1"] = _git(wt, "rev-parse", "HEAD") + (wt / "late.txt").write_text("late\n") + _git(wt, "add", "late.txt") + _git(wt, "commit", "-q", "-m", "late") + seen["h2"] = _git(wt, "rev-parse", "HEAD") + + rc, calls, r = _drive(monkeypatch, tmp_path, ["--worktree", str(wt), "--base", before, "--review-only"], + on_review=creator_commits_during_first_review, max_loop=2) + assert rc == 0 + assert [role for _, role, _ in calls] == ["Reviewer", "Reviewer"], "no correction job for a stale round" + assert r._rounds[0]["stale"] is True and r._rounds[1]["stale"] is False + assert r._rounds[1]["head_sha"] == seen["h2"] + assert f"- head: {seen['h2']}" in _reviews(calls)[1][1] + (side,) = _sidecars(main) + data = json.loads(side.read_text()) + assert data["loop_id"] == r.loop_id and data["head_sha"] == seen["h2"] + assert data["tree_sha"] == r._rounds[1]["tree_sha"] and data["base_sha"] == before + assert (data["transport"], data["evidence"], data["isolation"]) == ("herdr", "review-tree", "worktree") + + +def test_l3b_stale_last_round_fails(wt_repo, monkeypatch, tmp_path, capsys): + main, wt = wt_repo + before = _impl(wt) + + def commit(r, n, prompt): + (wt / f"late{n}.txt").write_text("late\n") + _git(wt, "add", ".") + _git(wt, "commit", "-q", "-m", "late") + + rc, calls, r = _drive(monkeypatch, tmp_path, ["--worktree", str(wt), "--base", before, "--review-only"], + on_review=commit, max_loop=1) + assert rc == 1 and "changed during review" in capsys.readouterr().err + assert _sidecars(main) == [] + + +def test_l4_source_changed_after_approval_is_not_promoted(wt_repo, monkeypatch, tmp_path, capsys): + main, wt = wt_repo + before = _impl(wt) + from lib_py.loop import runner as lr + real = lr.LoopRunner._identity_now + state = {"accepted": False} + + def identity_now(self): + return ("changed", "changed") if state["accepted"] else real(self) + + def mark(r, n, prompt): + state["accepted"] = False + + monkeypatch.setattr(lr.LoopRunner, "_identity_now", identity_now) + monkeypatch.setattr(lr.LoopRunner, "_round_is_stale", + lambda self, snap: state.__setitem__("accepted", True) or False) + rc, calls, r = _drive(monkeypatch, tmp_path, ["--worktree", str(wt), "--base", before, "--review-only"], + on_review=mark) + assert rc == 1 and "source changed after approval" in capsys.readouterr().err + assert not list((main / ".agents" / "reports").rglob("report-*.md")) + + +def test_l6_env_isolation_reviews_the_source_tree(wt_repo, monkeypatch, tmp_path): + main, wt = wt_repo + before = _impl(wt) + n = _worktree_count(main) + counts = [] + rc, calls, r = _drive(monkeypatch, tmp_path, + ["--worktree", str(wt), "--base", before, "--review-only", "--review-isolation", "env"], + on_review=lambda r, i, p: counts.append(_worktree_count(main))) + assert rc == 0 and counts == [n], "env isolation opens no review worktree" + prompt = _reviews(calls)[0][1] + assert f"- 검증 경로: {os.path.realpath(wt)}" in prompt and "PYTHONDONTWRITEBYTECODE=1" in prompt + data = json.loads(_sidecars(main)[0].read_text()) + assert (data["evidence"], data["isolation"]) == ("source-tree", "env") + + +def test_l7_review_tree_failure_refuses_without_lowering_isolation(wt_repo, monkeypatch, tmp_path, capsys): + main, wt = wt_repo + before = _impl(wt) + from lib_py.loop import runner as lr + real_git = lr.LoopRunner._git + + def git(cwd, *args): + if args[:2] == ("worktree", "add"): + return subprocess.CompletedProcess(args, 128, "", "fatal: simulated") + return real_git(cwd, *args) + + monkeypatch.setattr(lr.LoopRunner, "_git", staticmethod(git)) + rc, calls, r = _drive(monkeypatch, tmp_path, ["--worktree", str(wt), "--base", before, "--review-only"]) + assert rc == 1 and _reviews(calls) == [] + assert "cannot open the review worktree" in capsys.readouterr().err + + +def test_l8_release_guard_closes_open_review_trees(wt_repo): + main, wt = wt_repo + (wt / "a.txt").write_text("a\nx\n") + from lib_py.loop.runner import LoopRunner + r = LoopRunner(["--creator", "creator-claude-01", "--task", "t", "--worktree", str(wt)]) + r.parse_args() + r.base_commit = _git(wt, "rev-parse", "HEAD") + path = r._open_review_tree(r._snapshot(1), 1) + assert os.path.isdir(path) + r.release_guard() + assert not os.path.exists(path) and path not in _git(main, "worktree", "list") + + +@pytest.mark.parametrize("opt", [["--review-isolation", "env"], ["--remote-review", "diff-only"]]) +def test_l9_l16_legacy_shell_loop_refuses_the_new_options(tmp_path, opt): + marker = tmp_path / "loop-marker" + env = {**os.environ, "WORKSPACE_ROOT": str(tmp_path), "MAM_LOOP_PY": "0", "MAM_LOOP_NO_FREEZE": "1", + "MAM_LOOP_MARKER": str(marker)} + res = subprocess.run(["bash", str(RUN_LOOP_SH), "--creator", "c", "--task", "t", *opt], + capture_output=True, text=True, env=env, cwd=str(tmp_path), timeout=60) + assert res.returncode == 1 and "require the Python loop (MAM_LOOP_PY=1)" in res.stderr + assert not marker.exists() + + +def test_l9b_invalid_values_are_rejected(wt_repo): + from lib_py.loop.runner import LoopRunner + for opt in (["--review-isolation", "bogus"], ["--remote-review", "maybe"]): + with pytest.raises(SystemExit) as e: + LoopRunner(["--creator", "c", "--task", "t", *opt]).parse_args() + assert e.value.code == 1 + + +@pytest.mark.parametrize("variant", ["reviewer", "all-reviewer", "env", "self-review"]) +def test_l10_remote_reviewer_refused_before_any_delegation(wt_repo, monkeypatch, tmp_path, capsys, variant): + main, wt = wt_repo + before = _impl(wt) + rows = LOCAL_ROWS + [REMOTE_ROW] + argv = ["--worktree", str(wt), "--base", before, "--review-only"] + reviewers = "reviewer-far-01" + if variant == "all-reviewer": + argv.append("--all-reviewer") + reviewers = None + elif variant == "env": + argv += ["--review-isolation", "env"] + elif variant == "self-review": + rows = [dict(LOCAL_ROWS[0], transport="remote")] + reviewers = None + _state(monkeypatch, rows) + rc, calls, r = _drive(monkeypatch, tmp_path, argv, reviewers=reviewers) + err = capsys.readouterr().err + assert rc == 1 and calls == [], "nothing is delegated" + assert "--remote-review diff-only" in err + assert ("creator-claude-01" if variant == "self-review" else "reviewer-far-01") in err + + +def test_l11_diff_only_remote_reviewer_gets_shas_and_the_snapshot_diff(wt_repo, monkeypatch, tmp_path): + main, wt = wt_repo + before = _impl(wt) + (wt / "a.txt").write_text("a\nuncommitted change\n") + _state(monkeypatch, LOCAL_ROWS + [REMOTE_ROW]) + rc, calls, r = _drive(monkeypatch, tmp_path, + ["--worktree", str(wt), "--base", before, "--review-only", "--remote-review", "diff-only"], + reviewers="reviewer-claude-01,reviewer-far-01") + assert rc == 0 + prompts = dict(_reviews(calls)) + remote, local = prompts["reviewer-far-01"], prompts["reviewer-claude-01"] + snap = r._rounds[0] + assert os.path.realpath(wt) not in remote and "검증 경로" not in remote and "mam-review" not in remote + assert f"- head: {snap['head_sha']}" in remote and f"- tree: {snap['tree_sha']}" in remote + assert "diff 만으로" in remote and "+uncommitted change" in remote and "+impl line" in remote + assert "- 검증 경로: " in local and "mam-review" in local + assert snap["reviewers"]["reviewer-far-01"]["evidence"] == "diff-only" + assert snap["reviewers"]["reviewer-claude-01"]["evidence"] == "review-tree" + side = {json.loads(p.read_text())["reviewer"]: json.loads(p.read_text()) for p in _sidecars(main)} + assert (side["reviewer-far-01"]["evidence"], side["reviewer-far-01"]["isolation"], + side["reviewer-far-01"]["transport"]) == ("diff-only", "none (remote)", "remote") + assert side["reviewer-claude-01"]["evidence"] == "review-tree" + + +def test_l12_remote_reviewer_without_options_works_as_before(wt_repo, monkeypatch, tmp_path, capsys): + main, _ = wt_repo + (main / "a.txt").write_text("a\nchanged\n") + _state(monkeypatch, LOCAL_ROWS + [REMOTE_ROW]) + rc, calls, r = _drive(monkeypatch, tmp_path, ["--remote-review", "diff-only"], reviewers="reviewer-far-01") + assert rc == 0 and [s for s, _ in _reviews(calls)] == ["reviewer-far-01"] + assert "[리뷰 identity]" not in _reviews(calls)[0][1] and _sidecars(main) == [] + assert "only apply with --worktree or --base" in capsys.readouterr().out + + +def _diff_collect(repo, *args, env=None): + cmd = "source " + str(DIFF_SH) + " && mam_collect_changes_diff " + " ".join(f"'{a}'" for a in (repo, *args)) + return subprocess.run(["bash", "-c", cmd], capture_output=True, text=True, env={**os.environ, **(env or {})}) + + +def test_l13_snapshot_diff_is_commit_to_commit(wt_repo, monkeypatch, tmp_path): + main, wt = wt_repo + base = _git(wt, "rev-parse", "HEAD") + (wt / ".gitignore").write_text("*.log\n") + (wt / "a.txt").write_text("a\nmodified\n") + (wt / "new.txt").write_text("untracked\n") + (wt / "x.log").write_text("ignored\n") + from lib_py.loop.runner import LoopRunner + r = LoopRunner(["--creator", "creator-claude-01", "--task", "t", "--worktree", str(wt)]) + r.parse_args() + r.base_commit = base + snap = r._snapshot(1) + out = _diff_collect(wt, base, snap["review_commit"]) + assert out.returncode == 0 and "+modified" in out.stdout and "+untracked" in out.stdout + assert "x.log" not in out.stdout + bad = _diff_collect(wt, base, "0" * 40) + assert bad.returncode == 2 and "CHANGE SET UNAVAILABLE" in bad.stdout + cut = _diff_collect(wt, base, snap["review_commit"], env={"MAM_DIFF_MAX_LINES": "1"}) + assert "DIFF TRUNCATED" in cut.stdout and f"inspect the review snapshot ({snap['review_commit']})" in cut.stdout + # the two-argument form is unchanged: same output as the script before this change + old = subprocess.run(["git", "-C", str(SKILLS.parent.parent), "show", "25f4be3:.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh"], + capture_output=True, text=True) + old_sh = tmp_path / "diff_collect_old.sh" + old_sh.write_text(old.stdout) + new2 = _diff_collect(wt, base) + old2 = subprocess.run(["bash", "-c", f"source '{old_sh}' && mam_collect_changes_diff '{wt}' '{base}'"], + capture_output=True, text=True) + assert (new2.returncode, new2.stdout) == (old2.returncode, old2.stdout) + + +def test_l13b_prompt_diff_comes_from_the_snapshot(wt_repo, monkeypatch, tmp_path): + main, wt = wt_repo + before = _impl(wt) + from lib_py.loop import runner as lr + real = lr.LoopRunner._snapshot + + def snapshot_then_edit(self, n): + s = real(self, n) + (wt / "after-snapshot.txt").write_text("written after the snapshot\n") + return s + + monkeypatch.setattr(lr.LoopRunner, "_snapshot", snapshot_then_edit) + rc, calls, r = _drive(monkeypatch, tmp_path, ["--worktree", str(wt), "--base", before, "--review-only"]) + prompt = _reviews(calls)[0][1] + assert "+impl line" in prompt and "after-snapshot.txt" not in prompt + assert rc == 1, "the source changed after the snapshot: the round is stale" + + +def test_l14_untracked_nested_repository_is_refused(wt_repo, monkeypatch, tmp_path, capsys): + main, wt = wt_repo + before = _impl(wt) + nested = wt / "nested" + nested.mkdir() + _git(nested, "init", "-q") + (nested / "n.txt").write_text("n\n") + _git(nested, "add", "n.txt") + _git(nested, "commit", "-q", "-m", "n") + rc, calls, r = _drive(monkeypatch, tmp_path, ["--worktree", str(wt), "--base", before, "--review-only"]) + assert rc == 1 and _reviews(calls) == [] + assert "untracked nested git repository nested" in capsys.readouterr().err + + +def test_l14b_nested_repository_without_options_proceeds(wt_repo, monkeypatch, tmp_path): + main, _ = wt_repo + nested = main / "nested" + nested.mkdir() + _git(nested, "init", "-q") + rc, calls, r = _drive(monkeypatch, tmp_path, []) + assert rc == 0 and len(_reviews(calls)) == 1 + + +def test_l15_truncated_diff_refuses_diff_only_but_not_local(wt_repo, monkeypatch, tmp_path, capsys): + main, wt = wt_repo + before = _impl(wt) + # the runner uses the control root's collector (the python fallback never truncates) + dst = main / ".agents" / "skills" / "multi-agent-mux-loop" / "scripts" + dst.mkdir(parents=True) + (dst / "diff_collect.sh").write_text(DIFF_SH.read_text()) + monkeypatch.setenv("MAM_DIFF_MAX_LINES", "1") + _state(monkeypatch, LOCAL_ROWS + [REMOTE_ROW]) + argv = ["--worktree", str(wt), "--base", before, "--review-only", "--remote-review", "diff-only"] + rc, calls, r = _drive(monkeypatch, tmp_path, argv, reviewers="reviewer-claude-01,reviewer-far-01") + assert rc == 1 and _reviews(calls) == [] and "would have no evidence" in capsys.readouterr().err + rc, calls, r = _drive(monkeypatch, tmp_path, argv, reviewers="reviewer-claude-01") + assert rc == 0 and len(_reviews(calls)) == 1