[F139] scope active_task_owns_branch to the polled project

active_task_owns_branch did an unscoped WHERE branch_name = ? — a cross-project
branch_name collision (UUID-derived 8-char prefixes, theoretical) made the
internal-PR reviewer skip the WRONG project's PR (project A's leftover PR
skipped because project B happened to have an active task with the same
branch). Pass project_id (in scope at the orchestrator call site) and add
TaskTable.project_id == project_id to the WHERE. Correct for single-project
tasks and MegaTask multi-repo batches alike: each root-subtask carries its own
project_id matching its own repo, so a branch on project A's repo is owned
only by a task whose project_id == A.
This commit is contained in:
Renn F
2026-06-29 00:49:36 +02:00
parent e8f5d531c7
commit fb850e8235
4 changed files with 138 additions and 6 deletions
+2 -1
View File
@@ -6604,7 +6604,8 @@ Start by:
if not settings.internal_pr_enabled: if not settings.internal_pr_enabled:
return False return False
if await task_service.active_task_owns_branch( if await task_service.active_task_owns_branch(
str(pr.get("head_ref") or "") str(pr.get("head_ref") or ""),
cast("UUID", project.id),
): ):
return False return False
source = "internal_pr" source = "internal_pr"
+9 -2
View File
@@ -1144,19 +1144,26 @@ 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: async def active_task_owns_branch(self, branch_name: str, project_id: UUID) -> bool:
"""True if a non-terminal task already owns this git branch. """True if a non-terminal task on ``project_id`` already owns this branch.
Lets the internal-PR reviewer skip the org's own in-flight integration 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 PRs those whose head branch a live task created via the agent
task-flow (and which therefore already pass QA + PM review) and review task-flow (and which therefore already pass QA + PM review) and review
only org-repo PRs opened outside that flow. only org-repo PRs opened outside that flow.
Scoped to the polled project: a branch in project A's repo can only be
owned by a task whose ``project_id == A`` (each task branches in its
own project's repo, including each root-subtask of a multi-repo
MegaTask). An unscoped lookup would match the wrong project's task on a
cross-project branch_name collision and false-skip project A's PR.
""" """
if not branch_name: if not branch_name:
return False return False
result = await self.session.execute( result = await self.session.execute(
select(TaskTable.id).where( select(TaskTable.id).where(
TaskTable.branch_name == branch_name, TaskTable.branch_name == branch_name,
TaskTable.project_id == project_id,
TaskTable.status.notin_([TaskStatus.COMPLETED, TaskStatus.CANCELLED]), TaskTable.status.notin_([TaskStatus.COMPLETED, TaskStatus.CANCELLED]),
) )
) )
@@ -0,0 +1,123 @@
"""active_task_owns_branch must be scoped to the polled project.
The internal-PR reviewer (orchestrator) calls this to skip the org's own
in-flight integration PRs — a PR on project A's repo is "ours" only if a
non-terminal task ON PROJECT A owns its head branch. The query was unscoped
(``WHERE branch_name = ?``), so a cross-project branch_name collision (two
tasks sharing an 8-char-UUID-prefix branch on different projects) made it
match the WRONG project's task — project A's leftover PR was skipped because
project B happened to have an active task with the same branch_name.
Scoping by ``project_id`` is correct for both single-project tasks and
MegaTask multi-repo batches: each root-subtask carries its own ``project_id``
matching its own repo, so a branch on project A's repo is owned only by a
task whose ``project_id == A``.
"""
from __future__ import annotations
from typing import TYPE_CHECKING, cast
from uuid import UUID, uuid4
import pytest
from roboco.db.tables import AgentTable, ProjectTable, TaskTable
from roboco.foundation import identity as _foundation
from roboco.models.base import (
AgentRole,
AgentStatus,
Complexity,
TaskNature,
TaskStatus,
TaskType,
Team,
)
from roboco.services.task import get_task_service
if TYPE_CHECKING:
from sqlalchemy.ext.asyncio import AsyncSession
SYSTEM_UUID = _foundation.AGENTS["system"].uuid
_BRANCH = "feature/backend/collide001"
async def _seed_project(db: AsyncSession, slug: str) -> ProjectTable:
if await db.get(AgentTable, SYSTEM_UUID) is None:
db.add(
AgentTable(
id=SYSTEM_UUID,
name="System",
slug=f"system-{uuid4().hex[:8]}",
role=AgentRole.SYSTEM,
team=None,
status=AgentStatus.ACTIVE,
model_config={},
system_prompt="x",
capabilities=[],
permissions={},
metrics={},
)
)
await db.flush()
project = ProjectTable(
id=uuid4(),
name=slug,
slug=slug,
git_url=f"https://github.com/rennf93/{slug}",
assigned_cell=Team.BACKEND,
created_by=SYSTEM_UUID,
)
db.add(project)
await db.flush()
return project
def _task(project_id, *, branch: str, status: TaskStatus) -> TaskTable:
return TaskTable(
id=uuid4(),
title=f"task {branch}",
description="x",
acceptance_criteria=["criterion"],
status=status,
priority=2,
task_type=TaskType.CODE,
nature=TaskNature.TECHNICAL,
project_id=project_id,
branch_name=branch,
created_by=SYSTEM_UUID,
team=Team.BACKEND,
dependency_ids=[],
blocker_ids=[],
sequence=0,
estimated_complexity=Complexity.MEDIUM,
)
@pytest.mark.asyncio
async def test_branch_owned_only_by_its_own_project(db_session: AsyncSession) -> None:
"""Project A has an ACTIVE task on ``_BRANCH``; project B has a COMPLETED
task with the SAME ``_BRANCH``. Asking for project B must NOT match
project A's active task (the unscoped query did). Asking for project A
matches its own active task."""
proj_a = await _seed_project(db_session, "gca-collide-a")
proj_b = await _seed_project(db_session, "gca-collide-b")
db_session.add_all(
[
_task(proj_a.id, branch=_BRANCH, status=TaskStatus.IN_PROGRESS),
_task(proj_b.id, branch=_BRANCH, status=TaskStatus.COMPLETED),
]
)
await db_session.flush()
svc = get_task_service(db_session)
# The active task on project A owns it for project A.
assert await svc.active_task_owns_branch(_BRANCH, cast("UUID", proj_a.id)) is True
# Project B's task is terminal AND the only active owner is on project A —
# the unscoped query would wrongly return True here (matching A's task).
assert await svc.active_task_owns_branch(_BRANCH, cast("UUID", proj_b.id)) is False
@pytest.mark.asyncio
async def test_empty_branch_never_owned(db_session: AsyncSession) -> None:
proj = await _seed_project(db_session, "gca-collide-empty")
svc = get_task_service(db_session)
assert await svc.active_task_owns_branch("", cast("UUID", proj.id)) is False
@@ -146,17 +146,18 @@ def _branch_service(*, found: bool) -> TaskService:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_active_task_owns_branch_true_when_live_task_holds_it() -> None: async def test_active_task_owns_branch_true_when_live_task_holds_it() -> None:
assert ( assert (
await _branch_service(found=True).active_task_owns_branch("feature/x") is True await _branch_service(found=True).active_task_owns_branch("feature/x", uuid4())
is True
) )
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_active_task_owns_branch_false_when_no_live_task() -> None: async def test_active_task_owns_branch_false_when_no_live_task() -> None:
svc = _branch_service(found=False) svc = _branch_service(found=False)
assert await svc.active_task_owns_branch("feature/x") is False assert await svc.active_task_owns_branch("feature/x", uuid4()) is False
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_active_task_owns_branch_false_for_empty_branch() -> None: async def test_active_task_owns_branch_false_for_empty_branch() -> None:
# No branch → cannot be owned; never hits the DB. # No branch → cannot be owned; never hits the DB.
assert await TaskService(MagicMock()).active_task_owns_branch("") is False assert await TaskService(MagicMock()).active_task_owns_branch("", uuid4()) is False