fix(issue-21): loop B review follow-up - one verification target per review prompt, dirty submodules refused, identity self-review recorded like any verdict

- review jobs' [워크트리 컨텍스트] points at the [리뷰 identity] path only (B-2); remote re-adjudication gets a matching sentence (N-4) - dirty/untracked submodule content fails the snapshot and makes a round stale, regardless of submodule ignore settings (B-R1) - with --worktree/--base and no reviewer, the creator self-review takes the reviewer path: snapshot diff, evidence check, pre-promotion re-check and the identity sidecar; NOT PASS still re-reviews without a correction job (B-R2/B-R3) - snapshot diff failures keep the UNAVAILABLE marker (N-1); prune only when worktree remove fails (N-3); the empty per-loop review dir is removed (N-5) - tests L17-L21, B-2 assertions in L6/L11; SKILL.md env stale note; import order Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
2026-09-14 22:34:12 +00:00
1 parent e6d2168a13
commit a21986ec3a
@@ -27,6 +27,10 @@ from typing import Any, Dict, List, Optional, Set, Tuple

 from lib_py.loop import checkpoint, eager, parser

+UNAVAILABLE_MARKER = "!!! CHANGE SET UNAVAILABLE !!!"  # diff_collect.sh MAM_DIFF_UNAVAILABLE_MARKER
+SELF_REVIEW_PROMPT = ("작업 완료 상태에 대해 스스로 검증(Self-Review)하여 결함이 없음을 확인하고 종결해주세요. "
+                      "리뷰 리포트 마지막에 단독 행으로 반드시 '[VERDICT: PASS]' 혹은 '[VERDICT: NOT PASS]' 태그를 명시해주세요.")
+
 # ANSI colors matching run_loop.sh
 _COLOR_INFO = "\033[1;34m[*]\033[0m"
 _COLOR_SUCCESS = "\033[1;32m[✓]\033[0m"
@@ -531,6 +535,11 @@ class LoopRunner:
            res = subprocess.run(["bash", "-c", cmd], capture_output=True, text=True, cwd=src)
            if res.returncode == 0:
                return res.stdout
+            if head_commit:
+                # the snapshot's evidence is not replaced by anything else (B13):
+                # the collector's UNAVAILABLE output is passed on as it is
+                return res.stdout if UNAVAILABLE_MARKER in res.stdout else (
+                    f"{UNAVAILABLE_MARKER}\nreason: diff collector failed (rc {res.returncode})\n")
        # Fallback to direct git diff
        try:
            if head_commit:
@@ -538,6 +547,8 @@ class LoopRunner:
            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)
+            if head_commit and res.returncode != 0:
+                return f"{UNAVAILABLE_MARKER}\nreason: git diff failed (rc {res.returncode})\n"
            return res.stdout
        except Exception:
            return ""
@@ -550,9 +561,11 @@ class LoopRunner:
    def _context_active(self) -> bool:
        return self.worktree_given or self.base_ref is not None

-    def _worktree_context(self, remote: bool = False) -> str:
+    def _worktree_context(self, remote: bool = False, purpose: str = "work") -> str:
        """The source-root block put in front of every delegated prompt (D6);
-        empty without --worktree/--base, so prompts stay byte-identical."""
+        empty without --worktree/--base, so prompts stay byte-identical.
+        purpose: 'work' (creator/planner), 'review' (a verdict job, whose
+        target is the [리뷰 identity] path) or 'adjudication' (a re-hearing)."""
        if not self._context_active():
            return ""
        src = self.worktree_root
@@ -570,8 +583,21 @@ class LoopRunner:
        # 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")
+        if purpose == "review" and remote:
+            tail = "이 잡의 검증 대상은 아래 SHA 와 diff 다.\n\n"
+        elif purpose == "review" and self.review_isolation == "worktree":
+            # one verification target per prompt (loop B review B-2)
+            tail = ("리뷰 검증은 아래 [리뷰 identity] 의 검증 경로에서만 한다. 소스 경로는 creator 의 작업 트리이며 "
+                    "리뷰어는 거기서 실행·수정하지 않는다. 브리프와 리포트는 브리프에 적힌 절대경로를 그대로 쓴다.\n\n")
+        elif purpose == "review":
+            tail = ("리뷰 검증은 아래 [리뷰 identity] 의 검증 경로(= 위 소스 경로)에서 하며, 거기서 수정·커밋하지 않는다. "
+                    "브리프와 리포트는 브리프에 적힌 절대경로를 그대로 쓴다.\n\n")
+        elif purpose == "adjudication" and remote:
+            tail = "이 잡은 아래 반론의 재심이다. 판정 근거는 직전 리뷰에서 받은 SHA·diff 와 아래 반론 본문이다.\n\n"
+        elif remote:
+            tail = "이 잡의 검증 대상은 아래 SHA 와 diff 다.\n\n"
+        else:
+            tail = "이 잡의 코드 작업·검증 대상은 위 소스 경로다. 브리프와 리포트는 브리프에 적힌 절대경로를 그대로 쓴다.\n\n"
        return ("[워크트리 컨텍스트]\n"
                + src_line
                + f"- 브랜치: {branch}\n"
@@ -581,9 +607,9 @@ class LoopRunner:
                + tail)

    def _dispatch(self, agent_session: str, agent_type: str, role: str, prompt: str,
-                  remote: bool = False) -> Optional[str]:
+                  remote: bool = False, purpose: str = "work") -> 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(remote) + prompt)
+        return self.delegate_job(agent_session, agent_type, role, self._worktree_context(remote, purpose) + prompt)

    def _change_set_state(self) -> str:
        """'empty' | 'present' | 'unknown' for the source root against base,
@@ -653,7 +679,17 @@ class LoopRunner:

    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."""
+        (the real index, status and refs are untouched); None on failure.
+        A dirty submodule is a failure: the tree records only its gitlink, so
+        its uncommitted content would be neither reviewed nor pinned."""
+        dirty_subs = self._dirty_submodules()
+        if dirty_subs is None:
+            log_error(f"review snapshot: cannot check the submodules of {self.worktree_root}")
+            return None
+        if dirty_subs:
+            log_error(f"review snapshot: submodule(s) with uncommitted or untracked content: {' '.join(dirty_subs)}; "
+                      "the snapshot records only their commit - commit or clean them first")
+            return None
        fd, idx = tempfile.mkstemp(prefix="mam-review-index-")
        os.close(fd)
        os.unlink(idx)
@@ -673,6 +709,18 @@ class LoopRunner:
                except OSError:
                    pass

+    def _dirty_submodules(self) -> Optional[List[str]]:
+        """Checked-out submodules (recursively) with uncommitted or untracked
+        content; None when the check itself fails. Each submodule is asked
+        directly with --ignore-submodules=none, so no submodule.*.ignore or
+        diff.ignoreSubmodules setting can hide it."""
+        script = ('s=$(git status --porcelain --ignore-submodules=none) || exit 1; '
+                  '[ -z "$s" ] || printf "%s\\n" "$displaypath"')
+        r = self._git(self.worktree_root, "submodule", "foreach", "--recursive", "--quiet", script)
+        if r.returncode != 0:
+            return None
+        return [line for line in r.stdout.splitlines() if line.strip()]
+
    def _gitlinks(self, treeish: str) -> Optional[Set[str]]:
        r = self._git(self.worktree_root, "ls-tree", "-r", "-z", treeish)
        if r.returncode != 0:
@@ -735,11 +783,18 @@ class LoopRunner:
    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")
+        r = self._git(self.worktree_root, "worktree", "remove", "--force", path)
        if os.path.isdir(path):
            shutil.rmtree(path, ignore_errors=True)
+        if r.returncode != 0:
+            # only when remove failed: prune drops this entry (its directory is
+            # gone now) - and any other stale one, which is why it is not routine
+            self._git(self.worktree_root, "worktree", "prune")
        self._review_paths.remove(path)
+        try:
+            os.rmdir(os.path.dirname(path))  # TMPDIR/mam-review/<loop_id>, once empty
+        except OSError:
+            pass

    def _close_all_review_trees(self) -> None:
        for p in list(self._review_paths):
@@ -1003,6 +1058,13 @@ class LoopRunner:

        final_passed_reviewers: Dict[str, Tuple[str, Path]] = {}
        identity = self._context_active()
+        # Identity mode: a creator self-review is a verdict like any other, so it
+        # takes the reviewer path - snapshot diff, evidence check, B10 re-check
+        # and sidecar (loop B review B-R2/B-R3). A NOT PASS re-reviews, as before.
+        self_review = identity and not reviewers
+        if self_review:
+            log_warn("No reviewers specified. Conducting Creator Self-Review...")
+            reviewers = [self.creator]

        for loop_count in range(1, self.max_loop + 1):
            log_info(f"Review Loop Iteration {loop_count}/{self.max_loop}...")
@@ -1029,29 +1091,13 @@ class LoopRunner:
            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_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)
+                sr_prompt = SELF_REVIEW_PROMPT
+                sr_jid = self._dispatch(self.creator, creator_type, "Reviewer", sr_prompt)
                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
@@ -1077,11 +1123,15 @@ class LoopRunner:
            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}"
+                if self_review:
+                    r_prompt = f"{SELF_REVIEW_PROMPT} 변경분:\n{changes_diff}"
+                else:
+                    r_prompt = f"다음 구현 사항(작업 목표: {self.task}) 및 누적 변경분(git diff)에 대해 린트, 동작성, 유실 등의 관점에서 교차 코드 리뷰를 수행해주세요. 확인 후 최종 Verdict로 '[VERDICT: PASS]' 혹은 '[VERDICT: NOT PASS]' 태그를 리뷰 리포트 마지막에 단독 행으로 명시적으로 작성해주세요. 만약 단순 버그 수정으로는 부족하고 설계 변경/재작업 수준의 재계획이 필요하다고 판단되면, 리포트 아무 곳에나 단독 행으로 '[ESCALATE: PLANNER]' 태그도 함께 남겨주세요. 변경분:\n{changes_diff}"
                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)
+                r_jid = self._dispatch(rev, r_type, "Reviewer", r_prompt, remote=r_remote,
+                                       purpose="review" if identity else "work")
                if not r_jid:
                    log_error(f"Failed to register review job for '{rev}'.")
                    return 1
@@ -1143,6 +1193,13 @@ class LoopRunner:
                log_success("All reviewers issued [VERDICT: PASS]. Loop completed successfully.")
                break

+            if self_review:
+                log_warn("Self-Review NOT PASS.")
+                if loop_count == self.max_loop:
+                    log_error("Reached max loop count. Self-review loop aborted with failures.")
+                    return 1
+                continue
+
            if loop_count == self.max_loop:
                log_error(f"Reached max loop count ({self.max_loop}). Review loop aborted with failures.")
                return 1
@@ -1198,7 +1255,7 @@ class LoopRunner:
                        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,
-                                                 remote=rt in self._remote_recipients)
+                                                 remote=rt in self._remote_recipients, purpose="adjudication")
                        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)