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-<jid>.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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
This commit is contained in:
1 parent
4ad622e372
commit
e57663ac4c
4 files changed
+748
-17
No files matched your search
@@ -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 <path> Source worktree for diff/base/head (control plane stays here)",
|
||||
" --base <ref> Pin the review diff base (must be an ancestor of the source HEAD)",
|
||||
" --review-only Skip implementation; review <base>..HEAD (requires --base)",
|
||||
" --review-isolation <v> Review tree with --worktree/--base: worktree (default) or env",
|
||||
" --remote-review <v> 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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <path> Source worktree for diff/base/head (control plane stays here)"
|
||||
echo " --base <ref> Pin the review diff base (must be an ancestor of the source HEAD)"
|
||||
echo " --review-only Skip implementation; review <base>..HEAD (requires --base)"
|
||||
echo " --review-isolation <v> Review tree with --worktree/--base: worktree (default) or env"
|
||||
echo " --remote-review <v> 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
|
||||
|
||||
|
||||
Reference in new issue
Block a user