diff --git a/gitea_saas/config.py b/gitea_saas/config.py
index d4950c0..1a48c53 100644
--- a/gitea_saas/config.py
+++ b/gitea_saas/config.py
@@ -120,6 +120,8 @@ def load_review_config(env: Optional[Mapping[str, str]] = None) -> dict:
| REVIEW_TEST_TIMEOUT_S | test/build timeout seconds | "300" |
| REVIEW_SECRET_SCAN | secret scan on/off ("0" disables) | "1" |
| REVIEW_WORKTREES_DIR | worktree root where PRs are checked out | (none) |
+ | REVIEW_CLONE_DIR | READ-ONLY repository clone used to build the | (none) |
+ | | isolated synthetic-merge review workspace | |
"""
source: Mapping[str, str] = os.environ if env is None else env
timeout = 300
@@ -138,4 +140,5 @@ def load_review_config(env: Optional[Mapping[str, str]] = None) -> dict:
"test_timeout_s": timeout,
"secret_scan": source.get("REVIEW_SECRET_SCAN", "1") != "0",
"worktrees_dir": source.get("REVIEW_WORKTREES_DIR") or None,
+ "review_clone_dir": source.get("REVIEW_CLONE_DIR") or None,
}
diff --git a/gitea_saas/gitea_client.py b/gitea_saas/gitea_client.py
index 65b8b90..06ea1ff 100644
--- a/gitea_saas/gitea_client.py
+++ b/gitea_saas/gitea_client.py
@@ -167,6 +167,7 @@ class GiteaClient:
"number": pr.get("number"),
"body": pr.get("body") or "",
"head_sha": head.get("sha"),
+ "base_sha": base.get("sha"),
"base_ref": base.get("ref"),
"labels": labels,
"repo": repo,
@@ -262,9 +263,10 @@ class GiteaClient:
def get_pr(self, repo: str, number: int) -> dict:
"""Fetch a PR in the normalized candidate shape.
- ``{number, body, head_sha, base_ref, labels: [names], repo, merged,
- state, changed_files: [filename, ...]}`` — the review gate consumes
- ``head_sha`` and ``changed_files``; policy consumes the rest
+ ``{number, body, head_sha, base_sha, base_ref, labels: [names],
+ repo, merged, state, changed_files: [filename, ...]}`` — the review
+ gate consumes ``head_sha``/``base_sha`` (the exact evaluated pair,
+ issue #30) and ``changed_files``; policy consumes the rest
(``merged``/``state`` let recovery positively distinguish a merged
PR from a merely closed one).
"""
diff --git a/gitea_saas/review_policy.py b/gitea_saas/review_policy.py
index 73656ac..c3fd422 100644
--- a/gitea_saas/review_policy.py
+++ b/gitea_saas/review_policy.py
@@ -14,7 +14,8 @@ Seams (all duck-typed, faked in tests):
normalized PR shape carries ``merged`` (bool) and ``state`` so the
policy can positively distinguish a merged PR from a merely closed one.
* ``reviewer`` — the review gate: ``review_pr(repo, pr, linked_issue,
- repo_dir=...) -> Verdict`` and ``post_verdict(repo, pr, verdict)``.
+ workspace=...) -> Verdict`` (the isolated synthetic-merge workspace,
+ issue #30) and ``post_verdict(repo, pr, verdict)``.
Policy
------
@@ -33,6 +34,16 @@ head moves. The table is bootstrapped here (``CREATE TABLE IF NOT EXISTS``)
because ``schema.sql`` belongs to issue #1's PR and is out of this issue's
allowed file set — same schema intent, idempotent.
+Evaluated-pair integrity (issue #30): the reviewer evaluates a synthetic
+merge of the exact head onto the exact base and the verdict pins both
+SHAs. Before ANY verdict is applied, the policy re-reads the PR and
+refuses to apply the verdict unless BOTH the head and the base are still
+the evaluated pair — a verdict describing a pair that no longer exists is
+never acted on (no merge, label, comment, or close), and the new pair is
+simply eligible for a fresh review on the next poll. The merge request
+itself additionally pins the reviewed head (``expected_head_sha``, issue
+#28) as a second, tighter guard.
+
Transactional outcome application (issue #29): a head is recorded into
``reviews`` (i.e. considered COMPLETED) only after its required policy
action has fully succeeded. Failure/incomplete attempts are persisted in
@@ -178,6 +189,14 @@ class ReviewPolicy:
self._reviews_conn.execute(_REVIEW_STATE_DDL)
self._ensure_review_state_columns()
self._reviews_conn.commit()
+ # Isolated review workspace (issue #30): reviews run against a clean
+ # temporary synthetic-merge worktree taken from the configured clone
+ # (REVIEW_CLONE_DIR), never against an arbitrary worktrees root.
+ self._workspace = None
+ clone_dir = self.config.get("review_clone_dir")
+ if clone_dir:
+ from .review_workspace import ReviewWorkspace
+ self._workspace = ReviewWorkspace(clone_dir, self.reviewer.runner)
def _ensure_review_state_columns(self) -> None:
"""Idempotently add columns introduced after the first #29 build."""
@@ -361,14 +380,48 @@ class ReviewPolicy:
pr_body = pr.get("body") or ""
issue_body = self._fetch_issue_body(repo, linked_issue_number(pr_body))
verdict = self.reviewer.review_pr(
- repo, pr_number, issue_body,
- repo_dir=self.config.get("worktrees_dir"))
+ repo, pr_number, issue_body, workspace=self._workspace)
except Exception as exc: # noqa: BLE001 - any failure must be retryable
self._record_failure(repo_key, pr_number, head_sha, None,
self._error_text(exc))
results.append(self._failure_result(repo, pr_number, head_sha,
None, exc))
continue
+ # Evaluated-pair integrity (issue #30): the verdict pins the exact
+ # head/base it evaluated. Re-read the PR BEFORE applying anything
+ # and refuse when either end of the pair moved — a verdict for a
+ # pair that no longer exists must never merge/label/comment/close,
+ # and the new pair stays eligible for a fresh review on the next
+ # poll (nothing is recorded as completed for the stale pair).
+ try:
+ live_pr = self.gitea.get_pr(repo_key, pr_number) or {}
+ pair_moved = (live_pr.get("head_sha") != verdict.head_sha
+ or live_pr.get("base_sha") != verdict.base_sha)
+ except Exception as exc: # noqa: BLE001 - re-read failure is retryable
+ self._record_failure(repo_key, pr_number, head_sha, verdict.verdict,
+ self._error_text(exc))
+ results.append(self._failure_result(repo, pr_number, head_sha,
+ verdict.verdict, exc))
+ continue
+ if pair_moved:
+ results.append({
+ "repo": repo, "pr": pr_number,
+ "head_sha": head_sha, "verdict": verdict.verdict,
+ "outcome": "stale",
+ "error": {
+ "code": "PAIR_MOVED",
+ "message": (
+ f"PR pair moved after evaluation: evaluated head "
+ f"{verdict.head_sha!r} / base {verdict.base_sha!r}, "
+ f"live head {live_pr.get('head_sha')!r} / base "
+ f"{live_pr.get('base_sha')!r} — verdict NOT applied; "
+ "the new pair is eligible for a fresh review")},
+ "evaluated": {"head_sha": verdict.head_sha,
+ "base_sha": verdict.base_sha},
+ "live": {"head_sha": live_pr.get("head_sha"),
+ "base_sha": live_pr.get("base_sha")},
+ })
+ continue
try:
action = self._apply_policy(repo, pr, pr_body, issue_body, verdict)
except Exception as exc: # noqa: BLE001 - any failure must be retryable
diff --git a/gitea_saas/review_workspace.py b/gitea_saas/review_workspace.py
new file mode 100644
index 0000000..db10b60
--- /dev/null
+++ b/gitea_saas/review_workspace.py
@@ -0,0 +1,256 @@
+"""Isolated review workspace (issue #30).
+
+The review gate must never run tests against an arbitrary ``repo_dir``:
+a green verdict must describe the exact synthetic merge result it
+actually evaluated. This module builds that result in a clean,
+isolated, temporary Git worktree taken from a configured repository
+clone —
+
+* the workspace never changes or tests a shared ``main`` checkout or a
+ worker's issue worktree (the configured clone is only ever *read* by
+ ``git fetch`` / ``git worktree add``; its checked-out branch and
+ working tree are untouched);
+* the named base SHA and PR head SHA are fetched into the clone and the
+ exact head is merged onto the exact base (a real merge commit, so
+ tests and ``git diff --check HEAD`` run against a real,
+ inspectable merge result with an explicit base/result range);
+* a merge conflict produces a deterministic, evidence-carrying result
+ (``merge_ok=False``) and the caller — the review gate — never runs the
+ test suite against some fallback checkout;
+* the temporary worktree is removed on success, conflict, timeout, and
+ raised exceptions (best-effort ``git worktree remove --force`` plus a
+ belt-and-braces directory removal), and no branch refs are ever
+ created (the worktree is detached), so no temporary refs survive.
+
+All git commands go through the same ``Runner`` subprocess seam as the
+rest of the review gate, so hermetic tests can use local-path ``origin``
+remotes (zero network) and/or a scripted command runner to assert the
+exact commands, working directories, and ranges.
+"""
+
+from __future__ import annotations
+
+import re
+import shlex
+import shutil
+import tempfile
+from pathlib import Path
+from typing import Callable, Optional
+
+_SHA_RE = re.compile(r"^[0-9a-f]{40}$")
+
+# Characters kept in a mkdtemp prefix token. ``repo`` names routinely
+# contain separators (``owner/name``) and punctuation; a prefix is a bare
+# filename component, so everything else is collapsed to ``-`` (and the
+# results trimmed) to keep ``tempfile.mkdtemp`` able to create it.
+_FS_SAFE_RE = re.compile(r"[^A-Za-z0-9_-]+")
+
+
+def _fs_safe_token(value: str) -> str:
+ """Collapse ``value`` into a filesystem-safe prefix token."""
+ return _FS_SAFE_RE.sub("-", value).strip("-")
+
+_MERGE_MESSAGE = "synthetic merge for review"
+
+
+class ReviewWorkspaceError(Exception):
+ """The isolated workspace could not be constructed (fetch/resolve
+ failure, missing identity, etc.). Not a merge conflict — the caller
+ treats it as a retryable operational failure."""
+
+
+class MergeWorkspace:
+ """A constructed synthetic-merge workspace (context manager).
+
+ Attributes
+ ----------
+ dir : Path
+ The temporary worktree (working directory for test/build/hygiene
+ commands). HEAD is the synthetic merge commit of ``head_sha``
+ onto ``base_sha`` when ``merge_ok`` is True.
+ base_sha / head_sha : str
+ The exact SHAs that were merged (what the review evaluates and
+ pins).
+ merge_ok : bool
+ True when the synthetic merge completed; False on conflict.
+ evidence : dict
+ Machine-readable evidence: merge command, exit, output summary,
+ conflicted files (conflict path), result HEAD SHA (clean path).
+ """
+
+ def __init__(self, dir: Path, base_sha: str, head_sha: str,
+ merge_ok: bool, evidence: dict,
+ cleanup: Optional[Callable[[], None]] = None):
+ self.dir = Path(dir)
+ self.base_sha = base_sha
+ self.head_sha = head_sha
+ self.merge_ok = merge_ok
+ self.evidence = evidence
+ self._cleanup = cleanup
+
+ def __enter__(self) -> "MergeWorkspace":
+ return self
+
+ def __exit__(self, exc_type, exc, tb) -> None:
+ """Remove the temporary worktree + directory on every path.
+
+ ``git worktree remove --force`` cleans the clone's worktree
+ admin entry even when the tree is dirty or the merge left
+ conflict state behind; the directory removal is a belt-and-
+ braces cleanup that never masks the caller's result.
+ """
+ if self._cleanup is not None:
+ try:
+ self._cleanup()
+ except Exception: # noqa: BLE001 - cleanup must never mask the caller's result
+ pass
+ shutil.rmtree(self.dir, ignore_errors=True)
+
+ def __repr__(self) -> str: # pragma: no cover - debugging aid
+ return (f"")
+
+
+class ReviewWorkspace:
+ """Builds isolated synthetic-merge workspaces from a configured clone.
+
+ ``clone_dir`` is the READ-ONLY configured repository clone (the
+ clone owns the worktree administration; its checked-out branch and
+ working tree are never modified). ``runner`` is the review gate's
+ subprocess seam (``Runner`` or a scripted fake); all git commands
+ run through it so tests stay hermetic.
+ """
+
+ def __init__(self, clone_dir, runner):
+ self.clone_dir = Path(clone_dir)
+ self.runner = runner
+
+ # -- construction ------------------------------------------------------
+
+ def build(self, repo: str, pr_number: int, head_sha: str,
+ base_sha: str) -> MergeWorkspace:
+ """Construct the synthetic-merge workspace for one review.
+
+ Raises :class:`ReviewWorkspaceError` on operational failures
+ (unfetchable/unresolvable SHAs, missing git identity, worktree
+ creation failure). Returns a :class:`MergeWorkspace` with
+ ``merge_ok=False`` + conflict evidence for a genuine merge
+ conflict, or ``merge_ok=True`` with the merged worktree.
+ """
+ for label, sha in (("base", base_sha), ("head", head_sha)):
+ if not _SHA_RE.match(sha or ""):
+ raise ReviewWorkspaceError(
+ f"cannot build an isolated review workspace: {label} SHA "
+ f"{sha!r} is not a 40-char hex commit id — refusing to "
+ "review an unchecked-out pair")
+
+ self._fetch_refresh(base_sha, head_sha)
+
+ # ``repo`` is embedded in the temp-dir prefix: sanitize it so a
+ # separator/punctuation-containing name cannot turn the prefix into
+ # a nested path (which would make ``mkdtemp`` fail).
+ tmpdir = Path(tempfile.mkdtemp(
+ prefix=f"review-ws-{_fs_safe_token(repo)}-{pr_number}-"))
+ evidence: dict = {
+ "base_sha": base_sha,
+ "head_sha": head_sha,
+ "worktree_dir": str(tmpdir),
+ }
+ tmpdir_q = shlex.quote(str(tmpdir))
+ try:
+ add = self.runner.run(
+ self.clone_dir,
+ f"git worktree add --detach {tmpdir_q} {base_sha}")
+ if add.get("exit") != 0:
+ raise ReviewWorkspaceError(
+ "cannot create the temporary review worktree: "
+ f"{add.get('summary', '')[:400]}")
+
+ merge = self.runner.run(
+ tmpdir,
+ f"git merge --no-ff -m '{_MERGE_MESSAGE}' {head_sha}")
+ evidence["merge_command"] = (
+ f"git merge --no-ff -m '{_MERGE_MESSAGE}' {head_sha}")
+ evidence["merge_exit"] = merge.get("exit")
+ evidence["merge_summary"] = merge.get("summary", "")[:800]
+
+ if merge.get("exit") != 0:
+ # Distinguish a real merge conflict (MERGE_HEAD exists) from an
+ # environmental merge failure (no conflict state was entered).
+ merget = self.runner.run(
+ tmpdir, "git rev-parse -q --verify MERGE_HEAD")
+ if merget.get("exit") != 0:
+ raise ReviewWorkspaceError(
+ "synthetic merge failed without entering a conflict "
+ f"state (environmental failure): {merge.get('summary', '')[:400]}")
+ conflicted = self.runner.run(
+ tmpdir, "git diff --name-only --diff-filter=U")
+ status = self.runner.run(tmpdir, "git status --porcelain")
+ evidence["conflicted_files"] = [
+ line for line in (conflicted.get("summary") or "").splitlines()
+ if line.strip()]
+ evidence["status_summary"] = (status.get("summary") or "")[:800]
+ return MergeWorkspace(
+ tmpdir, base_sha, head_sha, merge_ok=False,
+ evidence=evidence, cleanup=self._make_cleanup(tmpdir))
+
+ result_head = self.runner.run(tmpdir, "git rev-parse HEAD")
+ evidence["result_head"] = (result_head.get("summary") or "").strip()
+ return MergeWorkspace(
+ tmpdir, base_sha, head_sha, merge_ok=True,
+ evidence=evidence, cleanup=self._make_cleanup(tmpdir))
+ except Exception:
+ # Guarantee cleanup even when construction failed mid-way: a
+ # dangling worktree or temp dir is exactly what must never
+ # survive a review pass.
+ try:
+ self._make_cleanup(tmpdir)()
+ except Exception: # noqa: BLE001 - best-effort cleanup on top of a failure
+ pass
+ shutil.rmtree(tmpdir, ignore_errors=True)
+ raise
+
+ # -- internals ----------------------------------------------------------
+
+ def _make_cleanup(self, tmpdir: Path) -> Callable[[], None]:
+ """A cleanup closure: ``git worktree remove --force`` + rmtree."""
+
+ # Shell-quoted exactly like construction: an unquoted path with
+ # spaces/shell metacharacters would be split by the shell, git
+ # would fail to remove the worktree's admin entry, and the clone
+ # would keep a dangling worktree record (violating criterion 7).
+ tmpdir_q = shlex.quote(str(tmpdir))
+
+ def cleanup() -> None:
+ try:
+ self.runner.run(
+ self.clone_dir, f"git worktree remove --force {tmpdir_q}")
+ except Exception: # noqa: BLE001 - cleanup is best-effort
+ pass
+ shutil.rmtree(tmpdir, ignore_errors=True)
+
+ return cleanup
+
+ def _fetch_refresh(self, base_sha: str, head_sha: str) -> None:
+ """Ensure the clone can resolve both SHAs.
+
+ Prefers the precise fetch of exactly the two SHAs; falls back to a
+ full ``git fetch origin`` (which refreshes every remote-tracking
+ ref — the configured clone must be current for reviews anyway) and
+ re-verifies. Hermetic tests use a local-path ``origin``.
+ """
+ res = self.runner.run(
+ self.clone_dir, f"git fetch origin {base_sha} {head_sha}")
+ if res.get("exit") != 0:
+ res = self.runner.run(self.clone_dir, "git fetch origin")
+ if res.get("exit") != 0:
+ raise ReviewWorkspaceError(
+ "cannot fetch the named base/head SHAs from the configured "
+ f"clone's origin: {res.get('summary', '')[:400]}")
+ for label, sha in (("base", base_sha), ("head", head_sha)):
+ chk = self.runner.run(
+ self.clone_dir, f"git rev-parse --verify {sha}^{{commit}}")
+ if chk.get("exit") != 0:
+ raise ReviewWorkspaceError(
+ f"cannot resolve the {label} SHA {sha} as a commit in the "
+ f"configured clone after fetching: {chk.get('summary', '')[:300]}")
diff --git a/gitea_saas/reviewer.py b/gitea_saas/reviewer.py
index 4d5bf3b..be510d0 100644
--- a/gitea_saas/reviewer.py
+++ b/gitea_saas/reviewer.py
@@ -9,9 +9,17 @@ Design
* ``Reviewer(repo_client, runner)`` — ``repo_client`` is a git/Gitea seam
(see ``gitea_client.GiteaClient`` for the real one), ``runner`` is a
subprocess seam (``Runner`` here; a scripted fake in tests).
-* ``review_pr(repo, pr_number, linked_issue, repo_dir)`` returns a
+* ``review_pr(repo, pr_number, linked_issue, workspace)`` returns a
``Verdict``; the caller decides what to do. ``post_verdict`` posts the
- formatted ``[review-gate]`` comment.
+ formatted ``[review-gate]`` comment. ``workspace`` is the isolated
+ review-workspace seam (``review_workspace.ReviewWorkspace``): the
+ review ALWAYS runs against the synthetic merge result of the exact
+ reviewed head onto the exact reviewed base, constructed in a clean
+ temporary Git worktree — never against an arbitrary ``repo_dir``, a
+ shared ``main`` checkout, or a worker's issue worktree (issue #30).
+ A merge conflict yields a deterministic BOUNCE with evidence and the
+ test suite is NOT run against any fallback checkout; with no
+ workspace configured the gate fails closed to NEEDS_GUIDANCE.
* FAIL-CLOSED: the review gate NEVER merges — there is no merge call in
this module. Policy and merge are issue #7.
* Every finding MUST quote the diff. The default (no LLM configured) pass
@@ -166,11 +174,20 @@ class Runner:
@dataclass
class Verdict:
- """Evidence-based outcome of a review gate run."""
+ """Evidence-based outcome of a review gate run.
+
+ ``base_sha`` pins the base commit the synthetic merge was built on
+ (alongside ``head_sha``) — every review records and pins the
+ repository, PR number, head SHA, and current base SHA it actually
+ evaluated (issue #30). ``workspace`` carries the isolated-workspace
+ evidence (``merge_ok``, ``conflicted_files``, …) when the verdict
+ came from a synthetic-merge review.
+ """
repo: str
pr: int
head_sha: str
+ base_sha: str = ""
acceptance: list[str] = field(default_factory=list)
tests: dict = field(default_factory=dict)
build: Optional[dict] = None
@@ -178,11 +195,22 @@ class Verdict:
findings: list[dict] = field(default_factory=list)
verdict: str = NEEDS_GUIDANCE
summary: str = ""
+ workspace: dict = field(default_factory=dict)
def to_comment(self) -> str:
"""Format the [review-gate] comment (checklist + findings)."""
lines = [f"[review-gate] VERDICT: **{self.verdict}**", "",
- f"Repo: `{self.repo}` · PR: #{self.pr} · head `{self.head_sha[:12]}`", ""]
+ f"Repo: `{self.repo}` · PR: #{self.pr} · head `{self.head_sha[:12]}`"
+ + (f" · base `{self.base_sha[:12]}`" if self.base_sha else ""), ""]
+ if self.workspace.get("conflicted_files"):
+ lines += ["Synthetic merge of the reviewed head onto the reviewed base: "
+ "**CONFLICT**",
+ "The test suite was NOT run — no fallback checkout is ever tested.",
+ "Conflicted files:"]
+ lines += [f"- `{f}`" for f in self.workspace["conflicted_files"]]
+ if self.workspace.get("status_summary"):
+ lines += ["```", self.workspace["status_summary"][:400], "```"]
+ lines += [""]
lines.append(f"Acceptance parsed ({len(self.acceptance)}):")
if self.acceptance:
lines += [f"- [ ] {a}" for a in self.acceptance]
@@ -230,11 +258,24 @@ class Reviewer:
# -- main entry points -------------------------------------------------
- def review_pr(self, repo: str, pr_number: int, linked_issue, repo_dir) -> Verdict:
+ def review_pr(self, repo: str, pr_number: int, linked_issue,
+ workspace=None) -> Verdict:
+ """Review one PR against the isolated synthetic-merge workspace.
+
+ ``workspace`` is the issue-#30 seam (``ReviewWorkspace``): its
+ ``build(repo, pr_number, head_sha, base_sha)`` context manager
+ yields the merged temporary worktree. A green verdict ALWAYS
+ describes that exact merge result — never an arbitrary
+ ``repo_dir``. With no workspace (or no resolvable base/head
+ SHAs) the gate fails closed to NEEDS_GUIDANCE; on a merge
+ conflict it returns a deterministic BOUNCE and the test suite is
+ not run.
+ """
issue_body = linked_issue.get("body") if isinstance(linked_issue, dict) else str(linked_issue or "")
pr = self.repo_client.get_pr(repo, pr_number)
head_sha = pr.get("head_sha") or "unknown"
+ base_sha = pr.get("base_sha") or "unknown"
changed_files = pr.get("changed_files") or []
diff_text = self.repo_client.get_pr_diff(repo, pr_number) or ""
@@ -243,28 +284,58 @@ class Reviewer:
scope_violations = [f for f in changed_files
if allowed and not _matches_allowed(f, allowed)]
- tests = self.runner.run(repo_dir, "python -m pytest -q",
- timeout_s=self.config["test_timeout_s"])
- build = None
- if self.config.get("build_cmd"):
- build = self.runner.run(repo_dir, self.config["build_cmd"],
+ if workspace is None:
+ return Verdict(
+ repo=repo, pr=pr_number, head_sha=head_sha, base_sha=base_sha,
+ acceptance=acceptance, tests=dict(exit="not_run"),
+ scope_violations=scope_violations, verdict=NEEDS_GUIDANCE,
+ summary=("no isolated review workspace configured — cannot "
+ "construct and test the synthetic merge; refusing to "
+ "review an arbitrary checkout (never silent-approve)"))
+
+ # The whole review happens inside the workspace: the working tree it
+ # yields IS the synthetic merge of the exact head onto the exact base.
+ with workspace.build(repo, pr_number, head_sha, base_sha) as ws:
+ if not ws.merge_ok:
+ return Verdict(
+ repo=repo, pr=pr_number, head_sha=head_sha, base_sha=base_sha,
+ acceptance=acceptance, tests=dict(
+ exit="not_run",
+ summary="test suite not run: the synthetic merge of the "
+ "reviewed head onto the reviewed base conflicts"),
+ scope_violations=scope_violations, verdict=BOUNCE,
+ summary=("synthetic merge conflict between the reviewed head "
+ "and the current base — see the CONFLICT evidence; "
+ "the test suite was not run against any fallback "
+ "checkout"),
+ workspace={**ws.evidence, "merge_ok": False})
+
+ tests = self.runner.run(ws.dir, "python -m pytest -q",
timeout_s=self.config["test_timeout_s"])
- build["command"] = self.config["build_cmd"]
+ build = None
+ if self.config.get("build_cmd"):
+ build = self.runner.run(ws.dir, self.config["build_cmd"],
+ timeout_s=self.config["test_timeout_s"])
+ build["command"] = self.config["build_cmd"]
- hygiene = self.runner.run(repo_dir, "git diff --check",
- timeout_s=self.config["test_timeout_s"])
+ # Explicit base/result range: the whitespace gate checks only the
+ # synthetic merge result vs the exact base it was built on.
+ hygiene = self.runner.run(
+ ws.dir, f"git diff --check {ws.base_sha} HEAD",
+ timeout_s=self.config["test_timeout_s"])
- findings = self._collect_findings(diff_text, tests, hygiene)
+ findings = self._collect_findings(diff_text, tests, hygiene)
- verdict, summary = self._decide(
- issue_body, acceptance, tests, build, scope_violations, findings, hygiene)
+ verdict, summary = self._decide(
+ issue_body, acceptance, tests, build, scope_violations,
+ findings, hygiene)
- return Verdict(
- repo=repo, pr=pr_number, head_sha=head_sha,
- acceptance=acceptance, tests=tests, build=build,
- scope_violations=scope_violations, findings=findings,
- verdict=verdict, summary=summary,
- )
+ return Verdict(
+ repo=repo, pr=pr_number, head_sha=ws.head_sha, base_sha=ws.base_sha,
+ acceptance=acceptance, tests=tests, build=build,
+ scope_violations=scope_violations, findings=findings,
+ verdict=verdict, summary=summary,
+ workspace={**ws.evidence, "merge_ok": True})
def post_verdict(self, repo: str, pr_number: int, verdict: Verdict) -> str:
"""Post the formatted [review-gate] comment. Returns the comment."""
diff --git a/tests/test_gitea_client.py b/tests/test_gitea_client.py
index c2b0340..f3cc382 100644
--- a/tests/test_gitea_client.py
+++ b/tests/test_gitea_client.py
@@ -209,6 +209,7 @@ def test_get_pr_normalized_shape_with_changed_files(make_client):
assert pr["number"] == 4
assert pr["body"] == "Closes #1\nSome PR"
assert pr["head_sha"] == "sha-head"
+ assert pr["base_sha"] == "sha-base"
assert pr["base_ref"] == "main"
assert pr["labels"] == ["in-review", "p1"]
assert pr["repo"] == "acme"
diff --git a/tests/test_review_policy.py b/tests/test_review_policy.py
index 8714b30..a4d73ee 100644
--- a/tests/test_review_policy.py
+++ b/tests/test_review_policy.py
@@ -108,9 +108,10 @@ class FakeReviewer:
self.verdicts = dict(verdicts)
self.calls = []
self.posted = []
+ self.runner = None # mirror Reviewer.runner (used to build the workspace)
- def review_pr(self, repo, pr, linked_issue, repo_dir=None):
- self.calls.append((repo, pr, linked_issue, repo_dir))
+ def review_pr(self, repo, pr, linked_issue, workspace=None):
+ self.calls.append((repo, pr, linked_issue, workspace))
verdict = self.verdicts[pr]
if callable(verdict):
return verdict(repo, pr)
@@ -121,18 +122,21 @@ class FakeReviewer:
return ""
-def make_verdict(pr_number, verdict, head_sha="sha-abc"):
+def make_verdict(pr_number, verdict, head_sha="sha-abc", base_sha="sha-base"):
return Verdict(repo="gitea-saas-core", pr=pr_number, head_sha=head_sha,
+ base_sha=base_sha,
acceptance=["tests pass"], tests={"exit": 0, "summary": "12 passed"},
build=None, scope_violations=[], findings=[],
verdict=verdict, summary="ok")
-def pr(number, head_sha="sha-abc", labels=("in-review",), body="Closes #1\nSome PR"):
+def pr(number, head_sha="sha-abc", base_sha="sha-base", labels=("in-review",),
+ body="Closes #1\nSome PR"):
# Production-normalized shape (as GiteaClient._normalize_pr returns):
- # merged bool + state, defaults open / not merged.
- return {"number": number, "head_sha": head_sha, "labels": list(labels),
- "body": body, "merged": False, "state": "open"}
+ # merged bool + state + head_sha + base_sha, defaults open / not merged.
+ return {"number": number, "head_sha": head_sha, "base_sha": base_sha,
+ "labels": list(labels), "body": body, "merged": False,
+ "state": "open"}
def tier_issue(tier, number=1):
@@ -179,7 +183,7 @@ def policy_factory(tmp_path):
def test_review_once_per_head_and_skip_same_head(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(10, head_sha="aaa")], gitea_issues=tier_issue("gold"),
- verdicts={10: make_verdict(10, APPROVE)})
+ verdicts={10: make_verdict(10, APPROVE, head_sha="aaa")})
first = policy.poll_and_review("gitea-saas-core")
second = policy.poll_and_review("gitea-saas-core")
@@ -206,6 +210,73 @@ def test_head_move_triggers_rereview(policy_factory):
assert [c[1] for c in reviewer.calls] == [10, 10]
+# ---------------------------------------------------------------------------
+# Evaluated-pair integrity (issue #30): refuse to apply a verdict whose
+# head/base pair moved after evaluation
+# ---------------------------------------------------------------------------
+
+def test_evaluated_head_move_refuses_verdict_and_new_pair_gets_fresh_shot(policy_factory):
+ # The reviewer evaluated head "sha-old"; the live PR has already moved
+ # to "sha-new" when the verdict is to be applied. A verdict for a pair
+ # that no longer exists must never merge/label/comment/close — and once
+ # the PR settles on the evaluated head, the pair is fresh-eligible and
+ # the same verdict flow applies normally.
+ policy, gitea, reviewer, _, _ = policy_factory(
+ gitea_prs=[pr(50, head_sha="sha-new")], gitea_issues=tier_issue("gold", number=1),
+ verdicts={50: make_verdict(50, APPROVE, head_sha="sha-old")})
+
+ results = policy.poll_and_review("gitea-saas-core")
+ r = results[0]
+ assert r["outcome"] == "stale"
+ assert r["error"]["code"] == "PAIR_MOVED"
+ assert r["evaluated"] == {"head_sha": "sha-old", "base_sha": "sha-base"}
+ assert r["live"] == {"head_sha": "sha-new", "base_sha": "sha-base"}
+ assert "not applied" in r["error"]["message"].lower()
+ # zero side effects for the stale pair
+ assert gitea.merged == [] and gitea.comments == []
+ assert gitea.labels_added == [] and gitea.labels_removed == []
+ assert gitea.closed == []
+ assert all(c[3] is None for c in reviewer.calls) # no clone configured -> no workspace
+
+ # The branch settles on the evaluated head: the pair now exists, the
+ # verdict applies (gold tier -> merge), nothing was recorded as complete
+ # for the stale attempt.
+ gitea.prs[0]["head_sha"] = "sha-old"
+ results = policy.poll_and_review("gitea-saas-core")
+ assert results[0]["action"] == "merged"
+ assert gitea.merged == [("gitea-saas-core", 50)]
+
+
+def test_base_move_after_evaluation_refuses_verdict(policy_factory):
+ # Same head, but the BASE moved after the review evaluated the pair:
+ # the synthetic merge (and therefore the verdict) describes a different
+ # base; refusing is mandatory regardless of head being unchanged.
+ policy, gitea, reviewer, _, _ = policy_factory(
+ gitea_prs=[pr(51, head_sha="sha-h", base_sha="sha-base-current")],
+ gitea_issues=tier_issue("gold", number=1),
+ verdicts={51: make_verdict(51, APPROVE, head_sha="sha-h",
+ base_sha="sha-base-evaluated")})
+
+ results = policy.poll_and_review("gitea-saas-core")
+ r = results[0]
+ assert r["outcome"] == "stale" and r["error"]["code"] == "PAIR_MOVED"
+ assert r["live"] == {"head_sha": "sha-h", "base_sha": "sha-base-current"}
+ assert gitea.merged == [] and gitea.comments == [] and gitea.closed == []
+
+
+def test_workspace_passed_through_when_clone_configured(policy_factory):
+ # With REVIEW_CLONE_DIR configured, the policy builds the isolated
+ # workspace seam and hands it to the reviewer on every call.
+ ws_sentinel = object()
+ policy, gitea, reviewer, _, _ = policy_factory(
+ gitea_prs=[pr(52)], gitea_issues=tier_issue("standard"),
+ verdicts={52: make_verdict(52, APPROVE)},
+ review_clone_dir="/nonexistent/clone")
+ policy._workspace = ws_sentinel # the fixture's clone dir does not exist
+ policy.poll_and_review("gitea-saas-core")
+ assert reviewer.calls[0][3] is ws_sentinel
+
+
def test_unlabelled_prs_skipped_unless_review_all(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(10, labels=("in-review",)), pr(11, labels=())],
@@ -275,7 +346,7 @@ def test_gold_bounce_adds_label_no_merge_no_debit(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(43, head_sha="sha-b", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={43: make_verdict(43, BOUNCE)})
+ verdicts={43: make_verdict(43, BOUNCE, head_sha="sha-b")})
results = policy.poll_and_review("gitea-saas-core")
@@ -292,7 +363,7 @@ def test_merge_failure_leaves_pr_open_and_no_close(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(44, head_sha="sha-c", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={44: make_verdict(44, APPROVE)})
+ verdicts={44: make_verdict(44, APPROVE, head_sha="sha-c")})
gitea.merge_results[44] = False
results = policy.poll_and_review("gitea-saas-core")
@@ -307,7 +378,7 @@ def test_debit_failure_is_fail_closed(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(45, head_sha="sha-d", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={45: make_verdict(45, APPROVE)},
+ verdicts={45: make_verdict(45, APPROVE, head_sha="sha-d")},
client_ids={})
results = policy.poll_and_review("gitea-saas-core")
@@ -325,7 +396,7 @@ def test_standard_approve_comment_only_no_merge(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(50, head_sha="sha-e", labels=("in-review",))],
gitea_issues=tier_issue("standard", number=1),
- verdicts={50: make_verdict(50, APPROVE)})
+ verdicts={50: make_verdict(50, APPROVE, head_sha="sha-e")})
results = policy.poll_and_review("gitea-saas-core")
@@ -341,7 +412,7 @@ def test_needs_guidance_adds_label_and_comment(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(51, head_sha="sha-f", labels=("in-review",))],
gitea_issues=tier_issue("standard", number=1),
- verdicts={51: make_verdict(51, NEEDS_GUIDANCE)})
+ verdicts={51: make_verdict(51, NEEDS_GUIDANCE, head_sha="sha-f")})
results = policy.poll_and_review("gitea-saas-core")
@@ -359,7 +430,7 @@ def test_debit_rate_comes_from_tier_config(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(60, head_sha="sha-g", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={60: make_verdict(60, APPROVE)},
+ verdicts={60: make_verdict(60, APPROVE, head_sha="sha-g")},
tiers={**DEFAULT_TIERS, "gold": {"auto_merge": True, "rate_minor": 250}})
policy.poll_and_review("gitea-saas-core")
@@ -374,7 +445,7 @@ def test_free_tier_merges_without_debit_when_auto_merge(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(61, head_sha="sha-h", labels=("in-review",))],
gitea_issues=tier_issue("free", number=1),
- verdicts={61: make_verdict(61, APPROVE)},
+ verdicts={61: make_verdict(61, APPROVE, head_sha="sha-h")},
tiers={**DEFAULT_TIERS, "free": {"auto_merge": True, "rate_minor": 0}})
results = policy.poll_and_review("gitea-saas-core")
@@ -447,7 +518,7 @@ def test_verdict_failure_reported_and_retried(policy_factory):
calls["n"] += 1
if calls["n"] == 1:
raise RuntimeError("verdict generation boom")
- return make_verdict(pr, APPROVE)
+ return make_verdict(pr, APPROVE, head_sha="sha-v")
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(80, head_sha="sha-v", labels=("in-review",))],
@@ -485,7 +556,7 @@ def test_merge_boundary_failure_retried_merged_once(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(81, head_sha="sha-m", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={81: make_verdict(81, APPROVE)})
+ verdicts={81: make_verdict(81, APPROVE, head_sha="sha-m")})
gitea.fail_next["merge_pr"] = RuntimeError("merge boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -507,7 +578,7 @@ def test_close_boundary_failure_no_double_merge_or_close(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(82, head_sha="sha-c", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={82: make_verdict(82, APPROVE)})
+ verdicts={82: make_verdict(82, APPROVE, head_sha="sha-c")})
gitea.fail_next["close_issue"] = RuntimeError("close boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -527,7 +598,7 @@ def test_debit_boundary_failure_never_double_debits(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(83, head_sha="sha-d", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={83: make_verdict(83, APPROVE)})
+ verdicts={83: make_verdict(83, APPROVE, head_sha="sha-d")})
real_debit = core.debit
calls = {"n": 0}
@@ -555,7 +626,7 @@ def test_label_boundary_failure_bounce_applied_once(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(84, head_sha="sha-l", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={84: make_verdict(84, BOUNCE)})
+ verdicts={84: make_verdict(84, BOUNCE, head_sha="sha-l")})
gitea.fail_next["add_label"] = RuntimeError("label boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -574,7 +645,7 @@ def test_comment_boundary_failure_retried(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(85, head_sha="sha-co", labels=("in-review",))],
gitea_issues=tier_issue("standard", number=1),
- verdicts={85: make_verdict(85, APPROVE)})
+ verdicts={85: make_verdict(85, APPROVE, head_sha="sha-co")})
gitea.fail_next["post_comment"] = RuntimeError("comment boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -591,7 +662,7 @@ def test_remove_label_boundary_failure_retried(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(86, head_sha="sha-rl", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={86: make_verdict(86, APPROVE)})
+ verdicts={86: make_verdict(86, APPROVE, head_sha="sha-rl")})
gitea.fail_next["remove_label"] = RuntimeError("label boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -611,7 +682,7 @@ def test_merge_refused_is_retryable_not_completed(policy_factory):
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(87, head_sha="sha-mf", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={87: make_verdict(87, APPROVE)})
+ verdicts={87: make_verdict(87, APPROVE, head_sha="sha-mf")})
gitea.merge_results[87] = False
first = policy.poll_and_review("gitea-saas-core")
@@ -640,7 +711,7 @@ def test_webhook_after_completed_poll_does_not_double_close(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(88, head_sha="sha-wh", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={88: make_verdict(88, APPROVE)})
+ verdicts={88: make_verdict(88, APPROVE, head_sha="sha-wh")})
policy.poll_and_review("gitea-saas-core")
assert gitea.closed == [("gitea-saas-core", 1)]
@@ -670,10 +741,17 @@ def test_legacy_review_row_still_skips_head(policy_factory):
def test_failed_attempt_then_head_move_reviews_new_head(policy_factory):
+ # The verdict follows the LIVE head (as the real reviewer's does), so a
+ # failed gold attempt retries at the same head, and after the head moves
+ # the NEW pair is reviewed and merged fresh.
+ def live_head_verdict(repo, pr):
+ return make_verdict(pr, APPROVE,
+ head_sha=gitea.prs[0]["head_sha"])
+
policy, gitea, reviewer, _, _ = policy_factory(
gitea_prs=[pr(90, head_sha="sha-a", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={90: make_verdict(90, APPROVE)})
+ verdicts={90: live_head_verdict})
gitea.fail_next["merge_pr"] = RuntimeError("merge boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -703,7 +781,7 @@ def test_merged_pr_gone_from_open_scan_recovered_after_post_merge_failure(policy
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(91, head_sha="sha-91", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={91: make_verdict(91, APPROVE)})
+ verdicts={91: make_verdict(91, APPROVE, head_sha="sha-91")})
gitea.fail_next["remove_label"] = RuntimeError("label boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -736,7 +814,7 @@ def test_crash_after_label_removed_no_duplicate_visible_actions(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(92, head_sha="sha-92", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={92: make_verdict(92, APPROVE)})
+ verdicts={92: make_verdict(92, APPROVE, head_sha="sha-92")})
gitea.fail_next["post_comment"] = RuntimeError("comment boom")
first = policy.poll_and_review("gitea-saas-core")
@@ -765,7 +843,7 @@ def test_crash_after_success_comment_before_completion_record(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(93, head_sha="sha-93", labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={93: make_verdict(93, APPROVE)})
+ verdicts={93: make_verdict(93, APPROVE, head_sha="sha-93")})
real_record = policy._record_completed
@@ -805,14 +883,19 @@ def test_crash_after_success_comment_before_completion_record(policy_factory):
# ---------------------------------------------------------------------------
def test_reviewed_head_pinned_so_moved_branch_is_never_merged(policy_factory):
- # Integration regression: review scans SHA A, the branch moves to SHA B
- # before the merge attempt. The policy must hand the REVIEWED head to
- # merge_pr, and the production-shaped client seam fails closed: no
- # merge POST, no debit, no close, no success comment. The moved head is
- # then a NEW head, re-reviewed and merged normally on the next poll.
+ # Integration regression: review evaluates SHA A, the branch moves to
+ # SHA B before the verdict is applied. The evaluated-pair gate (issue
+ # #30) refuses BEFORE any policy action — no merge POST, no debit, no
+ # close, no success comment. Once the branch sits at the SHAs the
+ # verdict describes, the pair is fresh-eligible and merges normally.
+ state = {"moved": False}
+
def review_then_move_head(repo, pr):
- gitea.prs[0]["head_sha"] = "sha-B" # branch moved after review
- return make_verdict(pr, APPROVE)
+ evaluated = gitea.prs[0]["head_sha"] # sha-A at review time
+ if not state["moved"]:
+ state["moved"] = True
+ gitea.prs[0]["head_sha"] = "sha-B" # branch moved after review
+ return make_verdict(pr, APPROVE, head_sha=evaluated)
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(94, head_sha="sha-A", labels=("in-review",))],
@@ -820,13 +903,13 @@ def test_reviewed_head_pinned_so_moved_branch_is_never_merged(policy_factory):
verdicts={94: review_then_move_head})
first = policy.poll_and_review("gitea-saas-core")
- assert first[0]["outcome"] == "failed"
- assert "refusing to merge" in first[0]["error"]["message"]
+ assert first[0]["outcome"] == "stale"
+ assert first[0]["error"]["code"] == "PAIR_MOVED"
assert "sha-A" in first[0]["error"]["message"] and "sha-B" in first[0]["error"]["message"]
assert gitea.merged == [] # NO merge POST happened
assert gitea.closed == []
assert not any("Merged by review-gate" in b for _, _, b in gitea.comments)
- assert _ledger_debits(core, c["id"]) == [] # no billing for the refused head
+ assert _ledger_debits(core, c["id"]) == [] # no billing for the moved pair
second = policy.poll_and_review("gitea-saas-core")
assert second and second[0]["action"] == "merged" # sha-B reviewed + merged
@@ -937,7 +1020,7 @@ def test_missing_reviewed_head_never_reaches_merge_client(policy_factory):
policy, gitea, reviewer, core, c = policy_factory(
gitea_prs=[pr(98, labels=("in-review",))],
gitea_issues=tier_issue("gold", number=1),
- verdicts={98: make_verdict(98, APPROVE)})
+ verdicts={98: make_verdict(98, APPROVE, head_sha=None)})
gitea.prs[0]["head_sha"] = None # no reviewed head on the candidate
results = policy.poll_and_review("gitea-saas-core")
diff --git a/tests/test_review_workspace.py b/tests/test_review_workspace.py
new file mode 100644
index 0000000..f32d829
--- /dev/null
+++ b/tests/test_review_workspace.py
@@ -0,0 +1,328 @@
+"""Hermetic tests for the isolated review workspace (issue #30).
+
+Real ``git`` binaries against LOCAL temporary repositories only — every
+"remote" is a local-path ``origin`` (a bare repo under ``tmp_path``), so
+the suite proves the exact production flow (fetch named SHAs, detached
+worktree, synthetic merge, cleanup) with zero network calls.
+
+Run with: python -m pytest tests/test_review_workspace.py -q
+"""
+
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+from types import SimpleNamespace
+
+import pytest
+
+# PEP 420 namespace package: make the project root importable regardless of
+# how pytest was invoked.
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from gitea_saas.review_workspace import ( # noqa: E402
+ ReviewWorkspace,
+ ReviewWorkspaceError,
+)
+from gitea_saas.reviewer import Runner # noqa: E402
+
+GIT = ["git"]
+
+
+def git(repo, *args):
+ """Run git in ``repo``; return the CompletedProcess (checked)."""
+ repo = Path(repo)
+ if not repo.exists() and args and args[0] == "init":
+ repo.mkdir(parents=True, exist_ok=True)
+ return subprocess.run(
+ [*GIT, "-C", str(repo), *args], check=True,
+ capture_output=True, text=True)
+
+
+def git_ok(repo, *args):
+ return git(repo, *args).returncode == 0
+
+
+def commit_file(repo, filename, content, message):
+ path = repo / filename
+ path.write_text(content)
+ git(repo, "add", filename)
+ git(repo, "commit", "-m", message)
+ return git(repo, "rev-parse", "HEAD").stdout.strip()
+
+
+@pytest.fixture
+def fixtures(tmp_path):
+ """Build a local origin (bare), a seeding checkout, and the configured
+ clone. Both the `clean` and the `conflict` head/base pairs come from
+ the same origin (all SHAs are advertised branch tips, so the exact-SHA
+ fetch works over the local-path transport exactly like production)."""
+ origin = tmp_path / "origin.git"
+ git(origin, "init", "--bare", "--initial-branch=main")
+
+ seed = tmp_path / "seed"
+ git(seed, "init", "--initial-branch=main")
+ git(seed, "config", "user.email", "seed@example.com")
+ git(seed, "config", "user.name", "Seed")
+ git(seed, "remote", "add", "origin", str(origin))
+
+ base0 = commit_file(seed, "alpha.txt", "line one\n", "base file")
+ git(seed, "push", "-u", "origin", "main")
+
+ # ---- clean pair: feature adds beta.txt, main adds gamma.txt ----
+ git(seed, "checkout", "-b", "feature", base0)
+ head_clean = commit_file(seed, "beta.txt", "feature add\n", "feature work")
+ git(seed, "push", "-u", "origin", "feature")
+ git(seed, "checkout", "main")
+ base_clean = commit_file(seed, "gamma.txt", "main add\n", "main work")
+ git(seed, "push", "origin", "main")
+
+ # ---- conflict pair: both sides edit alpha.txt after branching ----
+ git(seed, "checkout", "-b", "conflict-head", base0)
+ head_conflict = commit_file(seed, "alpha.txt", "feature change\n", "conflict head")
+ git(seed, "checkout", "main")
+ base_conflict = commit_file(seed, "alpha.txt", "main change\n", "conflict base")
+ git(seed, "push", "origin", "main")
+ git(seed, "push", "-u", "origin", "conflict-head")
+
+ clone = tmp_path / "clone"
+ subprocess.run(["git", "clone", str(origin), str(clone)],
+ check=True, capture_output=True, text=True)
+ git(clone, "config", "user.email", "clone@example.com")
+ git(clone, "config", "user.name", "Clone")
+ clone_head_before = git(clone, "rev-parse", "HEAD").stdout.strip()
+
+ return SimpleNamespace(
+ origin=origin, seed=seed, clone=clone,
+ head_clean=head_clean, base_clean=base_clean,
+ head_conflict=head_conflict, base_conflict=base_conflict,
+ clone_head_before=clone_head_before,
+ )
+
+
+def review_dirs(pr_number):
+ root = Path(tempfile.gettempdir())
+ return sorted(root.glob(f"review-ws-acme-{pr_number}-*"))
+
+
+def clone_worktree_lines(clone):
+ return git(clone, "worktree", "list").stdout.strip()
+
+
+def assert_clone_untouched(fx):
+ assert git(fx.clone, "rev-parse", "HEAD").stdout.strip() == fx.clone_head_before
+ assert git(fx.clone, "status", "--porcelain").stdout.strip() == ""
+
+
+# ---------------------------------------------------------------------------
+# Clean merge
+# ---------------------------------------------------------------------------
+
+def test_clean_merge_constructs_synthetic_merge(fixtures):
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ with ws.build("acme", 1, fixtures.head_clean, fixtures.base_clean) as mws:
+ assert mws.merge_ok is True
+ # The working tree IS the merge result: both sides' files present,
+ # both parent commits in history.
+ assert (mws.dir / "beta.txt").read_text() == "feature add\n"
+ assert (mws.dir / "gamma.txt").read_text() == "main add\n"
+ assert (mws.dir / "alpha.txt").read_text() == "line one\n"
+ parents = git(mws.dir, "rev-list", "--parents", "-n", "1", "HEAD").stdout.split()
+ assert len(parents) == 3 # merge commit + base + head
+ assert mws.evidence["base_sha"] == fixtures.base_clean
+ assert mws.evidence["head_sha"] == fixtures.head_clean
+ assert mws.evidence["result_head"] == parents[0]
+ # Explicit base/result range works from inside the workspace.
+ res = Runner().run(mws.dir, f"git diff --check {fixtures.base_clean} HEAD")
+ assert res["exit"] == 0
+ # Cleaned up after success; the configured clone is untouched.
+ assert review_dirs(1) == []
+ assert "review-ws" not in clone_worktree_lines(fixtures.clone)
+ assert_clone_untouched(fixtures)
+
+
+# ---------------------------------------------------------------------------
+# Merge conflict
+# ---------------------------------------------------------------------------
+
+def test_merge_conflict_is_deterministic_evidence(fixtures):
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ with ws.build("acme", 2, fixtures.head_conflict, fixtures.base_conflict) as mws:
+ assert mws.merge_ok is False
+ assert mws.evidence["merge_exit"] != 0
+ assert mws.evidence["conflicted_files"] == ["alpha.txt"]
+ assert "alpha.txt" in mws.evidence["status_summary"]
+ assert review_dirs(2) == []
+ assert "review-ws" not in clone_worktree_lines(fixtures.clone)
+ assert_clone_untouched(fixtures)
+
+
+# ---------------------------------------------------------------------------
+# No shared-checkout mutation (source clone AND a worker-style worktree)
+# ---------------------------------------------------------------------------
+
+def test_neither_clone_nor_worker_worktree_is_mutated(fixtures):
+ # A worker's issue worktree taken from the same clone must be untouched
+ # by the review pass (its branch, content, and dirty state all survive).
+ worker = fixtures.clone.parent / "worker-issue-42"
+ git(fixtures.clone, "worktree", "add", "--detach", str(worker), fixtures.base_clean)
+ (worker / "alpha.txt").write_text("worker dirty line\n") # simulate WIP
+ worker_head_before = git(worker, "rev-parse", "HEAD").stdout.strip()
+
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ with ws.build("acme", 3, fixtures.head_clean, fixtures.base_clean) as mws:
+ assert mws.merge_ok is True
+ # the worker checkout's content is untouched by the merge
+ assert (worker / "alpha.txt").read_text() == "worker dirty line\n"
+ assert git(worker, "rev-parse", "HEAD").stdout.strip() == worker_head_before
+ # (strip(): the porcelain status column's leading space is whitespace)
+ assert git(worker, "status", "--porcelain").stdout.strip() == "M alpha.txt"
+
+ assert "review-ws" not in clone_worktree_lines(fixtures.clone)
+ # the worker worktree itself still exists with its dirty state
+ assert worker.exists()
+ assert git(worker, "status", "--porcelain").stdout.strip() == "M alpha.txt"
+ assert_clone_untouched(fixtures)
+ git(fixtures.clone, "worktree", "remove", "--force", str(worker))
+
+
+# ---------------------------------------------------------------------------
+# Cleanup on failure paths
+# ---------------------------------------------------------------------------
+
+def test_cleanup_after_raised_exception(fixtures):
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ before = review_dirs(4)
+ with pytest.raises(RuntimeError, match="boom"):
+ with ws.build("acme", 4, fixtures.head_clean, fixtures.base_clean) as mws:
+ assert mws.merge_ok is True
+ raise RuntimeError("boom")
+ assert review_dirs(4) == before
+ assert "review-ws" not in clone_worktree_lines(fixtures.clone)
+ assert_clone_untouched(fixtures)
+
+
+def test_cleanup_after_merge_conflict_exit(fixtures):
+ # Simulates the context manager exiting with a conflict result — the
+ # workspace must still be fully removed.
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ before = review_dirs(5)
+ with ws.build("acme", 5, fixtures.head_conflict, fixtures.base_conflict) as mws:
+ assert mws.merge_ok is False
+ assert review_dirs(5) == before
+ assert "review-ws" not in clone_worktree_lines(fixtures.clone)
+ assert_clone_untouched(fixtures)
+
+
+def test_unfetchable_sha_raises_and_leaves_nothing(fixtures):
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ before = review_dirs(6)
+ with pytest.raises(ReviewWorkspaceError):
+ ws.build("acme", 6, "f" * 40, fixtures.base_clean)
+ assert review_dirs(6) == before
+ assert_clone_untouched(fixtures)
+
+
+def test_non_hex_sha_fails_closed_without_git(fixtures):
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ with pytest.raises(ReviewWorkspaceError, match="not a 40-char hex"):
+ ws.build("acme", 7, "HEAD", fixtures.base_clean) # a ref, not a SHA
+ with pytest.raises(ReviewWorkspaceError, match="not a 40-char hex"):
+ ws.build("acme", 7, fixtures.head_clean, "main")
+ assert_clone_untouched(fixtures)
+
+
+# ---------------------------------------------------------------------------
+# Commands run in the right place (cwd + explicit range)
+# ---------------------------------------------------------------------------
+
+def test_git_commands_run_in_workspace_cwd(fixtures):
+ calls = []
+
+ def spy(command, **kwargs):
+ calls.append((kwargs.get("cwd"), command))
+ return subprocess.run(command, shell=True, **kwargs)
+
+ runner = Runner(timeout_s=120, command_runner=spy)
+ ws = ReviewWorkspace(fixtures.clone, runner)
+ with ws.build("acme", 8, fixtures.head_clean, fixtures.base_clean) as mws:
+ assert mws.merge_ok is True
+ merges = [c for c in calls if "git merge --no-ff" in c[1]]
+ assert merges, "synthetic merge command must run"
+ assert Path(merges[0][0]) == mws.dir, "merge must run INSIDE the workspace"
+ assert "review-ws" in str(merges[0][0])
+ # fetch + worktree add run against the configured clone (read-only use)
+ adds = [c for c in calls if "worktree add --detach" in c[1]]
+ assert adds and Path(adds[0][0]) == fixtures.clone
+ fetches = [c for c in calls if "git fetch origin" in c[1]]
+ assert fetches and Path(fetches[0][0]) == fixtures.clone
+ # cleanup command targets the clone
+ removes = [c for c in calls if "worktree remove --force" in c[1]]
+ assert removes and Path(removes[0][0]) == fixtures.clone
+
+
+def test_explicit_range_diff_check_from_workspace(fixtures):
+ calls = []
+
+ def spy(command, **kwargs):
+ calls.append((kwargs.get("cwd"), command))
+ return subprocess.run(command, shell=True, **kwargs)
+
+ runner = Runner(timeout_s=120, command_runner=spy)
+ ws = ReviewWorkspace(fixtures.clone, runner)
+ with ws.build("acme", 9, fixtures.head_clean, fixtures.base_clean) as mws:
+ assert mws.merge_ok is True
+ # The reviewer's hygiene gate: diff --check over the EXPLICIT
+ # base..result range, run from INSIDE the isolated workspace.
+ res = runner.run(mws.dir, f"git diff --check {fixtures.base_clean} HEAD")
+ assert res["exit"] == 0
+ checks = [c for c in calls if "diff --check" in c[1]]
+ assert checks, "explicit-range diff --check must run"
+ assert fixtures.base_clean in checks[0][1] # explicit BASE
+ assert " HEAD" in checks[0][1] # explicit result end
+ assert "review-ws" in str(checks[0][0]) # inside the isolated workspace
+
+
+# ---------------------------------------------------------------------------
+# Bounce-fix: shell-safe path quoting + filesystem-safe temp prefix
+# ---------------------------------------------------------------------------
+
+def test_weird_temp_root_with_spaces_and_shell_char_success_and_cleanup(
+ fixtures, tmp_path, monkeypatch):
+ # A valid OS temp root whose path contains a space AND a shell-sensitive
+ # character ($). Unquoted interpolation into the Runner's shell=True
+ # commands would split the path (construction fails) or — worse —
+ # let `git worktree remove` fail while rmtree deletes the directory,
+ # leaving a dangling worktree admin entry in the clone.
+ weird_root = tmp_path / "review root $weird"
+ weird_root.mkdir(parents=True)
+ monkeypatch.setattr(tempfile, "tempdir", str(weird_root))
+
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ with ws.build("acme", 10, fixtures.head_clean, fixtures.base_clean) as mws:
+ assert mws.merge_ok is True
+ assert mws.dir.exists()
+ assert " " in str(mws.dir) # really lived under the weird root
+ assert "$" in str(mws.dir)
+ assert mws.dir.parent == weird_root
+ # Complete cleanup: directory gone AND no worktree admin record remains.
+ assert not mws.dir.exists()
+ assert "review-ws" not in git(
+ fixtures.clone, "worktree", "list", "--porcelain").stdout
+ assert_clone_untouched(fixtures)
+
+
+def test_repo_token_with_separators_is_filesystem_safe(fixtures):
+ # ``repo`` is embedded in the mkdtemp prefix; an owner/name-style token
+ # must be collapsed to a single filename component (a NESTED prefix
+ # would make tempfile.mkdtemp raise FileNotFoundError).
+ repo = "pbwraith/gitea-saas-core"
+ ws = ReviewWorkspace(fixtures.clone, Runner())
+ with ws.build(repo, 11, fixtures.head_clean, fixtures.base_clean) as mws:
+ assert mws.merge_ok is True
+ assert mws.dir.name.startswith("review-ws-pbwraith-gitea-saas-core-11-")
+ assert "/" not in mws.dir.name
+ assert not mws.dir.exists()
+ assert "review-ws" not in git(
+ fixtures.clone, "worktree", "list", "--porcelain").stdout
+ assert_clone_untouched(fixtures)
diff --git a/tests/test_reviewer.py b/tests/test_reviewer.py
index e8e3df6..1a13f5e 100644
--- a/tests/test_reviewer.py
+++ b/tests/test_reviewer.py
@@ -11,6 +11,7 @@ import ast
import subprocess
import sys
from pathlib import Path
+from types import SimpleNamespace
import pytest
@@ -36,13 +37,14 @@ class FakeGiteaClient:
"""In-memory Gitea seam: PRs, issues and posted comments."""
def __init__(self, prs=None, issues=None):
- self.prs = prs or {} # number -> {"head_sha", "changed_files", "diff"}
+ self.prs = prs or {} # number -> {"head_sha", "base_sha", "changed_files", "diff"}
self.issues = issues or {} # number -> {"body", ...}
self.comments = [] # (repo, number, body)
def get_pr(self, repo, number):
pr = self.prs[number]
- return {"head_sha": pr["head_sha"], "changed_files": list(pr["changed_files"])}
+ return {"head_sha": pr["head_sha"], "base_sha": pr.get("base_sha") or SAMPLE_BASE,
+ "changed_files": list(pr["changed_files"])}
def get_pr_diff(self, repo, number):
return self.prs[number].get("diff", "")
@@ -54,6 +56,37 @@ class FakeGiteaClient:
self.comments.append((repo, number, body))
+SAMPLE_BASE = "b" * 40
+
+
+class FakeWorkspace:
+ """Duck-typed review-workspace seam: no git, scripted merge result."""
+
+ def __init__(self, merge_ok=True, evidence=None, directory="/fake/ws",
+ base_sha=SAMPLE_BASE):
+ self.merge_ok = merge_ok
+ self.evidence = dict(evidence or {})
+ self.directory = str(directory)
+ self.base_sha = base_sha
+
+ def build(self, repo, pr_number, head_sha, base_sha):
+ return _FakeMergeContext(self)
+
+
+class _FakeMergeContext:
+ def __init__(self, ws):
+ self._ws = ws
+
+ def __enter__(self):
+ return SimpleNamespace(
+ dir=Path(self._ws.directory), base_sha=self._ws.base_sha,
+ head_sha="a" * 40, merge_ok=self._ws.merge_ok,
+ evidence=self._ws.evidence)
+
+ def __exit__(self, *exc):
+ return False
+
+
class FakeRunner:
"""Subprocess seam: scripted results, records commands."""
@@ -114,16 +147,19 @@ def test_approve_when_tests_pass_and_no_findings():
{"exit": 0, "summary": "clean"}]) # git diff --check
reviewer = Reviewer(client, runner, config={"secret_scan": True})
- verdict = reviewer.review_pr("gitea-saas-core", 1, client.issues[5], "/fake/repo")
+ verdict = reviewer.review_pr("gitea-saas-core", 1, client.issues[5],
+ workspace=FakeWorkspace())
assert verdict.verdict == APPROVE
assert verdict.tests == {"exit": 0, "summary": "12 passed"}
assert verdict.acceptance == ["tests pass", "no regressions"]
assert verdict.scope_violations == [] and verdict.findings == []
assert verdict.head_sha == "a" * 40
+ assert verdict.base_sha == "b" * 40
comment = reviewer.post_verdict("gitea-saas-core", 1, verdict)
assert "[review-gate] VERDICT: **APPROVE**" in comment
+ assert "head `aaaaaaaaaaaa`" in comment and "base `bbbbbbbbbbbb`" in comment
assert "- [ ] tests pass" in comment # checklist present
assert "No merge performed by the review gate" in comment
assert client.comments and client.comments[0][:2] == ("gitea-saas-core", 1)
@@ -132,7 +168,8 @@ def test_approve_when_tests_pass_and_no_findings():
def test_bounce_when_tests_fail():
client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: sample_issue()})
runner = FakeRunner([{"exit": 1, "summary": "1 failed, 11 passed"}, {"exit": 0, "summary": "clean"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, client.issues[5], "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, client.issues[5],
+ workspace=FakeWorkspace())
assert verdict.verdict == BOUNCE
assert "tests failed (exit=1)" in verdict.summary
# BOUNCE comment carries the test evidence
@@ -146,7 +183,8 @@ def test_bounce_on_scope_violation_even_with_green_tests():
pr = sample_pr(files=["gitea_saas/reviewer.py", "gitea_saas/server.py"])
client = FakeGiteaClient(prs={1: pr}, issues={5: issue})
runner = FakeRunner([{"exit": 0, "summary": "12 passed"}, {"exit": 0, "summary": "clean"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
assert verdict.verdict == BOUNCE
assert verdict.scope_violations == ["gitea_saas/server.py"]
@@ -155,7 +193,8 @@ def test_needs_guidance_when_no_acceptance_criteria():
issue = {"body": "## Goal\nnothing to verify here\n"}
client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
runner = FakeRunner([{"exit": 0, "summary": "12 passed"}, {"exit": 0, "summary": "clean"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
assert verdict.verdict == NEEDS_GUIDANCE
assert "never silent-approve" in verdict.summary
@@ -164,7 +203,8 @@ def test_allow_test_failure_in_issue_body_overrides_bounce():
issue = sample_issue(extra="note: allow_test_failure for this transitional PR\n")
client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
runner = FakeRunner([{"exit": 1, "summary": "known failure"}, {"exit": 0, "summary": "clean"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
assert verdict.verdict == APPROVE # tolerated, acceptance present, nothing else blocking
@@ -188,7 +228,8 @@ def test_secret_finding_quotes_and_redacts_diff_line():
pr = sample_pr(diff=SECRET_DIFF, files=["gitea_saas/server.py"])
client = FakeGiteaClient(prs={1: pr}, issues={5: issue})
runner = FakeRunner([{"exit": 0, "summary": "ok"}, {"exit": 0, "summary": "clean"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
blocking = [f for f in verdict.findings if f["severity"] == "blocking"]
assert blocking, "secret scan should produce a blocking finding"
f = blocking[0]
@@ -208,7 +249,7 @@ def test_todo_finding_is_warning_not_blocking():
client = FakeGiteaClient(prs={1: pr}, issues={5: issue})
runner = FakeRunner([{"exit": 0, "summary": "ok"}, {"exit": 0, "summary": "clean"}])
verdict = Reviewer(client, runner, config={"secret_scan": False}).review_pr(
- "gitea-saas-core", 1, issue, "/fake/repo")
+ "gitea-saas-core", 1, issue, workspace=FakeWorkspace())
todos = [f for f in verdict.findings if f["severity"] == "warning"]
assert todos and todos[0]["note"] == "TODO/FIXME/HACK marker in added line"
assert todos[0]["evidence_quote"] == " # TODO: replace placeholder"
@@ -224,7 +265,8 @@ def test_timeout_bounces_with_timed_out_exit():
client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
runner = FakeRunner([{"exit": "timed_out", "summary": "[timed out after 300s]"},
{"exit": 0, "summary": "clean"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
assert verdict.verdict == BOUNCE
assert verdict.tests["exit"] == "timed_out"
assert "timed out" in verdict.summary
@@ -270,7 +312,8 @@ def test_build_command_run_when_configured_and_build_failure_bounces():
{"exit": 1, "summary": "build error"}, # build
{"exit": 0, "summary": "clean"}]) # diff --check
reviewer = Reviewer(client, runner, config={"build_cmd": "python -m compileall -q ."})
- verdict = reviewer.review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = reviewer.review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
build = verdict.build
assert build is not None
assert build["exit"] == 1 and build["command"] == "python -m compileall -q ."
@@ -283,16 +326,70 @@ def test_diff_hygiene_failure_is_blocking_finding():
client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
runner = FakeRunner([{"exit": 0, "summary": "12 passed"},
{"exit": 1, "summary": "trailing whitespace at gitea_saas/x.py:3"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
assert verdict.verdict == BOUNCE
assert any("diff --check" in f["note"] for f in verdict.findings)
+def test_hygiene_diff_check_uses_explicit_base_head_range():
+ issue = sample_issue()
+ client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
+ runner = FakeRunner([{"exit": 0, "summary": "12 passed"},
+ {"exit": 0, "summary": "clean"}])
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
+ assert verdict.verdict == APPROVE
+ # The whitespace gate runs against the synthetic merge result with an
+ # explicit base..HEAD range (issue #30 / acceptance 5), from the
+ # workspace directory — never a bare `git diff --check`.
+ assert len(runner.commands) == 2 # pytest + explicit-range diff --check
+ ws_dir, hygiene_cmd, _ = runner.commands[1]
+ assert ws_dir == "/fake/ws"
+ assert hygiene_cmd == "git diff --check bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb HEAD"
+ assert "git diff --check" not in hygiene_cmd.replace(
+ "git diff --check bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb HEAD", "")
+
+
+def test_merge_conflict_bounces_without_running_tests():
+ issue = sample_issue()
+ client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
+ runner = FakeRunner([{"exit": 0, "summary": "12 passed"}]) # would be tests
+ workspace = FakeWorkspace(merge_ok=False, evidence={
+ "conflicted_files": ["gitea_saas/reviewer.py"],
+ "status_summary": "UU gitea_saas/reviewer.py",
+ "merge_exit": 1})
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=workspace)
+ assert verdict.verdict == BOUNCE
+ assert "synthetic merge conflict" in verdict.summary
+ assert verdict.tests["exit"] == "not_run"
+ assert runner.commands == [], "the test suite must NOT run on a conflicted merge"
+ assert verdict.workspace["merge_ok"] is False
+ assert verdict.workspace["conflicted_files"] == ["gitea_saas/reviewer.py"]
+ comment = verdict.to_comment()
+ assert "**CONFLICT**" in comment
+ assert "The test suite was NOT run" in comment
+ assert "`gitea_saas/reviewer.py`" in comment
+
+
+def test_no_workspace_fails_closed_to_needs_guidance():
+ issue = sample_issue()
+ client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
+ runner = FakeRunner([{"exit": 0, "summary": "12 passed"}])
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue)
+ assert verdict.verdict == NEEDS_GUIDANCE
+ assert "no isolated review workspace configured" in verdict.summary
+ assert verdict.tests["exit"] == "not_run"
+ assert runner.commands == [], "no commands may run without a workspace"
+
+
def test_verdict_shape_and_comment_never_mentions_merge_action():
issue = sample_issue()
client = FakeGiteaClient(prs={1: sample_pr()}, issues={5: issue})
runner = FakeRunner([{"exit": 0, "summary": "12 passed"}, {"exit": 0, "summary": "clean"}])
- verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue, "/fake/repo")
+ verdict = Reviewer(client, runner).review_pr("gitea-saas-core", 1, issue,
+ workspace=FakeWorkspace())
assert verdict.repo == "gitea-saas-core" and verdict.pr == 1
assert isinstance(verdict.acceptance, list)
assert isinstance(verdict.tests, dict) and "exit" in verdict.tests