mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(notifications): task titles and agent slugs replace raw UUIDs (#616)
* fix(notifications): task titles and agent slugs replace raw UUIDs
Notification producers interpolated raw task/agent UUIDs into subjects and
bodies ('Task 68e1e4db-... unblocked', 'handed back to 00000000-...-0004').
A tiny notification_text helper (task_display: title-first with a #id8
fallback; agent_display: identity-map slug first, DB lookup fallback) now
feeds every producer: all 13 NotificationService methods, the 7
delivery-service bodies whose subjects were already title-based, the
substitute-PM ad-hoc insert, and the orchestrator/choreographer callers,
which thread the task row's title one call deeper. Fixes the literal
'cell_pm' role string sent as an agent slug in the merge-conflict
notification. Tool-call examples like unblock('<uuid>') keep the raw id on
purpose — agents need it.
* test(notifications): board-review subject assertion matches the humanized format
---------
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -11286,10 +11286,16 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
error=str(exc),
|
error=str(exc),
|
||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
await self._notify_stale_claim_reaped(task_id, reaped_agent, ts)
|
await self._notify_stale_claim_reaped(
|
||||||
|
task_id, reaped_agent, ts, getattr(t, "title", None)
|
||||||
|
)
|
||||||
|
|
||||||
async def _notify_stale_claim_reaped(
|
async def _notify_stale_claim_reaped(
|
||||||
self, task_id: "UUID", reaped_agent: Any, last_heartbeat: datetime | None
|
self,
|
||||||
|
task_id: "UUID",
|
||||||
|
reaped_agent: Any,
|
||||||
|
last_heartbeat: datetime | None,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Best-effort coordination notification for a reaped stale claim.
|
"""Best-effort coordination notification for a reaped stale claim.
|
||||||
|
|
||||||
@@ -11307,6 +11313,7 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
task_id=str(task_id),
|
task_id=str(task_id),
|
||||||
reaped_agent=str(reaped_agent),
|
reaped_agent=str(reaped_agent),
|
||||||
last_heartbeat=last_heartbeat.isoformat() if last_heartbeat else None,
|
last_heartbeat=last_heartbeat.isoformat() if last_heartbeat else None,
|
||||||
|
task_title=task_title,
|
||||||
)
|
)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
@@ -11637,7 +11644,19 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
Best-effort: a notification failure must not wedge dispatch, so any
|
Best-effort: a notification failure must not wedge dispatch, so any
|
||||||
error is logged and swallowed.
|
error is logged and swallowed.
|
||||||
"""
|
"""
|
||||||
|
from uuid import UUID
|
||||||
|
|
||||||
|
from roboco.db.base import get_db_context
|
||||||
from roboco.services.notification import NotificationService
|
from roboco.services.notification import NotificationService
|
||||||
|
from roboco.services.task import TaskService
|
||||||
|
|
||||||
|
task_title: str | None = None
|
||||||
|
try:
|
||||||
|
async with get_db_context() as db:
|
||||||
|
task = await TaskService(db).get(UUID(task_id))
|
||||||
|
task_title = task.title if task else None
|
||||||
|
except Exception:
|
||||||
|
task_title = None
|
||||||
|
|
||||||
try:
|
try:
|
||||||
await NotificationService().send_stuck_agent_notification(
|
await NotificationService().send_stuck_agent_notification(
|
||||||
@@ -11645,6 +11664,7 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
agent_slug=agent_slug,
|
agent_slug=agent_slug,
|
||||||
task_status=task_status or "unknown",
|
task_status=task_status or "unknown",
|
||||||
to_agent="ceo",
|
to_agent="ceo",
|
||||||
|
task_title=task_title,
|
||||||
)
|
)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
@@ -11900,10 +11920,13 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
async with get_db_context() as db:
|
async with get_db_context() as db:
|
||||||
await TaskService(db).mark_board_review_complete(UUID(task_id))
|
task_service = TaskService(db)
|
||||||
|
task = await task_service.get(UUID(task_id))
|
||||||
|
await task_service.mark_board_review_complete(UUID(task_id))
|
||||||
await db.commit()
|
await db.commit()
|
||||||
await NotificationService().send_board_review_complete_notification(
|
await NotificationService().send_board_review_complete_notification(
|
||||||
task_id=task_id,
|
task_id=task_id,
|
||||||
|
task_title=task.title if task else None,
|
||||||
)
|
)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
# Don't wedge dispatch on a failure; allow a retry by clearing the
|
# Don't wedge dispatch on a failure; allow a retry by clearing the
|
||||||
|
|||||||
@@ -6887,7 +6887,7 @@ class Choreographer:
|
|||||||
)
|
)
|
||||||
|
|
||||||
t = await self.task.unblock_with_restore(pm_agent_id, task_id, restore=restore)
|
t = await self.task.unblock_with_restore(pm_agent_id, task_id, restore=restore)
|
||||||
await self._maybe_notify_block_flip(task_id, t)
|
await self._maybe_notify_block_flip(task_id, t, t.title)
|
||||||
next_msg = (
|
next_msg = (
|
||||||
"task restored to its pre-block state — original assignee will resume"
|
"task restored to its pre-block state — original assignee will resume"
|
||||||
if restore
|
if restore
|
||||||
@@ -6900,7 +6900,9 @@ class Choreographer:
|
|||||||
context_briefing=await self._briefing_for(pm_agent_id, task_id),
|
context_briefing=await self._briefing_for(pm_agent_id, task_id),
|
||||||
).with_introspection(task=t, role=role)
|
).with_introspection(task=t, role=role)
|
||||||
|
|
||||||
async def _maybe_notify_block_flip(self, task_id: UUID, t: Any) -> None:
|
async def _maybe_notify_block_flip(
|
||||||
|
self, task_id: UUID, t: Any, task_title: str | None = None
|
||||||
|
) -> None:
|
||||||
"""Bump the flip counter; alert the CEO once past the threshold.
|
"""Bump the flip counter; alert the CEO once past the threshold.
|
||||||
|
|
||||||
A resolver that keeps unblocking a task that keeps re-blocking
|
A resolver that keeps unblocking a task that keeps re-blocking
|
||||||
@@ -6915,16 +6917,18 @@ class Choreographer:
|
|||||||
and not markers.is_block_flip_notified(t)
|
and not markers.is_block_flip_notified(t)
|
||||||
):
|
):
|
||||||
markers.mark_block_flip_notified(t)
|
markers.mark_block_flip_notified(t)
|
||||||
await self._notify_ceo_block_flip(task_id, flip_count)
|
await self._notify_ceo_block_flip(task_id, flip_count, task_title)
|
||||||
|
|
||||||
async def _notify_ceo_block_flip(self, task_id: UUID, flip_count: int) -> None:
|
async def _notify_ceo_block_flip(
|
||||||
|
self, task_id: UUID, flip_count: int, task_title: str | None = None
|
||||||
|
) -> None:
|
||||||
"""Best-effort CEO alert for a repeating block/unblock cycle; never
|
"""Best-effort CEO alert for a repeating block/unblock cycle; never
|
||||||
raises — the breaker signals, it does not wedge unblock itself."""
|
raises — the breaker signals, it does not wedge unblock itself."""
|
||||||
from roboco.services.notification import NotificationService
|
from roboco.services.notification import NotificationService
|
||||||
|
|
||||||
try:
|
try:
|
||||||
await NotificationService().send_block_flip_notification(
|
await NotificationService().send_block_flip_notification(
|
||||||
task_id=str(task_id), flip_count=flip_count
|
task_id=str(task_id), flip_count=flip_count, task_title=task_title
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
@@ -7284,7 +7288,7 @@ class Choreographer:
|
|||||||
actor_id=pm_agent_id,
|
actor_id=pm_agent_id,
|
||||||
actor_role="cell_pm",
|
actor_role="cell_pm",
|
||||||
)
|
)
|
||||||
await self._notify_ceo_merge_conflict(task_id, files)
|
await self._notify_ceo_merge_conflict(task_id, files, pm_agent_id)
|
||||||
t = await self.task.get(task_id)
|
t = await self.task.get(task_id)
|
||||||
detail = f" ({len(files)} conflicting file(s))" if files else ""
|
detail = f" ({len(files)} conflicting file(s))" if files else ""
|
||||||
return Envelope.ok(
|
return Envelope.ok(
|
||||||
@@ -7298,14 +7302,18 @@ class Choreographer:
|
|||||||
context_briefing=await self._briefing_for(pm_agent_id, task_id),
|
context_briefing=await self._briefing_for(pm_agent_id, task_id),
|
||||||
).with_introspection(task=t, role="cell_pm")
|
).with_introspection(task=t, role="cell_pm")
|
||||||
|
|
||||||
async def _notify_ceo_merge_conflict(self, task_id: UUID, files: list[str]) -> None:
|
async def _notify_ceo_merge_conflict(
|
||||||
|
self, task_id: UUID, files: list[str], pm_agent_id: UUID
|
||||||
|
) -> None:
|
||||||
"""Best-effort CEO alert for a wedged merge conflict; never raises."""
|
"""Best-effort CEO alert for a wedged merge conflict; never raises."""
|
||||||
from roboco.services.notification import NotificationService
|
from roboco.services.notification import NotificationService
|
||||||
|
from roboco.services.notification_text import agent_display
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
pm_slug = await agent_display(pm_agent_id, self.task.session)
|
||||||
await NotificationService().send_stuck_agent_notification(
|
await NotificationService().send_stuck_agent_notification(
|
||||||
task_id=str(task_id),
|
task_id=str(task_id),
|
||||||
agent_slug="cell_pm",
|
agent_slug=pm_slug or str(pm_agent_id),
|
||||||
task_status="awaiting_ceo_approval",
|
task_status="awaiting_ceo_approval",
|
||||||
to_agent="ceo",
|
to_agent="ceo",
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -210,6 +210,7 @@ class PRReviewerMixin(_Base):
|
|||||||
task_id=str(task_id),
|
task_id=str(task_id),
|
||||||
pr_number=pr_number,
|
pr_number=pr_number,
|
||||||
pr_url=str(getattr(t, "pr_url", "") or ""),
|
pr_url=str(getattr(t, "pr_url", "") or ""),
|
||||||
|
task_title=getattr(t, "title", None),
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.exception(
|
logger.exception(
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ from roboco.foundation.policy.communications import ACK_REQUIRED_BY_TYPE
|
|||||||
from roboco.models import NotificationPriority, NotificationType
|
from roboco.models import NotificationPriority, NotificationType
|
||||||
from roboco.models.notification import CreateNotificationParams
|
from roboco.models.notification import CreateNotificationParams
|
||||||
from roboco.services.notification_dedup import all_recipients_recently_notified
|
from roboco.services.notification_dedup import all_recipients_recently_notified
|
||||||
|
from roboco.services.notification_text import agent_display, task_display
|
||||||
from roboco.utils.converters import require_uuid
|
from roboco.utils.converters import require_uuid
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
@@ -65,6 +66,7 @@ class NotificationService:
|
|||||||
blocker_reason: str,
|
blocker_reason: str,
|
||||||
from_agent: str | None,
|
from_agent: str | None,
|
||||||
to_pm: str,
|
to_pm: str,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Send notification about a blocked task."""
|
"""Send notification about a blocked task."""
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -72,10 +74,11 @@ class NotificationService:
|
|||||||
task_id=task_id,
|
task_id=task_id,
|
||||||
to_pm=to_pm,
|
to_pm=to_pm,
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
|
|
||||||
# System notifications bypass normal permission checks
|
# System notifications bypass normal permission checks
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} has been blocked.\n\n"
|
f"Task {display} has been blocked.\n\n"
|
||||||
f"Reason: {blocker_reason}\n\n"
|
f"Reason: {blocker_reason}\n\n"
|
||||||
"Please investigate and help resolve."
|
"Please investigate and help resolve."
|
||||||
)
|
)
|
||||||
@@ -85,7 +88,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.HIGH,
|
priority=NotificationPriority.HIGH,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=[to_pm],
|
to_agents=[to_pm],
|
||||||
subject=f"Task {task_id} is blocked",
|
subject=f"Task {display} is blocked",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -97,6 +100,7 @@ class NotificationService:
|
|||||||
agent_slug: str,
|
agent_slug: str,
|
||||||
task_status: str,
|
task_status: str,
|
||||||
to_agent: str,
|
to_agent: str,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Alert an overseer that an agent is wedged in an unproductive loop.
|
"""Alert an overseer that an agent is wedged in an unproductive loop.
|
||||||
|
|
||||||
@@ -110,8 +114,9 @@ class NotificationService:
|
|||||||
agent=agent_slug,
|
agent=agent_slug,
|
||||||
to_agent=to_agent,
|
to_agent=to_agent,
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Agent {agent_slug} was repeatedly spawned on task {task_id} "
|
f"Agent {agent_slug} was repeatedly spawned on task {display} "
|
||||||
f"(status: {task_status}) without advancing it, so further automatic "
|
f"(status: {task_status}) without advancing it, so further automatic "
|
||||||
"spawns have been paused. Please investigate and intervene manually."
|
"spawns have been paused. Please investigate and intervene manually."
|
||||||
)
|
)
|
||||||
@@ -121,7 +126,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.HIGH,
|
priority=NotificationPriority.HIGH,
|
||||||
from_agent="system",
|
from_agent="system",
|
||||||
to_agents=[to_agent],
|
to_agents=[to_agent],
|
||||||
subject=f"Agent {agent_slug} stuck on task {task_id}",
|
subject=f"Agent {agent_slug} stuck on task {display}",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -132,6 +137,7 @@ class NotificationService:
|
|||||||
task_id: str,
|
task_id: str,
|
||||||
flip_count: int,
|
flip_count: int,
|
||||||
to_agent: str = "ceo",
|
to_agent: str = "ceo",
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Alert an overseer that a task keeps flip-flopping block/unblock.
|
"""Alert an overseer that a task keeps flip-flopping block/unblock.
|
||||||
|
|
||||||
@@ -144,8 +150,9 @@ class NotificationService:
|
|||||||
logger.info(
|
logger.info(
|
||||||
"Sending block-flip notification", task_id=task_id, flip_count=flip_count
|
"Sending block-flip notification", task_id=task_id, flip_count=flip_count
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} has been blocked and unblocked {flip_count} times. "
|
f"Task {display} has been blocked and unblocked {flip_count} times. "
|
||||||
"This flip-flop usually means a structural wedge rather than a "
|
"This flip-flop usually means a structural wedge rather than a "
|
||||||
"one-off block — please investigate."
|
"one-off block — please investigate."
|
||||||
)
|
)
|
||||||
@@ -155,7 +162,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.HIGH,
|
priority=NotificationPriority.HIGH,
|
||||||
from_agent="system",
|
from_agent="system",
|
||||||
to_agents=[to_agent],
|
to_agents=[to_agent],
|
||||||
subject=f"Task {task_id} flip-flopping block/unblock ({flip_count}x)",
|
subject=f"Task {display} flip-flopping block/unblock ({flip_count}x)",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -166,6 +173,7 @@ class NotificationService:
|
|||||||
task_id: str,
|
task_id: str,
|
||||||
from_agent: str | None,
|
from_agent: str | None,
|
||||||
to_qa: str,
|
to_qa: str,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Send notification that task is ready for QA."""
|
"""Send notification that task is ready for QA."""
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -174,8 +182,9 @@ class NotificationService:
|
|||||||
to_qa=to_qa,
|
to_qa=to_qa,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} is ready for QA review.\n\n"
|
f"Task {display} is ready for QA review.\n\n"
|
||||||
"Please review and provide feedback."
|
"Please review and provide feedback."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
@@ -184,7 +193,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.NORMAL,
|
priority=NotificationPriority.NORMAL,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=[to_qa],
|
to_agents=[to_qa],
|
||||||
subject=f"Task {task_id} ready for QA",
|
subject=f"Task {display} ready for QA",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -195,6 +204,7 @@ class NotificationService:
|
|||||||
task_id: str,
|
task_id: str,
|
||||||
from_agent: str | None,
|
from_agent: str | None,
|
||||||
to_documenter: str,
|
to_documenter: str,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Send notification that task is ready for documentation."""
|
"""Send notification that task is ready for documentation."""
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -203,8 +213,9 @@ class NotificationService:
|
|||||||
to_documenter=to_documenter,
|
to_documenter=to_documenter,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} has passed QA and is ready for documentation.\n\n"
|
f"Task {display} has passed QA and is ready for documentation.\n\n"
|
||||||
"Please create the required documentation."
|
"Please create the required documentation."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
@@ -213,7 +224,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.NORMAL,
|
priority=NotificationPriority.NORMAL,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=[to_documenter],
|
to_agents=[to_documenter],
|
||||||
subject=f"Task {task_id} needs documentation",
|
subject=f"Task {display} needs documentation",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -225,6 +236,7 @@ class NotificationService:
|
|||||||
handoff_id: str,
|
handoff_id: str,
|
||||||
from_agent: str | None,
|
from_agent: str | None,
|
||||||
to_documenter: str,
|
to_documenter: str,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Send notification that task needs handoff documentation."""
|
"""Send notification that task needs handoff documentation."""
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -234,8 +246,9 @@ class NotificationService:
|
|||||||
to_documenter=to_documenter,
|
to_documenter=to_documenter,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} is ready for handoff (ID: {handoff_id}).\n\n"
|
f"Task {display} is ready for handoff (ID: {handoff_id}).\n\n"
|
||||||
"Please review and create handoff documentation."
|
"Please review and create handoff documentation."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
@@ -244,7 +257,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.NORMAL,
|
priority=NotificationPriority.NORMAL,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=[to_documenter],
|
to_agents=[to_documenter],
|
||||||
subject=f"Handoff required: Task {task_id}",
|
subject=f"Handoff required: Task {display}",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -255,6 +268,7 @@ class NotificationService:
|
|||||||
task_id: str,
|
task_id: str,
|
||||||
qa_notes: str,
|
qa_notes: str,
|
||||||
to_developer: str,
|
to_developer: str,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Send notification that task failed QA."""
|
"""Send notification that task failed QA."""
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -263,8 +277,9 @@ class NotificationService:
|
|||||||
to_developer=to_developer,
|
to_developer=to_developer,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} has failed QA review.\n\n"
|
f"Task {display} has failed QA review.\n\n"
|
||||||
f"Notes: {qa_notes}\n\n"
|
f"Notes: {qa_notes}\n\n"
|
||||||
"Please address the issues and resubmit."
|
"Please address the issues and resubmit."
|
||||||
)
|
)
|
||||||
@@ -274,7 +289,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.HIGH,
|
priority=NotificationPriority.HIGH,
|
||||||
from_agent="system",
|
from_agent="system",
|
||||||
to_agents=[to_developer],
|
to_agents=[to_developer],
|
||||||
subject=f"QA Failed: Task {task_id}",
|
subject=f"QA Failed: Task {display}",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -285,6 +300,7 @@ class NotificationService:
|
|||||||
task_id: str,
|
task_id: str,
|
||||||
from_agent: str | None = None,
|
from_agent: str | None = None,
|
||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the CEO a board review is complete and ready for Approve & Start.
|
"""Tell the CEO a board review is complete and ready for Approve & Start.
|
||||||
|
|
||||||
@@ -305,8 +321,9 @@ class NotificationService:
|
|||||||
to_ceo=to_ceo,
|
to_ceo=to_ceo,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Board review complete for task {task_id}.\n\n"
|
f"Board review complete for task {display}.\n\n"
|
||||||
"The Product Owner and Head of Marketing have both reviewed and "
|
"The Product Owner and Head of Marketing have both reviewed and "
|
||||||
"recorded their requirements. The task is ready for your "
|
"recorded their requirements. The task is ready for your "
|
||||||
"Approve & Start decision (hand to Main PM) or rejection."
|
"Approve & Start decision (hand to Main PM) or rejection."
|
||||||
@@ -317,7 +334,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.HIGH,
|
priority=NotificationPriority.HIGH,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=[to_ceo],
|
to_agents=[to_ceo],
|
||||||
subject=f"Board review complete: Task {task_id}",
|
subject=f"Board review complete: Task {display}",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
@@ -357,6 +374,7 @@ class NotificationService:
|
|||||||
pr_url: str,
|
pr_url: str,
|
||||||
from_agent: str | None = None,
|
from_agent: str | None = None,
|
||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the CEO an inbound external PR has been reviewed — their call.
|
"""Tell the CEO an inbound external PR has been reviewed — their call.
|
||||||
|
|
||||||
@@ -374,10 +392,12 @@ class NotificationService:
|
|||||||
pr_number=pr_number,
|
pr_number=pr_number,
|
||||||
to_ceo=to_ceo,
|
to_ceo=to_ceo,
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
body = (
|
body = (
|
||||||
f"External PR #{pr_number} has been reviewed and a change-request "
|
f"External PR #{pr_number} on {display} has been reviewed and a "
|
||||||
f"posted ({pr_url}).\n\nYour call: supersede it (the org takes the "
|
f"change-request posted ({pr_url}).\n\nYour call: supersede it (the "
|
||||||
"contribution over and finishes it to our standards) or dismiss it."
|
"org takes the contribution over and finishes it to our standards) "
|
||||||
|
"or dismiss it."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
CreateNotificationParams(
|
CreateNotificationParams(
|
||||||
@@ -399,6 +419,7 @@ class NotificationService:
|
|||||||
from_agent: str | None = None,
|
from_agent: str | None = None,
|
||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
db_session: AsyncSession | None = None,
|
db_session: AsyncSession | None = None,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the outgoing + incoming owner (and the CEO) a task moved.
|
"""Tell the outgoing + incoming owner (and the CEO) a task moved.
|
||||||
|
|
||||||
@@ -417,9 +438,12 @@ class NotificationService:
|
|||||||
previous_assignee=previous_assignee,
|
previous_assignee=previous_assignee,
|
||||||
new_assignee=new_assignee,
|
new_assignee=new_assignee,
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
|
previous_label = await agent_display(previous_assignee, db_session)
|
||||||
|
new_label = await agent_display(new_assignee, db_session)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} was reassigned from "
|
f"Task {display} was reassigned from "
|
||||||
f"{previous_assignee or 'unassigned'} to {new_assignee or 'unassigned'}."
|
f"{previous_label or 'unassigned'} to {new_label or 'unassigned'}."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
CreateNotificationParams(
|
CreateNotificationParams(
|
||||||
@@ -427,7 +451,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.NORMAL,
|
priority=NotificationPriority.NORMAL,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=recipients,
|
to_agents=recipients,
|
||||||
subject=f"Task {task_id} reassigned",
|
subject=f"Task {display} reassigned",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
),
|
),
|
||||||
@@ -441,6 +465,8 @@ class NotificationService:
|
|||||||
held_back_assignee: str | None,
|
held_back_assignee: str | None,
|
||||||
from_agent: str | None = None,
|
from_agent: str | None = None,
|
||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
|
held_back_title: str | None = None,
|
||||||
|
blocking_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the held-back task's owner (+ CEO) it now waits on a sibling.
|
"""Tell the held-back task's owner (+ CEO) it now waits on a sibling.
|
||||||
|
|
||||||
@@ -456,9 +482,11 @@ class NotificationService:
|
|||||||
held_back_task_id=held_back_task_id,
|
held_back_task_id=held_back_task_id,
|
||||||
blocking_task_id=blocking_task_id,
|
blocking_task_id=blocking_task_id,
|
||||||
)
|
)
|
||||||
|
held_back_display = task_display(held_back_title, held_back_task_id)
|
||||||
|
blocking_display = task_display(blocking_title, blocking_task_id)
|
||||||
body = (
|
body = (
|
||||||
f"Task {held_back_task_id} was held back by the collision-sequencing "
|
f"Task {held_back_display} was held back by the collision-sequencing "
|
||||||
f"analyzer: it now depends on task {blocking_task_id}, which surfaced "
|
f"analyzer: it now depends on task {blocking_display}, which surfaced "
|
||||||
"an overlapping file/migration/shared-surface collision. It will "
|
"an overlapping file/migration/shared-surface collision. It will "
|
||||||
"resume once that task reaches a terminal state."
|
"resume once that task reaches a terminal state."
|
||||||
)
|
)
|
||||||
@@ -468,7 +496,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.NORMAL,
|
priority=NotificationPriority.NORMAL,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=recipients,
|
to_agents=recipients,
|
||||||
subject=f"Task {held_back_task_id} sequenced behind a sibling",
|
subject=f"Task {held_back_display} sequenced behind a sibling",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=held_back_task_id,
|
related_task_id=held_back_task_id,
|
||||||
)
|
)
|
||||||
@@ -481,6 +509,7 @@ class NotificationService:
|
|||||||
from_agent: str | None = None,
|
from_agent: str | None = None,
|
||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
db_session: AsyncSession | None = None,
|
db_session: AsyncSession | None = None,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the restored owner (+ CEO) a blocked task is workable again.
|
"""Tell the restored owner (+ CEO) a blocked task is workable again.
|
||||||
|
|
||||||
@@ -497,9 +526,11 @@ class NotificationService:
|
|||||||
task_id=task_id,
|
task_id=task_id,
|
||||||
restored_owner=restored_owner,
|
restored_owner=restored_owner,
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
|
owner_label = await agent_display(restored_owner, db_session)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} has been unblocked and handed back to "
|
f"Task {display} has been unblocked and handed back to "
|
||||||
f"{restored_owner or 'its owner'}. It is ready to resume."
|
f"{owner_label or 'its owner'}. It is ready to resume."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
CreateNotificationParams(
|
CreateNotificationParams(
|
||||||
@@ -507,7 +538,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.NORMAL,
|
priority=NotificationPriority.NORMAL,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=recipients,
|
to_agents=recipients,
|
||||||
subject=f"Task {task_id} unblocked",
|
subject=f"Task {display} unblocked",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
),
|
),
|
||||||
@@ -522,6 +553,8 @@ class NotificationService:
|
|||||||
from_agent: str | None = None,
|
from_agent: str | None = None,
|
||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
db_session: AsyncSession | None = None,
|
db_session: AsyncSession | None = None,
|
||||||
|
task_title: str | None = None,
|
||||||
|
completed_dependency_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the revived task's owner (+ CEO) its last dependency landed.
|
"""Tell the revived task's owner (+ CEO) its last dependency landed.
|
||||||
|
|
||||||
@@ -544,9 +577,13 @@ class NotificationService:
|
|||||||
task_id=task_id,
|
task_id=task_id,
|
||||||
completed_dependency_id=completed_dependency_id,
|
completed_dependency_id=completed_dependency_id,
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
|
dependency_display = task_display(
|
||||||
|
completed_dependency_title, completed_dependency_id
|
||||||
|
)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id} was revived: its dependency "
|
f"Task {display} was revived: its dependency "
|
||||||
f"{completed_dependency_id} just completed and no other "
|
f"{dependency_display} just completed and no other "
|
||||||
"dependencies remain. It is ready to resume."
|
"dependencies remain. It is ready to resume."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
@@ -555,7 +592,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.NORMAL,
|
priority=NotificationPriority.NORMAL,
|
||||||
from_agent=from_agent or "system",
|
from_agent=from_agent or "system",
|
||||||
to_agents=recipients,
|
to_agents=recipients,
|
||||||
subject=f"Task {task_id} revived by dependency completion",
|
subject=f"Task {display} revived by dependency completion",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
),
|
),
|
||||||
@@ -569,6 +606,7 @@ class NotificationService:
|
|||||||
last_heartbeat: str | None = None,
|
last_heartbeat: str | None = None,
|
||||||
from_agent: str = "system",
|
from_agent: str = "system",
|
||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the reaped agent (+ CEO) its stale claim was released.
|
"""Tell the reaped agent (+ CEO) its stale claim was released.
|
||||||
|
|
||||||
@@ -586,10 +624,12 @@ class NotificationService:
|
|||||||
task_id=task_id,
|
task_id=task_id,
|
||||||
reaped_agent=reaped_agent,
|
reaped_agent=reaped_agent,
|
||||||
)
|
)
|
||||||
|
display = task_display(task_title, task_id)
|
||||||
|
agent_label = await agent_display(reaped_agent)
|
||||||
body = (
|
body = (
|
||||||
f"Task {task_id}'s claim went stale "
|
f"Task {display}'s claim went stale "
|
||||||
f"(last heartbeat: {last_heartbeat or 'unknown'}) and was reaped "
|
f"(last heartbeat: {last_heartbeat or 'unknown'}) and was reaped "
|
||||||
f"back to pending, releasing it from {reaped_agent or 'its holder'}."
|
f"back to pending, releasing it from {agent_label or 'its holder'}."
|
||||||
)
|
)
|
||||||
await self._create_notification(
|
await self._create_notification(
|
||||||
CreateNotificationParams(
|
CreateNotificationParams(
|
||||||
@@ -597,7 +637,7 @@ class NotificationService:
|
|||||||
priority=NotificationPriority.HIGH,
|
priority=NotificationPriority.HIGH,
|
||||||
from_agent=from_agent,
|
from_agent=from_agent,
|
||||||
to_agents=recipients,
|
to_agents=recipients,
|
||||||
subject=f"Task {task_id}: stale claim reaped",
|
subject=f"Task {display}: stale claim reaped",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ from roboco.services.notification_dedup import (
|
|||||||
all_recipients_recently_notified,
|
all_recipients_recently_notified,
|
||||||
clear_dedup_key,
|
clear_dedup_key,
|
||||||
)
|
)
|
||||||
|
from roboco.services.notification_text import task_display
|
||||||
from roboco.utils.converters import require_uuid
|
from roboco.utils.converters import require_uuid
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
@@ -753,7 +754,8 @@ class NotificationDeliveryService(BaseService):
|
|||||||
to_agents=[pm.id],
|
to_agents=[pm.id],
|
||||||
subject=f"ACTION REQUIRED: Blocked - {task_title[:40]}",
|
subject=f"ACTION REQUIRED: Blocked - {task_title[:40]}",
|
||||||
body=(
|
body=(
|
||||||
f"Task {task_id} has been BLOCKED by {blocker_name}.\n\n"
|
f"Task {task_display(task_title, task_id)} has been BLOCKED by "
|
||||||
|
f"{blocker_name}.\n\n"
|
||||||
f"Type: {details.blocker_type}\n"
|
f"Type: {details.blocker_type}\n"
|
||||||
f"Reason: {details.reason}\n"
|
f"Reason: {details.reason}\n"
|
||||||
f"What's needed: {details.what_needed}\n\n"
|
f"What's needed: {details.what_needed}\n\n"
|
||||||
@@ -790,8 +792,8 @@ class NotificationDeliveryService(BaseService):
|
|||||||
to_agents=[pm.id],
|
to_agents=[pm.id],
|
||||||
subject=f"Documentation complete: {task.title or 'Unknown task'}",
|
subject=f"Documentation complete: {task.title or 'Unknown task'}",
|
||||||
body=(
|
body=(
|
||||||
f"Task {task_id} documentation is complete and ready "
|
f"Task {task_display(task, task_id)} documentation is complete "
|
||||||
"for final review.\n\nPlease review and complete the task."
|
"and ready for final review.\n\nPlease review and complete the task."
|
||||||
),
|
),
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
requires_ack=ACK_REQUIRED_BY_TYPE[NotificationType.TASK_ASSIGNMENT],
|
requires_ack=ACK_REQUIRED_BY_TYPE[NotificationType.TASK_ASSIGNMENT],
|
||||||
@@ -819,8 +821,8 @@ class NotificationDeliveryService(BaseService):
|
|||||||
to_agents=[pm.id],
|
to_agents=[pm.id],
|
||||||
subject=f"Task ready for review: {task.title or 'Unknown task'}",
|
subject=f"Task ready for review: {task.title or 'Unknown task'}",
|
||||||
body=(
|
body=(
|
||||||
f"Task {task_id} has been submitted for PM review.\n\n"
|
f"Task {task_display(task, task_id)} has been submitted for PM "
|
||||||
f"Notes: {notes or 'None'}\n\n"
|
f"review.\n\nNotes: {notes or 'None'}\n\n"
|
||||||
"Please review and complete the task."
|
"Please review and complete the task."
|
||||||
),
|
),
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
@@ -845,8 +847,8 @@ class NotificationDeliveryService(BaseService):
|
|||||||
to_agents=[assignee_agent_id],
|
to_agents=[assignee_agent_id],
|
||||||
subject=f"CEO Revision Required: {task.title or 'Unknown task'}",
|
subject=f"CEO Revision Required: {task.title or 'Unknown task'}",
|
||||||
body=(
|
body=(
|
||||||
f"Task {task_id} was rejected by CEO and requires revision.\n\n"
|
f"Task {task_display(task, task_id)} was rejected by CEO and "
|
||||||
f"Reason: {notes}\n\n"
|
f"requires revision.\n\nReason: {notes}\n\n"
|
||||||
"Please address the feedback and resubmit."
|
"Please address the feedback and resubmit."
|
||||||
),
|
),
|
||||||
related_task_id=task_id,
|
related_task_id=task_id,
|
||||||
@@ -888,7 +890,10 @@ class NotificationDeliveryService(BaseService):
|
|||||||
if not target:
|
if not target:
|
||||||
raise EscalationError(f"Escalation target not found: {default_target}")
|
raise EscalationError(f"Escalation target not found: {default_target}")
|
||||||
|
|
||||||
body = f"Task {task_id} escalated by {escalator.slug}.\n\nReason: {reason}"
|
body = (
|
||||||
|
f"Task {task_display(task, task_id)} escalated by "
|
||||||
|
f"{escalator.slug}.\n\nReason: {reason}"
|
||||||
|
)
|
||||||
notification = NotificationTable(
|
notification = NotificationTable(
|
||||||
type=NotificationType.BLOCKER_ESCALATION,
|
type=NotificationType.BLOCKER_ESCALATION,
|
||||||
priority=NotificationPriority.HIGH,
|
priority=NotificationPriority.HIGH,
|
||||||
@@ -1041,8 +1046,8 @@ class NotificationDeliveryService(BaseService):
|
|||||||
to_agents=[ceo.id],
|
to_agents=[ceo.id],
|
||||||
subject=f"CEO Approval Required: {task.title or 'Unknown task'}",
|
subject=f"CEO Approval Required: {task.title or 'Unknown task'}",
|
||||||
body=(
|
body=(
|
||||||
f"Task {task_id} requires CEO approval for completion.\n\n"
|
f"Task {task_display(task, task_id)} requires CEO approval for "
|
||||||
f"Escalated by: {escalator_role}\n"
|
f"completion.\n\nEscalated by: {escalator_role}\n"
|
||||||
f"Notes: {notes or 'None'}\n\n"
|
f"Notes: {notes or 'None'}\n\n"
|
||||||
"Use /ceo-approve or /ceo-reject to respond."
|
"Use /ceo-approve or /ceo-reject to respond."
|
||||||
),
|
),
|
||||||
@@ -1110,7 +1115,7 @@ class NotificationDeliveryService(BaseService):
|
|||||||
role_label = actor_role or (actor.role if actor else "system")
|
role_label = actor_role or (actor.role if actor else "system")
|
||||||
|
|
||||||
body_lines = [
|
body_lines = [
|
||||||
f"Task {task_id} ({title}) entered needs_revision.",
|
f"Task {task_display(title, task_id)} entered needs_revision.",
|
||||||
"",
|
"",
|
||||||
f"Reason: {reason}",
|
f"Reason: {reason}",
|
||||||
f"Actor role: {role_label}",
|
f"Actor role: {role_label}",
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
"""Human-readable formatting for notification subjects/bodies.
|
||||||
|
|
||||||
|
Producers used to interpolate raw task UUIDs and agent UUIDs/role-literals
|
||||||
|
straight into text a human reads (panel list, bell dropdown, Telegram DM —
|
||||||
|
``notification_delivery.py`` reuses ``subject`` verbatim for Telegram). These
|
||||||
|
two helpers give every producer one place to render a task as its title
|
||||||
|
(falling back to a short id) and an agent as its slug (falling back to the
|
||||||
|
raw value), instead of re-deriving it ad hoc per call site.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from typing import TYPE_CHECKING, Any
|
||||||
|
from uuid import UUID
|
||||||
|
|
||||||
|
from roboco.foundation.identity import AGENTS
|
||||||
|
|
||||||
|
if TYPE_CHECKING:
|
||||||
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
|
_TITLE_MAX = 40
|
||||||
|
# Built once — the fixed 26-agent roster never changes at runtime.
|
||||||
|
_UUID_TO_SLUG: dict[str, str] = {str(row.uuid): slug for slug, row in AGENTS.items()}
|
||||||
|
|
||||||
|
|
||||||
|
def task_display(task: Any | None, task_id: str | UUID) -> str:
|
||||||
|
""" "'<title>' (#<id8>)" when a title is available, else "#<id8>".
|
||||||
|
|
||||||
|
``task`` may be a task row (``.title`` attribute), a bare title string,
|
||||||
|
or ``None`` — every call site already holds one of those, so this never
|
||||||
|
triggers a fresh DB fetch just to render text.
|
||||||
|
"""
|
||||||
|
id8 = str(task_id)[:8]
|
||||||
|
title = task if isinstance(task, str) else getattr(task, "title", None)
|
||||||
|
return f"'{title[:_TITLE_MAX]}' (#{id8})" if title else f"#{id8}"
|
||||||
|
|
||||||
|
|
||||||
|
async def agent_display(
|
||||||
|
value: str | UUID | None, db: AsyncSession | None = None
|
||||||
|
) -> str | None:
|
||||||
|
"""Slug for an agent UUID/slug, or the raw value if unresolvable.
|
||||||
|
|
||||||
|
``None`` passes through unchanged — callers keep their own "unassigned"
|
||||||
|
/ "its owner" wording. The static reverse map (built above) is checked
|
||||||
|
first with no I/O at all; a DB lookup only fires when a session is
|
||||||
|
supplied AND the value parses as a UUID the static map doesn't have
|
||||||
|
(e.g. a freshly-seeded row), failing open to the raw string on a miss.
|
||||||
|
"""
|
||||||
|
if value is None:
|
||||||
|
return None
|
||||||
|
key = str(value)
|
||||||
|
slug = _UUID_TO_SLUG.get(key)
|
||||||
|
if slug:
|
||||||
|
return slug
|
||||||
|
if db is not None:
|
||||||
|
try:
|
||||||
|
resolved_uuid = UUID(key)
|
||||||
|
except ValueError:
|
||||||
|
return key
|
||||||
|
from roboco.services.repositories.query_helpers import get_agent_slug
|
||||||
|
|
||||||
|
found = await get_agent_slug(db, resolved_uuid)
|
||||||
|
if found:
|
||||||
|
return found
|
||||||
|
return key
|
||||||
+34
-8
@@ -4882,11 +4882,13 @@ class TaskService(BaseService):
|
|||||||
self._background_tasks.add(bg_task)
|
self._background_tasks.add(bg_task)
|
||||||
bg_task.add_done_callback(self._background_tasks.discard)
|
bg_task.add_done_callback(self._background_tasks.discard)
|
||||||
|
|
||||||
await self._notify_unblock(task_id, task.assigned_to)
|
await self._notify_unblock(task_id, task.assigned_to, task.title)
|
||||||
|
|
||||||
return task
|
return task
|
||||||
|
|
||||||
async def _notify_unblock(self, task_id: UUID, restored_owner: Any) -> None:
|
async def _notify_unblock(
|
||||||
|
self, task_id: UUID, restored_owner: Any, task_title: str | None = None
|
||||||
|
) -> None:
|
||||||
"""Best-effort coordination notification when a blocked task resumes.
|
"""Best-effort coordination notification when a blocked task resumes.
|
||||||
|
|
||||||
Called from both `unblock` and `unblock_with_restore`'s restore=True
|
Called from both `unblock` and `unblock_with_restore`'s restore=True
|
||||||
@@ -4903,6 +4905,7 @@ class TaskService(BaseService):
|
|||||||
task_id=str(task_id),
|
task_id=str(task_id),
|
||||||
restored_owner=str(restored_owner),
|
restored_owner=str(restored_owner),
|
||||||
db_session=self.session,
|
db_session=self.session,
|
||||||
|
task_title=task_title,
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
self.log.warning(
|
self.log.warning(
|
||||||
@@ -7253,6 +7256,7 @@ class TaskService(BaseService):
|
|||||||
assignee=str(owner),
|
assignee=str(owner),
|
||||||
completed_dependency_id=str(completed_dependency_id),
|
completed_dependency_id=str(completed_dependency_id),
|
||||||
db_session=self.session,
|
db_session=self.session,
|
||||||
|
task_title=task.title,
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
self.log.warning(
|
self.log.warning(
|
||||||
@@ -7879,13 +7883,23 @@ class TaskService(BaseService):
|
|||||||
created = await self.add_dependency(held_back_id, blocking_id)
|
created = await self.add_dependency(held_back_id, blocking_id)
|
||||||
if created:
|
if created:
|
||||||
held_back = siblings_by_id.get(held_back_id)
|
held_back = siblings_by_id.get(held_back_id)
|
||||||
|
blocking = siblings_by_id.get(blocking_id)
|
||||||
owner = getattr(held_back, "assigned_to", None) if held_back else None
|
owner = getattr(held_back, "assigned_to", None) if held_back else None
|
||||||
await self._notify_collision_sequencing(
|
await self._notify_collision_sequencing(
|
||||||
held_back_id, blocking_id, owner
|
held_back_id,
|
||||||
|
blocking_id,
|
||||||
|
owner,
|
||||||
|
getattr(held_back, "title", None),
|
||||||
|
getattr(blocking, "title", None),
|
||||||
)
|
)
|
||||||
|
|
||||||
async def _notify_collision_sequencing(
|
async def _notify_collision_sequencing(
|
||||||
self, held_back_task_id: UUID, blocking_task_id: UUID, owner: Any
|
self,
|
||||||
|
held_back_task_id: UUID,
|
||||||
|
blocking_task_id: UUID,
|
||||||
|
owner: Any,
|
||||||
|
held_back_title: str | None = None,
|
||||||
|
blocking_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Best-effort coordination notification for a newly-wired collision edge."""
|
"""Best-effort coordination notification for a newly-wired collision edge."""
|
||||||
try:
|
try:
|
||||||
@@ -7895,6 +7909,8 @@ class TaskService(BaseService):
|
|||||||
held_back_task_id=str(held_back_task_id),
|
held_back_task_id=str(held_back_task_id),
|
||||||
blocking_task_id=str(blocking_task_id),
|
blocking_task_id=str(blocking_task_id),
|
||||||
held_back_assignee=str(owner) if owner is not None else None,
|
held_back_assignee=str(owner) if owner is not None else None,
|
||||||
|
held_back_title=held_back_title,
|
||||||
|
blocking_title=blocking_title,
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
self.log.warning("Collision-sequencing notify failed", error=str(e))
|
self.log.warning("Collision-sequencing notify failed", error=str(e))
|
||||||
@@ -8733,6 +8749,8 @@ class TaskService(BaseService):
|
|||||||
raise ServiceError("Update failed")
|
raise ServiceError("Update failed")
|
||||||
|
|
||||||
if new_status == TaskStatus.AWAITING_PM_REVIEW and target_pm_slug:
|
if new_status == TaskStatus.AWAITING_PM_REVIEW and target_pm_slug:
|
||||||
|
from roboco.services.notification_text import task_display
|
||||||
|
|
||||||
await notify_pm_for_substitute(
|
await notify_pm_for_substitute(
|
||||||
self.session,
|
self.session,
|
||||||
pm_slug=target_pm_slug,
|
pm_slug=target_pm_slug,
|
||||||
@@ -8740,7 +8758,8 @@ class TaskService(BaseService):
|
|||||||
from_agent_id=agent.agent_id,
|
from_agent_id=agent.agent_id,
|
||||||
message=(
|
message=(
|
||||||
f"Task needs review: {updated.title or 'Unknown task'}",
|
f"Task needs review: {updated.title or 'Unknown task'}",
|
||||||
f"Task {task_id} requires PM review.\n\n"
|
f"Task {task_display(updated.title, task_id)} requires PM "
|
||||||
|
"review.\n\n"
|
||||||
f"Reason: {reason.value}\n"
|
f"Reason: {reason.value}\n"
|
||||||
f"Details: {details}\n\n"
|
f"Details: {details}\n\n"
|
||||||
"Please review and reassign as needed.",
|
"Please review and reassign as needed.",
|
||||||
@@ -9538,11 +9557,17 @@ class TaskService(BaseService):
|
|||||||
task_id=str(task_id),
|
task_id=str(task_id),
|
||||||
new_assignee=str(effective_assignee) if effective_assignee else None,
|
new_assignee=str(effective_assignee) if effective_assignee else None,
|
||||||
)
|
)
|
||||||
await self._notify_reassignment(task_id, previous_assignee, effective_assignee)
|
await self._notify_reassignment(
|
||||||
|
task_id, previous_assignee, effective_assignee, task.title
|
||||||
|
)
|
||||||
return task
|
return task
|
||||||
|
|
||||||
async def _notify_reassignment(
|
async def _notify_reassignment(
|
||||||
self, task_id: UUID, previous_assignee: Any, new_assignee: Any
|
self,
|
||||||
|
task_id: UUID,
|
||||||
|
previous_assignee: Any,
|
||||||
|
new_assignee: Any,
|
||||||
|
task_title: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Best-effort coordination notification for a real ownership change.
|
"""Best-effort coordination notification for a real ownership change.
|
||||||
|
|
||||||
@@ -9561,6 +9586,7 @@ class TaskService(BaseService):
|
|||||||
else None,
|
else None,
|
||||||
new_assignee=str(new_assignee) if new_assignee is not None else None,
|
new_assignee=str(new_assignee) if new_assignee is not None else None,
|
||||||
db_session=self.session,
|
db_session=self.session,
|
||||||
|
task_title=task_title,
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
self.log.warning(
|
self.log.warning(
|
||||||
@@ -10186,7 +10212,7 @@ class TaskService(BaseService):
|
|||||||
return await self.unblock(task_id, agent_role="cell_pm")
|
return await self.unblock(task_id, agent_role="cell_pm")
|
||||||
|
|
||||||
restored = await self._apply_pre_block_restore(task, restored_status)
|
restored = await self._apply_pre_block_restore(task, restored_status)
|
||||||
await self._notify_unblock(task_id, restored.assigned_to)
|
await self._notify_unblock(task_id, restored.assigned_to, restored.title)
|
||||||
return restored
|
return restored
|
||||||
|
|
||||||
async def _apply_pre_block_restore(
|
async def _apply_pre_block_restore(
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ from http import HTTPStatus
|
|||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
|
from roboco.services.notification_text import task_display
|
||||||
from tests.e2e_smoke.arcs import seed_company, seed_project, seed_task
|
from tests.e2e_smoke.arcs import seed_company, seed_project, seed_task
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
@@ -225,7 +226,8 @@ def test_unblock_persists_alert_notification(e2e_stack: E2EStack) -> None:
|
|||||||
note = notifications[0]
|
note = notifications[0]
|
||||||
assert "alert" in note["type"].lower(), note
|
assert "alert" in note["type"].lower(), note
|
||||||
assert note["related_task_id"] == task_id, note
|
assert note["related_task_id"] == task_id, note
|
||||||
assert note["subject"] == f"Task {task_id} unblocked", note
|
expected_display = task_display("Rotate the expired staging credential", task_id)
|
||||||
|
assert note["subject"] == f"Task {expected_display} unblocked", note
|
||||||
assert note["priority"], "priority must be populated"
|
assert note["priority"], "priority must be populated"
|
||||||
assert company.dev_id in note["to_agents"], note
|
assert company.dev_id in note["to_agents"], note
|
||||||
|
|
||||||
@@ -294,8 +296,11 @@ def test_dependency_revival_persists_alert_notification(e2e_stack: E2EStack) ->
|
|||||||
note = notifications[0]
|
note = notifications[0]
|
||||||
assert "alert" in note["type"].lower(), note
|
assert "alert" in note["type"].lower(), note
|
||||||
assert note["related_task_id"] == dependent_id, note
|
assert note["related_task_id"] == dependent_id, note
|
||||||
assert note["subject"] == f"Task {dependent_id} revived by dependency completion", (
|
expected_display = task_display(
|
||||||
note
|
"Wire the new endpoint to the shared auth helper", dependent_id
|
||||||
)
|
)
|
||||||
|
assert (
|
||||||
|
note["subject"] == f"Task {expected_display} revived by dependency completion"
|
||||||
|
), note
|
||||||
assert note["priority"], "priority must be populated"
|
assert note["priority"], "priority must be populated"
|
||||||
assert company.dev_id in note["to_agents"], note
|
assert company.dev_id in note["to_agents"], note
|
||||||
|
|||||||
@@ -238,4 +238,5 @@ async def test_board_review_gate_flips_only_after_both_reviewers_go_idle(
|
|||||||
note = notes[0]
|
note = notes[0]
|
||||||
assert note.from_agent == UUID(_SYSTEM_UUID)
|
assert note.from_agent == UUID(_SYSTEM_UUID)
|
||||||
assert UUID(_CEO_UUID) in note.to_agents
|
assert UUID(_CEO_UUID) in note.to_agents
|
||||||
assert str(task_id) in note.subject
|
# Subjects are human-readable now: task title + #id8, never the raw UUID.
|
||||||
|
assert f"#{str(task_id)[:8]}" in note.subject
|
||||||
|
|||||||
@@ -97,7 +97,7 @@ async def test_third_unblock_notifies_ceo_once() -> None:
|
|||||||
env = await _unblock_once(c, pm_id, task_id, t)
|
env = await _unblock_once(c, pm_id, task_id, t)
|
||||||
assert env.error is None, env.as_dict()
|
assert env.error is None, env.as_dict()
|
||||||
|
|
||||||
notify.assert_awaited_once_with(task_id, _THREE_FLIPS)
|
notify.assert_awaited_once_with(task_id, _THREE_FLIPS, t.title)
|
||||||
assert markers.is_block_flip_notified(t) is True
|
assert markers.is_block_flip_notified(t) is True
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -226,6 +226,7 @@ async def test_ceo_handoff_once_when_board_review_complete() -> None:
|
|||||||
|
|
||||||
svc = AsyncMock()
|
svc = AsyncMock()
|
||||||
task_svc = AsyncMock()
|
task_svc = AsyncMock()
|
||||||
|
task_svc.get.return_value = SimpleNamespace(title="Strategic feature")
|
||||||
db_ctx, task_ctx = _patch_handoff_db(task_svc)
|
db_ctx, task_ctx = _patch_handoff_db(task_svc)
|
||||||
with (
|
with (
|
||||||
patch.object(orch, "_is_agent_active", return_value=False),
|
patch.object(orch, "_is_agent_active", return_value=False),
|
||||||
@@ -239,7 +240,7 @@ async def test_ceo_handoff_once_when_board_review_complete() -> None:
|
|||||||
|
|
||||||
task_svc.mark_board_review_complete.assert_awaited_once()
|
task_svc.mark_board_review_complete.assert_awaited_once()
|
||||||
svc.send_board_review_complete_notification.assert_awaited_once_with(
|
svc.send_board_review_complete_notification.assert_awaited_once_with(
|
||||||
task_id=task_id
|
task_id=task_id, task_title="Strategic feature"
|
||||||
)
|
)
|
||||||
assert task_id in orch._board_review_ceo_notified
|
assert task_id in orch._board_review_ceo_notified
|
||||||
|
|
||||||
|
|||||||
@@ -159,7 +159,8 @@ async def test_send_blocker_notification(svc: NotificationService) -> None:
|
|||||||
from_agent="system",
|
from_agent="system",
|
||||||
to_pm="cell-pm",
|
to_pm="cell-pm",
|
||||||
)
|
)
|
||||||
assert any("Task t1" in row.subject for row in db.added)
|
# No task_title passed → falls back to the short-id display, not the raw id.
|
||||||
|
assert any("Task #t1" in row.subject for row in db.added)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|||||||
@@ -0,0 +1,88 @@
|
|||||||
|
"""Unit coverage for the shared notification-text helpers.
|
||||||
|
|
||||||
|
``task_display``/``agent_display`` are the producer-side fix so a human
|
||||||
|
reading a notification (panel, bell, Telegram) sees a task title / agent
|
||||||
|
slug instead of a raw UUID — see ``roboco/services/notification_text.py``.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from typing import Any
|
||||||
|
from unittest.mock import AsyncMock, MagicMock
|
||||||
|
from uuid import uuid4
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from roboco.foundation.identity import AGENTS
|
||||||
|
from roboco.services.notification_text import agent_display, task_display
|
||||||
|
|
||||||
|
|
||||||
|
class _Task:
|
||||||
|
def __init__(self, title: str | None) -> None:
|
||||||
|
self.title = title
|
||||||
|
|
||||||
|
|
||||||
|
def test_task_display_prefers_title_from_row() -> None:
|
||||||
|
task_id = uuid4()
|
||||||
|
display = task_display(_Task("Fix login bug"), task_id)
|
||||||
|
assert display == f"'Fix login bug' (#{str(task_id)[:8]})"
|
||||||
|
|
||||||
|
|
||||||
|
def test_task_display_accepts_bare_title_string() -> None:
|
||||||
|
task_id = uuid4()
|
||||||
|
display = task_display("Fix login bug", task_id)
|
||||||
|
assert display == f"'Fix login bug' (#{str(task_id)[:8]})"
|
||||||
|
|
||||||
|
|
||||||
|
def test_task_display_truncates_long_titles() -> None:
|
||||||
|
long_title = "x" * 100
|
||||||
|
task_id = uuid4()
|
||||||
|
display = task_display(long_title, task_id)
|
||||||
|
assert display == f"'{'x' * 40}' (#{str(task_id)[:8]})"
|
||||||
|
|
||||||
|
|
||||||
|
def test_task_display_falls_back_to_short_id_when_no_title() -> None:
|
||||||
|
task_id = uuid4()
|
||||||
|
assert task_display(None, task_id) == f"#{str(task_id)[:8]}"
|
||||||
|
assert task_display(_Task(None), task_id) == f"#{str(task_id)[:8]}"
|
||||||
|
assert task_display(_Task(""), task_id) == f"#{str(task_id)[:8]}"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_agent_display_resolves_via_static_map() -> None:
|
||||||
|
"""A fixed-roster agent's UUID resolves to its slug with zero DB I/O."""
|
||||||
|
row = AGENTS["be-dev-1"]
|
||||||
|
assert await agent_display(row.uuid) == "be-dev-1"
|
||||||
|
assert await agent_display(str(row.uuid)) == "be-dev-1"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_agent_display_falls_back_to_db_for_unknown_uuid() -> None:
|
||||||
|
"""A UUID absent from the static map resolves via ``get_agent_slug`` when
|
||||||
|
a db session is supplied."""
|
||||||
|
unknown_uuid = uuid4()
|
||||||
|
db: Any = MagicMock()
|
||||||
|
result = MagicMock()
|
||||||
|
result.scalar_one_or_none.return_value = "fresh-agent"
|
||||||
|
db.execute = AsyncMock(return_value=result)
|
||||||
|
|
||||||
|
assert await agent_display(unknown_uuid, db) == "fresh-agent"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_agent_display_raw_fallback_without_db() -> None:
|
||||||
|
"""No static-map hit + no db session ⇒ the raw value passes through."""
|
||||||
|
unknown_uuid = uuid4()
|
||||||
|
assert await agent_display(unknown_uuid) == str(unknown_uuid)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_agent_display_raw_fallback_for_non_uuid_slug() -> None:
|
||||||
|
"""A plain slug string that isn't in the UUID-keyed map passes through
|
||||||
|
unchanged (e.g. a value that's already a friendly slug)."""
|
||||||
|
assert await agent_display("some-slug") == "some-slug"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_agent_display_passes_through_none() -> None:
|
||||||
|
"""None stays None — "unassigned"/"its owner" wording stays at call sites."""
|
||||||
|
assert await agent_display(None) is None
|
||||||
Reference in New Issue
Block a user