GodopuandClaude Opus 5
e6d2168a13
The byte-identity check of the two-argument form read the old script with `git show 25f4be3:...`, which fails in a `git archive` export (no .git) and failed the test there. The comparison now runs only when the old script can be read; the two-argument path is otherwise still covered by test_b7_diff_untracked. L13 passes in the repository and in an export. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
400 lines
18 KiB
Python
400 lines
18 KiB
Python
"""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)
|
|
new2 = _diff_collect(wt, base)
|
|
assert new2.returncode == 0 and "+modified" in new2.stdout and "+untracked" in new2.stdout
|
|
if old.returncode == 0 and old.stdout: # needs git history (absent in a `git archive` export)
|
|
old_sh = tmp_path / "diff_collect_old.sh"
|
|
old_sh.write_text(old.stdout)
|
|
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
|