mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(orchestrator): repo-aware external-PR polling (monorepo no longer triplicates)
Multiple projects can map to ONE repo — a monorepo product's backend/ frontend/ux cells each have their own Project pointing at the same git_url. The poll ingested per-project with a per-(project,pr) dedup, so one external PR (e.g. #170 on github.com/rennf93/roboco) created one review task per cell project — three identical reviews for the same PR. Collapse active projects to one canonical project per distinct repo before polling (_projects_one_per_repo, deterministic by slug so the pick is stable across polls). A monorepo product now yields ONE review per external PR; genuinely separate repos (multi-repo) each still get polled.
This commit is contained in:
@@ -4645,11 +4645,43 @@ Start by:
|
|||||||
except Exception:
|
except Exception:
|
||||||
logger.exception("external-PR poll cycle failed")
|
logger.exception("external-PR poll cycle failed")
|
||||||
|
|
||||||
async def _poll_external_prs_once(self, db: "AsyncSession") -> int:
|
@staticmethod
|
||||||
"""One discovery pass across active projects; returns tasks ingested.
|
def _repo_key(git_url: str) -> str:
|
||||||
|
"""Normalized repo identity (case/.git/trailing-slash insensitive)."""
|
||||||
|
return git_url.lower().rstrip("/").removesuffix(".git")
|
||||||
|
|
||||||
Lists each active project's open PRs, keeps the external ones, and
|
@classmethod
|
||||||
ingests a de-duped review task for each. Commits once at the end.
|
def _projects_one_per_repo(cls, projects: list[Any]) -> list[Any]:
|
||||||
|
"""One canonical project per distinct repo.
|
||||||
|
|
||||||
|
Many projects can point at the SAME repo — a monorepo product's
|
||||||
|
backend/frontend/ux cells each have their own Project mapping to one
|
||||||
|
git_url. Polling per-project would then ingest one review task per cell
|
||||||
|
for a single external PR (the per-(project,pr) dedup can't see across
|
||||||
|
projects). Collapse to one canonical project per repo (deterministic by
|
||||||
|
slug so the pick is stable across polls); genuinely separate repos
|
||||||
|
(multi-repo) each keep their own. Projects without a git_url are skipped.
|
||||||
|
"""
|
||||||
|
seen: set[str] = set()
|
||||||
|
canonical: list[Any] = []
|
||||||
|
for project in sorted(projects, key=lambda p: str(p.slug)):
|
||||||
|
git_url = getattr(project, "git_url", None)
|
||||||
|
if not git_url:
|
||||||
|
continue
|
||||||
|
key = cls._repo_key(git_url)
|
||||||
|
if key in seen:
|
||||||
|
continue
|
||||||
|
seen.add(key)
|
||||||
|
canonical.append(project)
|
||||||
|
return canonical
|
||||||
|
|
||||||
|
async def _poll_external_prs_once(self, db: "AsyncSession") -> int:
|
||||||
|
"""One discovery pass across active repos; returns tasks ingested.
|
||||||
|
|
||||||
|
Repo-aware: collapses active projects to one canonical project per
|
||||||
|
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,
|
||||||
|
and ingests a de-duped review task for each. 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
|
||||||
@@ -4661,7 +4693,7 @@ Start by:
|
|||||||
system_id = _foundation.AGENTS["system"].uuid
|
system_id = _foundation.AGENTS["system"].uuid
|
||||||
allowlist = {a.lower() for a in settings.external_pr_author_allowlist}
|
allowlist = {a.lower() for a in settings.external_pr_author_allowlist}
|
||||||
ingested = 0
|
ingested = 0
|
||||||
for project in 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")
|
number = pr.get("number")
|
||||||
if number is None or not self._is_external_pr(pr):
|
if number is None or not self._is_external_pr(pr):
|
||||||
|
|||||||
@@ -7,10 +7,42 @@ side) for anything it does not positively recognize as internal.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from types import SimpleNamespace
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from roboco.runtime.orchestrator import AgentOrchestrator
|
from roboco.runtime.orchestrator import AgentOrchestrator
|
||||||
|
|
||||||
|
|
||||||
|
def _proj(slug: str, git_url: str | None) -> SimpleNamespace:
|
||||||
|
return SimpleNamespace(slug=slug, git_url=git_url)
|
||||||
|
|
||||||
|
|
||||||
|
def test_projects_one_per_repo_collapses_monorepo() -> None:
|
||||||
|
# Three cell-projects all pointing at the same repo (a monorepo product)
|
||||||
|
# collapse to ONE canonical project; a genuinely separate repo is kept.
|
||||||
|
projects = [
|
||||||
|
_proj("roboco-uix", "https://github.com/rennf93/roboco.git"),
|
||||||
|
_proj("roboco-api", "https://github.com/rennf93/roboco.git"),
|
||||||
|
_proj("roboco-panel", "https://github.com/rennf93/roboco.git"),
|
||||||
|
_proj("other", "https://github.com/rennf93/other-repo.git"),
|
||||||
|
]
|
||||||
|
out = AgentOrchestrator._projects_one_per_repo(projects)
|
||||||
|
slugs = [p.slug for p in out]
|
||||||
|
# one per distinct repo; canonical pick is deterministic (first by slug).
|
||||||
|
assert slugs == ["other", "roboco-api"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_projects_one_per_repo_normalizes_and_skips_repoless() -> None:
|
||||||
|
projects = [
|
||||||
|
_proj("a", "https://github.com/rennf93/roboco"), # no .git
|
||||||
|
_proj("b", "https://github.com/rennf93/roboco.git/"), # .git + slash
|
||||||
|
_proj("coordination", None), # product/coordination project, no repo
|
||||||
|
]
|
||||||
|
out = AgentOrchestrator._projects_one_per_repo(projects)
|
||||||
|
# a & b are the same repo; coordination (no git_url) is skipped.
|
||||||
|
assert [p.slug for p in out] == ["a"]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize(
|
@pytest.mark.parametrize(
|
||||||
("pr", "expected"),
|
("pr", "expected"),
|
||||||
[
|
[
|
||||||
|
|||||||
Reference in New Issue
Block a user