mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(notifications): CEO lookup tolerates duplicate rows; unbreak slave CI (#620)
#615 merged on a false green: the CI paths filter excludes motion/**, so a motion-only commit fired no quality run and the merge landed two latent breakages on slave. - _get_ceo_agent used scalar_one_or_none on role==CEO, which raises MultipleResultsFound once a second CEO-role row exists. It now pins to the earliest-created CEO, mirroring the sibling _get_auditor_agent. This is what made test_brand_voice_nudge_fires_once fail under the full-suite ordering. - _process_mentions tipped to xenon rank C when the skip-drafting branch was added; extracted the cap check and the skip filter into two small helpers. - ci.yml push paths now include motion/** so a motion-only commit can't false-green the quality gate again. Regression test: two CEO rows no longer break the lookup. Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -42,6 +42,10 @@ on:
|
||||
- 'CLAUDE.md'
|
||||
- 'CHANGELOG.md'
|
||||
- 'docs/**'
|
||||
# `make quality`'s prose gate lints motion/**, so a motion-only commit
|
||||
# can still turn the gate red — without this it fires no run and a PR
|
||||
# merges on a false green.
|
||||
- 'motion/**'
|
||||
pull_request:
|
||||
branches:
|
||||
- master
|
||||
|
||||
@@ -1205,8 +1205,17 @@ class NotificationDeliveryService(BaseService):
|
||||
return result.scalar_one_or_none()
|
||||
|
||||
async def _get_ceo_agent(self) -> AgentTable | None:
|
||||
"""Find the CEO agent (org-wide singleton; earliest-created if many).
|
||||
|
||||
Mirrors `_get_auditor_agent`: a plain one-or-none raises
|
||||
MultipleResultsFound if a second CEO-role row ever exists, so pin to
|
||||
the earliest-created — the canonical seeded CEO — instead.
|
||||
"""
|
||||
result = await self.session.execute(
|
||||
select(AgentTable).where(AgentTable.role == AgentRole.CEO)
|
||||
select(AgentTable)
|
||||
.where(AgentTable.role == AgentRole.CEO)
|
||||
.order_by(AgentTable.created_at)
|
||||
.limit(1)
|
||||
)
|
||||
return result.scalar_one_or_none()
|
||||
|
||||
|
||||
@@ -493,13 +493,9 @@ class XEngine(BaseService):
|
||||
originated: list[TaskTable] = []
|
||||
product_name: str | None = None
|
||||
for mention in mentions:
|
||||
if len(originated) >= settings.x_mentions_max_per_cycle:
|
||||
if self._cycle_cap_reached(len(originated), open_count):
|
||||
break
|
||||
if open_count + len(originated) >= settings.x_max_open_posts:
|
||||
break
|
||||
if not mention.id or await self._already_seen(mention.id):
|
||||
continue
|
||||
if not _is_meaningful(mention, settings.x_mentions_min_engagement):
|
||||
if await self._skip_mention(mention):
|
||||
continue
|
||||
if project is None or project.id is None:
|
||||
self.log.warning(
|
||||
@@ -518,6 +514,22 @@ class XEngine(BaseService):
|
||||
originated.append(reply_task)
|
||||
return originated
|
||||
|
||||
def _cycle_cap_reached(self, originated_count: int, open_count: int) -> bool:
|
||||
"""Stop originating once this cycle's per-run or rolling open-post cap
|
||||
is hit."""
|
||||
return (
|
||||
originated_count >= settings.x_mentions_max_per_cycle
|
||||
or open_count + originated_count >= settings.x_max_open_posts
|
||||
)
|
||||
|
||||
async def _skip_mention(self, mention: XMention) -> bool:
|
||||
"""A mention already handled, or below the engagement floor, is skipped.
|
||||
The floor skip is deliberately not marked seen, so a later viral
|
||||
re-fetch can still draft it."""
|
||||
if not mention.id or await self._already_seen(mention.id):
|
||||
return True
|
||||
return not _is_meaningful(mention, settings.x_mentions_min_engagement)
|
||||
|
||||
async def _since_id_get(self) -> str | None:
|
||||
"""Best-effort read of the persisted mentions cursor; None on miss or
|
||||
Redis failure (a failed read just fetches from the top this cycle)."""
|
||||
|
||||
@@ -208,3 +208,35 @@ async def test_notify_ceo_of_completion_creates_alert(env: dict) -> None:
|
||||
assert env["ceo"].id in note.to_agents
|
||||
assert "Active effort" in note.body
|
||||
assert "Ship it" in note.subject
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_ceo_agent_tolerates_duplicate_ceo_rows(env: dict) -> None:
|
||||
"""A second role=CEO row (a real hazard: sibling tests commit one into the
|
||||
shared session DB, and nothing forbids two in prod) must not make
|
||||
`_get_ceo_agent()` raise MultipleResultsFound — it resolves the
|
||||
earliest-created CEO, mirroring `_get_auditor_agent`."""
|
||||
db = env["db"]
|
||||
later_ceo = AgentTable(
|
||||
id=uuid4(),
|
||||
name="CEO 2",
|
||||
slug=f"ceo-{uuid4().hex[:6]}",
|
||||
role=AgentRole.CEO,
|
||||
team=None,
|
||||
status=AgentStatus.ACTIVE,
|
||||
model_config={},
|
||||
system_prompt="x",
|
||||
capabilities=[],
|
||||
permissions={},
|
||||
metrics={},
|
||||
created_at=datetime.now(UTC) + timedelta(hours=1),
|
||||
)
|
||||
db.add(later_ceo)
|
||||
await db.flush()
|
||||
|
||||
delivery = get_notification_delivery_service(db)
|
||||
resolved = await delivery._get_ceo_agent()
|
||||
|
||||
# Does not raise, and pins to the earliest-created (never the later row).
|
||||
assert resolved is not None
|
||||
assert resolved.id != later_ceo.id
|
||||
|
||||
Reference in New Issue
Block a user