External/roboco
1
0
Fork 0
mirror of https://github.com/rennf93/roboco.git synced 2026-08-03 07:23:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

[chore] ci-watch/dep-update dedupe: normalize git_url + treat empty-string workflow as default (#148 #1267)

Browse Source 
The per-repo open-task dedupe filtered ProjectTable.git_url == git_url
(exact), while the orchestrator collapses its poll set by repo_key
(lower / strip trailing '/' / drop '.git'). Two projects whose git_url
differs only by those accidentals (a monorepo's cell-projects, or a
re-registered canonical project) defeated the one-open-task-per-repo
invariant and opened duplicate fix / dep-update tasks. Extract
roboco.utils.converters.repo_key as the single source and match the
dedupe query on its SQL mirror (regexp_replace(rtrim(lower(...)))).

The ci_watch (git_url, workflow) dedupe used func.coalesce(ci_watch_workflow,
default), but SQL COALESCE only substitutes for NULL — a project saved with
ci_watch_workflow='' (reachable via panel/API) yielded coalesce('', default)
= '' != default, so the DB diverged from the engine/orchestrator (which
collapse '' to the default via Python truthiness) and opened a duplicate
fix task every red cycle. Wrap with func.nullif(..., '') so an empty string
collapses to the default too.

Tests: a ''-workflow + NULL-workflow project on one repo dedupe to one task;
git_url accidentals (.git suffix / trailing slash) dedupe across both
ci_watch and dep_update. The orchestrator _repo_key now delegates to repo_key.
This commit is contained in:
Renn F
2026-06-30 19:19:31 +02:00
parent 6907103073
commit d34bc1a7a5
5 changed files with 100 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
roboco/runtime/orchestrator.py
+8 -2
View File
@@ -6828,8 +6828,14 @@ Start by:
@staticmethod
def _repo_key(git_url: str) -> str:
"""Normalized repo identity (case/.git/trailing-slash insensitive)."""
return git_url.lower().rstrip("/").removesuffix(".git")
"""Normalized repo identity (case/.git/trailing-slash insensitive).
Delegates to :func:`roboco.utils.converters.repo_key` so the dedupe
queries and the poll-set collapse share one source of truth (#1267).
"""
from roboco.utils.converters import repo_key
return repo_key(git_url)
@classmethod
def _projects_one_per_repo(cls, projects: list[Any]) -> list[Any]:
roboco/services/task.py
+25 -6
View File
@@ -71,7 +71,7 @@ from roboco.services.base import (
)
from roboco.services.content_notes import apply_structured_note
from roboco.services.work_session import WorkSessionService
from roboco.utils.converters import require_uuid, to_python_uuid
from roboco.utils.converters import repo_key, require_uuid, to_python_uuid
if TYPE_CHECKING:
from roboco.services.permissions import PermissionService
@@ -81,6 +81,14 @@ _UUID_LENGTH = 36 # Standard UUID string length
_UUID_HYPHEN_COUNT = 4 # Number of hyphens in a UUID
def _repo_key_expr(column: Any) -> Any:
"""SQL mirror of :func:`roboco.utils.converters.repo_key`: lower, strip a
trailing ``/``, drop a ``.git`` suffix — so two projects whose git_url
differs only by case / ``.git`` / trailing-slash collapse to one repo for
ci_watch / dep_update dedupe (#1267)."""
return func.regexp_replace(func.rtrim(func.lower(column), "/"), r"\.git$", "")
_ROLE_CLAIM_STATUSES: dict[str, set[TaskStatus]] = {
"qa": {TaskStatus.PENDING, TaskStatus.AWAITING_QA},
"documenter": {TaskStatus.PENDING, TaskStatus.AWAITING_DOCUMENTATION},
@@ -1254,8 +1262,14 @@ class TaskService(BaseService):
dedupe key is ``(git_url, effective workflow)`` so a multi-workflow
monorepo with two RED workflows gets a fix task per workflow, not one
collapsed task that silently leaves the second workflow un-remediated
(#44). The effective workflow is ``COALESCE(ci_watch_workflow, default)``
so a NULL-workflow project row matches the default workflow.
(#44). The effective workflow is
``COALESCE(NULLIF(ci_watch_workflow, ''), default)`` so a NULL OR
empty-string workflow row matches the default workflow — the engine and
orchestrator collapse an empty string to the default via Python
truthiness, and the DB dedupe must match that semantics or a
''-workflow repo opens a duplicate fix task every red cycle (#148). The
git_url scope is matched on the normalized repo key (case / ``.git`` /
trailing ``/``), mirroring the orchestrator's poll-set collapse (#1267).
"""
stmt = select(TaskTable).where(
TaskTable.source == CI_WATCH_SOURCE,
@@ -1264,12 +1278,15 @@ class TaskService(BaseService):
if git_url is not None or workflow is not None:
stmt = stmt.join(ProjectTable, TaskTable.project_id == ProjectTable.id)
if git_url is not None:
stmt = stmt.where(ProjectTable.git_url == git_url)
stmt = stmt.where(
_repo_key_expr(ProjectTable.git_url) == repo_key(git_url)
)
if workflow is not None:
from roboco.config import settings
effective = func.coalesce(
ProjectTable.ci_watch_workflow, settings.ci_watch_default_workflow
func.nullif(ProjectTable.ci_watch_workflow, ""),
settings.ci_watch_default_workflow,
)
stmt = stmt.where(effective == workflow)
result = await self.session.execute(stmt)
@@ -1284,6 +1301,8 @@ class TaskService(BaseService):
cell-projects, one git_url) gets at most one open dependency-update task,
not one per cell-project. While an open task exists for a repo the bot
must not originate a second; the rolling open-task cap counts these.
The git_url scope is matched on the normalized repo key (case / ``.git``
/ trailing ``/``), mirroring the orchestrator's poll-set collapse (#1267).
"""
stmt = select(TaskTable).where(
TaskTable.source == DEP_UPDATE_SOURCE,
@@ -1292,7 +1311,7 @@ class TaskService(BaseService):
if git_url is not None:
stmt = stmt.join(
ProjectTable, TaskTable.project_id == ProjectTable.id
).where(ProjectTable.git_url == git_url)
).where(_repo_key_expr(ProjectTable.git_url) == repo_key(git_url))
result = await self.session.execute(stmt)
return list(result.scalars().all())
roboco/utils/converters.py
+14
View File
@@ -28,6 +28,20 @@ def require_uuid(value: Any) -> PythonUUID:
return PythonUUID(str(value))
def repo_key(git_url: str) -> str:
"""Normalized repo identity — case / ``.git`` suffix / trailing-slash
insensitive.
Two projects registered with git_url strings that differ only by those
accidentals are the SAME repo for ci_watch / dep_update dedupe (a monorepo
often registers several cell-projects on one git_url, and a re-registered
canonical project may carry a slightly different string). The orchestrator
collapses its poll set by this key; the dedupe queries mirror it so the
one-open-task-per-repo invariant holds across the accidentals (#1267).
"""
return git_url.lower().rstrip("/").removesuffix(".git")
def to_python_uuid(value: Any) -> PythonUUID | None:
"""
Convert SQLAlchemy UUID to Python UUID.
tests/integration/services/test_ci_watch_engine.py
+37
View File
@@ -209,3 +209,40 @@ async def test_default_workflow_null_rows_deduped(
src = _FakeSource([_breach("mono3-a"), _breach("mono3-b")])
created = await get_ci_watch_engine(db_session, source=src).run_cycle([p1, p2])
assert len(created) == 1
# ---------------------------------------------------------------------------
# #148: an empty-string ci_watch_workflow (saved via panel/API, not NULL) must
# collapse to the default workflow for dedupe — SQL COALESCE alone treats '' as
# a real value, so the DB query diverged from the engine's Python truthiness and
# opened a duplicate fix task every red cycle.
# #1267: git_url accidentals (case / .git suffix / trailing slash) must collapse
# to one repo for dedupe, mirroring the orchestrator's poll-set repo_key.
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_empty_string_workflow_deduped(db_session: AsyncSession) -> None:
"""#148: a ''-workflow project and a NULL-workflow project on one repo both
use the default workflow -> deduped to one task (NULLIF treats '' as NULL)."""
git = "https://github.com/x/empty-wf.git"
p1 = await _seed_project(db_session, "empty-wf-a", git, workflow="")
p2 = await _seed_project(db_session, "empty-wf-b", git) # ci_watch_workflow=None
src = _FakeSource([_breach("empty-wf-a"), _breach("empty-wf-b")])
created = await get_ci_watch_engine(db_session, source=src).run_cycle([p1, p2])
assert len(created) == 1
@pytest.mark.asyncio
async def test_git_url_accidentals_deduped(db_session: AsyncSession) -> None:
"""#1267: two projects whose git_url differs only by a ``.git`` suffix (and
trailing slash) are the same repo -> one fix task, not two."""
p1 = await _seed_project(
db_session, "acc-a", "https://github.com/x/acc.git", workflow="wf.yml"
)
p2 = await _seed_project(
db_session, "acc-b", "https://github.com/x/acc/", workflow="wf.yml"
)
src = _FakeSource([_breach("acc-a"), _breach("acc-b")])
created = await get_ci_watch_engine(db_session, source=src).run_cycle([p1, p2])
assert len(created) == 1
tests/integration/services/test_dep_update_source.py
+16
View File
@@ -130,3 +130,19 @@ async def test_git_url_scoping(db_session: AsyncSession) -> None:
scoped = await svc.list_open_dep_update_tasks(git_url="https://github.com/x/a.git")
assert len(scoped) == 1
assert scoped[0].project_id == proj_a.id
@pytest.mark.asyncio
async def test_git_url_accidentals_scoping(db_session: AsyncSession) -> None:
"""#1267: a dep_update task open on ``.../a.git`` is found when scoping by a
git_url that differs only by a ``.git`` suffix / trailing slash — the dedupe
key is the normalized repo, not the exact string."""
proj_a = await _seed_project(db_session, "https://github.com/x/a.git")
await _make_task(db_session, proj_a)
svc = get_task_service(db_session)
# Same repo, accidental variants — each scope finds the one open task.
for variant in ("https://github.com/x/a", "https://github.com/x/a.git/"):
scoped = await svc.list_open_dep_update_tasks(git_url=variant)
assert len(scoped) == 1
assert scoped[0].project_id == proj_a.id