mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(pr-review): internal-PR safety reviewer — review off-task-flow org PRs
Extend the inbound-PR reviewer beyond external/fork PRs to internal org-repo PRs that bypassed the agent task-flow (a human-pushed branch). The org's own in-flight integration PRs are skipped — a live task owns their branch and they already pass QA + PM review — so the reviewer only flags off-process PRs. - config: internal_pr_enabled (default OFF, like external_pr_enabled) - PR_REVIEW_SOURCES = (external_pr, internal_pr); generalize dispatch, dedup, the decision queue, the git-gate exemption, and supersede to both sources - TaskService.active_task_owns_branch (skip lifecycle PRs) + ingest source param with source-aware wording - poll loop runs when EITHER flag is on; _ingest_pr_if_reviewable picks the source per PR (external: flag+author-allow; internal: flag+not-task-owned) - 11 unit tests (decision logic + branch-ownership)
This commit is contained in:
@@ -341,6 +341,16 @@ class Settings(BaseSettings):
|
|||||||
"checks out, or executes external contributor code."
|
"checks out, or executes external contributor code."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
internal_pr_enabled: bool = Field(
|
||||||
|
default=False,
|
||||||
|
description=(
|
||||||
|
"Master switch for the internal-PR safety reviewer. OFF by default. "
|
||||||
|
"When on, the same poll also reviews org-repo (non-fork) PRs that are "
|
||||||
|
"NOT tied to an active task — i.e. branches pushed outside the agent "
|
||||||
|
"task-flow. The org's own in-flight integration PRs (whose branch a "
|
||||||
|
"live task owns) are skipped, since they already pass QA + PM review."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
# ==========================================================================
|
# ==========================================================================
|
||||||
# Workspaces (Multi-Agent Git)
|
# Workspaces (Multi-Agent Git)
|
||||||
|
|||||||
@@ -58,6 +58,7 @@ from roboco.models.runtime import (
|
|||||||
WaitingRecord,
|
WaitingRecord,
|
||||||
)
|
)
|
||||||
from roboco.seeds.initial_data import AGENT_UUIDS
|
from roboco.seeds.initial_data import AGENT_UUIDS
|
||||||
|
from roboco.services.task import PR_REVIEW_SOURCES
|
||||||
|
|
||||||
logger = structlog.get_logger()
|
logger = structlog.get_logger()
|
||||||
|
|
||||||
@@ -4626,15 +4627,17 @@ Start by:
|
|||||||
logger.exception("strategy engine cycle failed")
|
logger.exception("strategy engine cycle failed")
|
||||||
|
|
||||||
async def _external_pr_poll_loop(self) -> None:
|
async def _external_pr_poll_loop(self) -> None:
|
||||||
"""Engine 3: discover inbound external PRs and open review tasks.
|
"""Engine 3: discover inbound PRs and open review tasks.
|
||||||
|
|
||||||
Dormant by default — returns immediately unless ``external_pr_enabled``,
|
Dormant by default — returns immediately unless ``external_pr_enabled``
|
||||||
so a standard deployment makes no inbound GitHub call. This only lists
|
OR ``internal_pr_enabled``, so a standard deployment makes no inbound
|
||||||
open PRs and records a review task per newly-seen external one; it never
|
GitHub call. This only lists open PRs and records a review task per
|
||||||
fetches or runs contributor code (that waits on a human confirmation
|
newly-seen reviewable one (external/fork PRs, and — when internal review
|
||||||
downstream). New review tasks wake the dispatcher.
|
is on — org-repo PRs not tied to an active task); it never fetches or
|
||||||
|
runs contributor code (that waits on a human confirmation downstream).
|
||||||
|
New review tasks wake the dispatcher.
|
||||||
"""
|
"""
|
||||||
if not settings.external_pr_enabled:
|
if not (settings.external_pr_enabled or settings.internal_pr_enabled):
|
||||||
return
|
return
|
||||||
from roboco.db import get_db_context
|
from roboco.db import get_db_context
|
||||||
|
|
||||||
@@ -4686,8 +4689,10 @@ Start by:
|
|||||||
|
|
||||||
Repo-aware: collapses active projects to one canonical project per
|
Repo-aware: collapses active projects to one canonical project per
|
||||||
distinct repo (so a monorepo product yields ONE review per PR, not one
|
distinct repo (so a monorepo product yields ONE review per PR, not one
|
||||||
per cell-project), lists each repo's open PRs, keeps the external ones,
|
per cell-project), lists each repo's open PRs, and ingests a de-duped
|
||||||
and ingests a de-duped review task for each. Commits once at the end.
|
review task for each reviewable one — external/fork PRs, and (when
|
||||||
|
internal review is on) org-repo PRs not tied to an active task. Commits
|
||||||
|
once at the end.
|
||||||
"""
|
"""
|
||||||
from roboco.services.git import GitService
|
from roboco.services.git import GitService
|
||||||
from roboco.services.project import get_project_service
|
from roboco.services.project import get_project_service
|
||||||
@@ -4701,22 +4706,54 @@ Start by:
|
|||||||
ingested = 0
|
ingested = 0
|
||||||
for project in self._projects_one_per_repo(projects):
|
for project in self._projects_one_per_repo(projects):
|
||||||
for pr in await git.list_open_prs(project.slug):
|
for pr in await git.list_open_prs(project.slug):
|
||||||
number = pr.get("number")
|
if await self._ingest_pr_if_reviewable(
|
||||||
if number is None or not self._is_external_pr(pr):
|
task_service, project, pr, system_id, allowlist
|
||||||
continue
|
):
|
||||||
if not self._pr_author_allowed(pr, allowlist):
|
|
||||||
continue
|
|
||||||
created = await task_service.ingest_external_pr(
|
|
||||||
project_id=cast("UUID", project.id),
|
|
||||||
pr=pr,
|
|
||||||
created_by=system_id,
|
|
||||||
team=Team.BOARD,
|
|
||||||
)
|
|
||||||
if created is not None:
|
|
||||||
ingested += 1
|
ingested += 1
|
||||||
await db.commit()
|
await db.commit()
|
||||||
return ingested
|
return ingested
|
||||||
|
|
||||||
|
async def _ingest_pr_if_reviewable(
|
||||||
|
self,
|
||||||
|
task_service: "TaskService",
|
||||||
|
project: Any,
|
||||||
|
pr: dict[str, Any],
|
||||||
|
system_id: "UUID",
|
||||||
|
allowlist: set[str],
|
||||||
|
) -> bool:
|
||||||
|
"""Ingest a review task for one open PR if it qualifies; True if ingested.
|
||||||
|
|
||||||
|
External/fork PRs (when external review is on and the author is allowed)
|
||||||
|
are ingested as ``external_pr``. Org-repo PRs whose head branch no active
|
||||||
|
task owns (when internal review is on) are ingested as ``internal_pr`` —
|
||||||
|
the org's own in-flight integration PRs are skipped, since a live task
|
||||||
|
owns their branch and they already pass QA + PM review.
|
||||||
|
"""
|
||||||
|
if pr.get("number") is None:
|
||||||
|
return False
|
||||||
|
if self._is_external_pr(pr):
|
||||||
|
if not settings.external_pr_enabled or not self._pr_author_allowed(
|
||||||
|
pr, allowlist
|
||||||
|
):
|
||||||
|
return False
|
||||||
|
source = "external_pr"
|
||||||
|
else:
|
||||||
|
if not settings.internal_pr_enabled:
|
||||||
|
return False
|
||||||
|
if await task_service.active_task_owns_branch(
|
||||||
|
str(pr.get("head_ref") or "")
|
||||||
|
):
|
||||||
|
return False
|
||||||
|
source = "internal_pr"
|
||||||
|
created = await task_service.ingest_external_pr(
|
||||||
|
project_id=cast("UUID", project.id),
|
||||||
|
pr=pr,
|
||||||
|
created_by=system_id,
|
||||||
|
team=Team.BOARD,
|
||||||
|
source=source,
|
||||||
|
)
|
||||||
|
return created is not None
|
||||||
|
|
||||||
async def _close_superseded_prs(
|
async def _close_superseded_prs(
|
||||||
self, git: Any, task_service: Any, system_id: "UUID"
|
self, git: Any, task_service: Any, system_id: "UUID"
|
||||||
) -> int:
|
) -> int:
|
||||||
@@ -4818,8 +4855,8 @@ Start by:
|
|||||||
async with self._supersede_lock, get_db_context() as db:
|
async with self._supersede_lock, get_db_context() as db:
|
||||||
task_service = get_task_service(db)
|
task_service = get_task_service(db)
|
||||||
review = await task_service.get(review_task_id)
|
review = await task_service.get(review_task_id)
|
||||||
if review is None or getattr(review, "source", "") != "external_pr":
|
if review is None or getattr(review, "source", "") not in PR_REVIEW_SOURCES:
|
||||||
return {"ok": False, "error": "not an external-PR review task"}
|
return {"ok": False, "error": "not a PR-review task"}
|
||||||
if not review.project_id or not review.pr_number:
|
if not review.project_id or not review.pr_number:
|
||||||
return {
|
return {
|
||||||
"ok": False,
|
"ok": False,
|
||||||
@@ -6674,7 +6711,7 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
continue
|
continue
|
||||||
# External-PR review tasks are owned by _dispatch_pr_review_work; the
|
# External-PR review tasks are owned by _dispatch_pr_review_work; the
|
||||||
# PM hierarchy never routes or spawns them.
|
# PM hierarchy never routes or spawns them.
|
||||||
if task.get("source") == "external_pr":
|
if task.get("source") in PR_REVIEW_SOURCES:
|
||||||
continue
|
continue
|
||||||
assigned_to = task.get("assigned_to")
|
assigned_to = task.get("assigned_to")
|
||||||
if assigned_to:
|
if assigned_to:
|
||||||
@@ -7009,7 +7046,7 @@ Never `commit`, never write code, never run `git`. PMs coordinate.
|
|||||||
continue
|
continue
|
||||||
# External-PR review tasks belong to the pr_reviewer, never a dev —
|
# External-PR review tasks belong to the pr_reviewer, never a dev —
|
||||||
# _dispatch_pr_review_work owns them.
|
# _dispatch_pr_review_work owns them.
|
||||||
if task.get("source") == "external_pr":
|
if task.get("source") in PR_REVIEW_SOURCES:
|
||||||
continue
|
continue
|
||||||
await self._dev_dispatch_one(client, task)
|
await self._dev_dispatch_one(client, task)
|
||||||
|
|
||||||
@@ -7265,7 +7302,7 @@ Never `commit`, never write code, never run `git`. PMs coordinate.
|
|||||||
return
|
return
|
||||||
tasks = await self._fetch_tasks(client, "pending")
|
tasks = await self._fetch_tasks(client, "pending")
|
||||||
for task in tasks:
|
for task in tasks:
|
||||||
if task.get("source") != "external_pr":
|
if task.get("source") not in PR_REVIEW_SOURCES:
|
||||||
continue
|
continue
|
||||||
if self._is_task_handled_this_tick(task.get("id")):
|
if self._is_task_handled_this_tick(task.get("id")):
|
||||||
continue
|
continue
|
||||||
|
|||||||
+56
-21
@@ -365,6 +365,13 @@ def extract_required_cells(quick_context: str | None) -> list[str]:
|
|||||||
return []
|
return []
|
||||||
|
|
||||||
|
|
||||||
|
# Review-task sources. The inbound-PR reviewer handles both external/fork PRs
|
||||||
|
# and (when enabled) internal org-repo PRs that bypassed the agent task-flow.
|
||||||
|
# Dispatch, dedup, the decision surface, the git-gate exemption, and supersede
|
||||||
|
# are identical for both, so they share this set.
|
||||||
|
PR_REVIEW_SOURCES = ("external_pr", "internal_pr")
|
||||||
|
|
||||||
|
|
||||||
_SUPERSEDE_MARKER_PREFIX = "external_pr_supersede"
|
_SUPERSEDE_MARKER_PREFIX = "external_pr_supersede"
|
||||||
|
|
||||||
|
|
||||||
@@ -456,7 +463,7 @@ class TaskService(BaseService):
|
|||||||
is_coordination=(task.project_id is None and task.product_id is not None),
|
is_coordination=(task.project_id is None and task.product_id is not None),
|
||||||
# An external-PR review task reviews someone else's PR read-only —
|
# An external-PR review task reviews someone else's PR read-only —
|
||||||
# no branch of its own — so it is branch-gate exempt.
|
# no branch of its own — so it is branch-gate exempt.
|
||||||
is_external_review=(getattr(task, "source", "manual") == "external_pr"),
|
is_external_review=(getattr(task, "source", "manual") in PR_REVIEW_SOURCES),
|
||||||
)
|
)
|
||||||
validate_git_requirements(current, target, git_ctx)
|
validate_git_requirements(current, target, git_ctx)
|
||||||
|
|
||||||
@@ -680,7 +687,7 @@ class TaskService(BaseService):
|
|||||||
result = await self.session.execute(
|
result = await self.session.execute(
|
||||||
select(TaskTable.quick_context).where(
|
select(TaskTable.quick_context).where(
|
||||||
TaskTable.project_id == project_id,
|
TaskTable.project_id == project_id,
|
||||||
TaskTable.source == "external_pr",
|
TaskTable.source.in_(PR_REVIEW_SOURCES),
|
||||||
TaskTable.pr_number == pr_number,
|
TaskTable.pr_number == pr_number,
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
@@ -703,16 +710,17 @@ class TaskService(BaseService):
|
|||||||
pr: dict[str, Any],
|
pr: dict[str, Any],
|
||||||
created_by: UUID,
|
created_by: UUID,
|
||||||
team: Team,
|
team: Team,
|
||||||
|
source: str = "external_pr",
|
||||||
) -> TaskTable | None:
|
) -> TaskTable | None:
|
||||||
"""Create one review task for a newly-seen external PR; ``None`` if it exists.
|
"""Create one review task for a newly-seen inbound PR; ``None`` if it exists.
|
||||||
|
|
||||||
``pr`` is a normalized record from ``GitService.list_open_prs`` (number,
|
``pr`` is a normalized record from ``GitService.list_open_prs`` (number,
|
||||||
url, title, head_sha). De-duped per ``(project_id, pr_number, head_sha)``
|
url, title, head_sha). De-duped per ``(project_id, pr_number, head_sha)``
|
||||||
— re-polling an unchanged PR is skipped, but new commits (a new head SHA)
|
across both review sources — re-polling an unchanged PR is skipped, but
|
||||||
open a fresh review (see ``external_review_task_exists``).
|
new commits (a new head SHA) open a fresh review (see
|
||||||
The task is CODE-typed with ``source='external_pr'`` and
|
``external_review_task_exists``). ``source`` is ``external_pr`` (fork /
|
||||||
``confirmed_by_human=False`` — a deliberate gate: no agent fetches, checks
|
untrusted) or ``internal_pr`` (an org-repo PR opened outside the agent
|
||||||
out, or runs the contributor's code until a human confirms the PR. Caller
|
task-flow). Both are CODE-typed with ``confirmed_by_human=False``. Caller
|
||||||
commits.
|
commits.
|
||||||
"""
|
"""
|
||||||
pr_number = int(pr["number"])
|
pr_number = int(pr["number"])
|
||||||
@@ -721,16 +729,25 @@ class TaskService(BaseService):
|
|||||||
head_sha = str(pr.get("head_sha") or "")
|
head_sha = str(pr.get("head_sha") or "")
|
||||||
if await self.external_review_task_exists(project_id, pr_number, head_sha):
|
if await self.external_review_task_exists(project_id, pr_number, head_sha):
|
||||||
return None
|
return None
|
||||||
title = f"Review external PR #{pr_number}: {pr_title}".strip()
|
kind = "internal" if source == "internal_pr" else "external"
|
||||||
|
title = f"Review {kind} PR #{pr_number}: {pr_title}".strip()
|
||||||
|
if source == "internal_pr":
|
||||||
|
description = (
|
||||||
|
f"An internal PR #{pr_number} ({pr_url}) was opened on an org repo "
|
||||||
|
"outside the agent task-flow — no active task owns its branch. "
|
||||||
|
"Review it adversarially and post a single, complete change-request "
|
||||||
|
"with per-criterion findings."
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
description = (
|
||||||
|
f"An external contributor opened PR #{pr_number} ({pr_url}).\n\n"
|
||||||
|
"Review it adversarially and post a single, complete change-request "
|
||||||
|
"with per-criterion findings. Do not fetch, check out, or run the "
|
||||||
|
"contributor's code until a human has confirmed this PR."
|
||||||
|
)
|
||||||
req = TaskCreateRequest(
|
req = TaskCreateRequest(
|
||||||
title=title[:200],
|
title=title[:200],
|
||||||
description=(
|
description=description,
|
||||||
f"An external contributor opened PR #{pr_number} ({pr_url}).\n\n"
|
|
||||||
"Review it adversarially and post a single, complete "
|
|
||||||
"change-request with per-criterion findings. Do not fetch, check "
|
|
||||||
"out, or run the contributor's code until a human has confirmed "
|
|
||||||
"this PR."
|
|
||||||
),
|
|
||||||
acceptance_criteria=[
|
acceptance_criteria=[
|
||||||
"Exactly one complete GitHub review is posted with per-criterion "
|
"Exactly one complete GitHub review is posted with per-criterion "
|
||||||
"findings",
|
"findings",
|
||||||
@@ -741,7 +758,7 @@ class TaskService(BaseService):
|
|||||||
nature=TaskNature.TECHNICAL,
|
nature=TaskNature.TECHNICAL,
|
||||||
estimated_complexity=Complexity.MEDIUM,
|
estimated_complexity=Complexity.MEDIUM,
|
||||||
project_id=project_id,
|
project_id=project_id,
|
||||||
source="external_pr",
|
source=source,
|
||||||
confirmed_by_human=False,
|
confirmed_by_human=False,
|
||||||
)
|
)
|
||||||
task = await self.create(req)
|
task = await self.create(req)
|
||||||
@@ -754,6 +771,24 @@ class TaskService(BaseService):
|
|||||||
await self.session.flush()
|
await self.session.flush()
|
||||||
return task
|
return task
|
||||||
|
|
||||||
|
async def active_task_owns_branch(self, branch_name: str) -> bool:
|
||||||
|
"""True if a non-terminal task already owns this git branch.
|
||||||
|
|
||||||
|
Lets the internal-PR reviewer skip the org's own in-flight integration
|
||||||
|
PRs — those whose head branch a live task created via the agent
|
||||||
|
task-flow (and which therefore already pass QA + PM review) — and review
|
||||||
|
only org-repo PRs opened outside that flow.
|
||||||
|
"""
|
||||||
|
if not branch_name:
|
||||||
|
return False
|
||||||
|
result = await self.session.execute(
|
||||||
|
select(TaskTable.id).where(
|
||||||
|
TaskTable.branch_name == branch_name,
|
||||||
|
TaskTable.status.notin_([TaskStatus.COMPLETED, TaskStatus.CANCELLED]),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
return result.first() is not None
|
||||||
|
|
||||||
async def list_external_pr_reviews_awaiting_decision(self) -> list[TaskTable]:
|
async def list_external_pr_reviews_awaiting_decision(self) -> list[TaskTable]:
|
||||||
"""Completed external-PR reviews still awaiting the CEO's decision.
|
"""Completed external-PR reviews still awaiting the CEO's decision.
|
||||||
|
|
||||||
@@ -764,7 +799,7 @@ class TaskService(BaseService):
|
|||||||
"""
|
"""
|
||||||
result = await self.session.execute(
|
result = await self.session.execute(
|
||||||
select(TaskTable).where(
|
select(TaskTable).where(
|
||||||
TaskTable.source == "external_pr",
|
TaskTable.source.in_(PR_REVIEW_SOURCES),
|
||||||
TaskTable.status == TaskStatus.COMPLETED,
|
TaskTable.status == TaskStatus.COMPLETED,
|
||||||
TaskTable.confirmed_by_human.is_(False),
|
TaskTable.confirmed_by_human.is_(False),
|
||||||
)
|
)
|
||||||
@@ -792,7 +827,7 @@ class TaskService(BaseService):
|
|||||||
"""
|
"""
|
||||||
result = await self.session.execute(
|
result = await self.session.execute(
|
||||||
select(TaskTable).where(
|
select(TaskTable).where(
|
||||||
TaskTable.source == "external_pr",
|
TaskTable.source.in_(PR_REVIEW_SOURCES),
|
||||||
TaskTable.status != TaskStatus.CANCELLED,
|
TaskTable.status != TaskStatus.CANCELLED,
|
||||||
or_(
|
or_(
|
||||||
TaskTable.status != TaskStatus.COMPLETED,
|
TaskTable.status != TaskStatus.COMPLETED,
|
||||||
@@ -814,7 +849,7 @@ class TaskService(BaseService):
|
|||||||
is missing or is not an external-PR review.
|
is missing or is not an external-PR review.
|
||||||
"""
|
"""
|
||||||
task = await self.get(task_id)
|
task = await self.get(task_id)
|
||||||
if task is None or getattr(task, "source", "") != "external_pr":
|
if task is None or getattr(task, "source", "") not in PR_REVIEW_SOURCES:
|
||||||
return None
|
return None
|
||||||
if "dismissed=1" not in (task.quick_context or "").split():
|
if "dismissed=1" not in (task.quick_context or "").split():
|
||||||
task.quick_context = f"{task.quick_context or ''} dismissed=1".strip()
|
task.quick_context = f"{task.quick_context or ''} dismissed=1".strip()
|
||||||
@@ -901,7 +936,7 @@ class TaskService(BaseService):
|
|||||||
task is missing or is not an external-PR review.
|
task is missing or is not an external-PR review.
|
||||||
"""
|
"""
|
||||||
review = await self.get(review_task_id)
|
review = await self.get(review_task_id)
|
||||||
if review is None or getattr(review, "source", "") != "external_pr":
|
if review is None or getattr(review, "source", "") not in PR_REVIEW_SOURCES:
|
||||||
return None
|
return None
|
||||||
pr_number = review.pr_number
|
pr_number = review.pr_number
|
||||||
req = TaskCreateRequest(
|
req = TaskCreateRequest(
|
||||||
|
|||||||
@@ -8,8 +8,11 @@ side) for anything it does not positively recognize as internal.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
|
from unittest.mock import AsyncMock, MagicMock
|
||||||
|
from uuid import uuid4
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
import roboco.runtime.orchestrator as orch_mod
|
||||||
from roboco.runtime.orchestrator import AgentOrchestrator
|
from roboco.runtime.orchestrator import AgentOrchestrator
|
||||||
|
|
||||||
|
|
||||||
@@ -100,3 +103,96 @@ def test_pr_author_allowed(
|
|||||||
)
|
)
|
||||||
def test_parse_supersede_pr(quick_context: str, expected: int | None) -> None:
|
def test_parse_supersede_pr(quick_context: str, expected: int | None) -> None:
|
||||||
assert AgentOrchestrator._parse_supersede_pr(quick_context) == expected
|
assert AgentOrchestrator._parse_supersede_pr(quick_context) == expected
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# _ingest_pr_if_reviewable — the external/internal review decision (#3)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _orch() -> AgentOrchestrator:
|
||||||
|
"""A bare orchestrator — the method only uses staticmethods + the service."""
|
||||||
|
return object.__new__(AgentOrchestrator)
|
||||||
|
|
||||||
|
|
||||||
|
def _svc(*, owns_branch: bool = False) -> MagicMock:
|
||||||
|
svc = MagicMock()
|
||||||
|
svc.ingest_external_pr = AsyncMock(return_value=object()) # truthy "created"
|
||||||
|
svc.active_task_owns_branch = AsyncMock(return_value=owns_branch)
|
||||||
|
return svc
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_ingest_external_fork_when_enabled(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
monkeypatch.setattr(orch_mod.settings, "external_pr_enabled", True)
|
||||||
|
svc = _svc()
|
||||||
|
pr = {"number": 5, "is_fork": True, "user_login": "corey", "head_ref": "x"}
|
||||||
|
ok = await _orch()._ingest_pr_if_reviewable(
|
||||||
|
svc, SimpleNamespace(id=uuid4()), pr, uuid4(), set()
|
||||||
|
)
|
||||||
|
assert ok is True
|
||||||
|
assert svc.ingest_external_pr.await_args.kwargs["source"] == "external_pr"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_skip_external_when_disabled(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
monkeypatch.setattr(orch_mod.settings, "external_pr_enabled", False)
|
||||||
|
svc = _svc()
|
||||||
|
pr = {"number": 5, "is_fork": True, "user_login": "corey"}
|
||||||
|
ok = await _orch()._ingest_pr_if_reviewable(
|
||||||
|
svc, SimpleNamespace(id=uuid4()), pr, uuid4(), set()
|
||||||
|
)
|
||||||
|
assert ok is False
|
||||||
|
svc.ingest_external_pr.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_ingest_internal_off_task_flow(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
# A non-fork org PR no live task owns is an off-task-flow PR → review it.
|
||||||
|
monkeypatch.setattr(orch_mod.settings, "internal_pr_enabled", True)
|
||||||
|
svc = _svc(owns_branch=False)
|
||||||
|
pr = {
|
||||||
|
"number": 9,
|
||||||
|
"is_fork": False,
|
||||||
|
"author_association": "MEMBER",
|
||||||
|
"head_ref": "hotfix/manual",
|
||||||
|
}
|
||||||
|
ok = await _orch()._ingest_pr_if_reviewable(
|
||||||
|
svc, SimpleNamespace(id=uuid4()), pr, uuid4(), set()
|
||||||
|
)
|
||||||
|
assert ok is True
|
||||||
|
assert svc.ingest_external_pr.await_args.kwargs["source"] == "internal_pr"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_skip_internal_lifecycle_pr(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
# A live task owns the branch → it's the org's own integration PR → skip.
|
||||||
|
monkeypatch.setattr(orch_mod.settings, "internal_pr_enabled", True)
|
||||||
|
svc = _svc(owns_branch=True)
|
||||||
|
pr = {
|
||||||
|
"number": 9,
|
||||||
|
"is_fork": False,
|
||||||
|
"author_association": "MEMBER",
|
||||||
|
"head_ref": "feature/main_pm/abc",
|
||||||
|
}
|
||||||
|
ok = await _orch()._ingest_pr_if_reviewable(
|
||||||
|
svc, SimpleNamespace(id=uuid4()), pr, uuid4(), set()
|
||||||
|
)
|
||||||
|
assert ok is False
|
||||||
|
svc.ingest_external_pr.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_skip_internal_when_disabled(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
monkeypatch.setattr(orch_mod.settings, "internal_pr_enabled", False)
|
||||||
|
svc = _svc()
|
||||||
|
pr = {"number": 9, "is_fork": False, "author_association": "MEMBER"}
|
||||||
|
ok = await _orch()._ingest_pr_if_reviewable(
|
||||||
|
svc, SimpleNamespace(id=uuid4()), pr, uuid4(), set()
|
||||||
|
)
|
||||||
|
assert ok is False
|
||||||
|
svc.ingest_external_pr.assert_not_awaited()
|
||||||
|
|||||||
@@ -108,3 +108,35 @@ async def test_dismiss_rejects_non_external_pr() -> None:
|
|||||||
svc = TaskService(session)
|
svc = TaskService(session)
|
||||||
_bind(svc, "get", AsyncMock(return_value=task))
|
_bind(svc, "get", AsyncMock(return_value=task))
|
||||||
assert await svc.dismiss_external_pr_review(uuid4()) is None
|
assert await svc.dismiss_external_pr_review(uuid4()) is None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# active_task_owns_branch — the internal-PR "is this a lifecycle PR?" check (#3)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _branch_service(*, found: bool) -> TaskService:
|
||||||
|
res = MagicMock()
|
||||||
|
res.first.return_value = ("some-task-id",) if found else None
|
||||||
|
session = MagicMock()
|
||||||
|
session.execute = AsyncMock(return_value=res)
|
||||||
|
return TaskService(session)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_active_task_owns_branch_true_when_live_task_holds_it() -> None:
|
||||||
|
assert (
|
||||||
|
await _branch_service(found=True).active_task_owns_branch("feature/x") is True
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_active_task_owns_branch_false_when_no_live_task() -> None:
|
||||||
|
svc = _branch_service(found=False)
|
||||||
|
assert await svc.active_task_owns_branch("feature/x") is False
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_active_task_owns_branch_false_for_empty_branch() -> None:
|
||||||
|
# No branch → cannot be owned; never hits the DB.
|
||||||
|
assert await TaskService(MagicMock()).active_task_owns_branch("") is False
|
||||||
|
|||||||
Reference in New Issue
Block a user