mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[77719d3f] A2A team telemetry: coordination event notifications for 5 event types (#477)
* [13d03d5c] Add 5 coordination-event notification producers + wire at chokepoints (#472) (#474) * [13d03d5c] Add 5 coordination-event notification producer methods * [13d03d5c] Wire reassignment/collision/unblock/dependency-revival notifications * [13d03d5c] Wire stale-claim-reaped notification into orchestrator reaper * [13d03d5c] fix(runtime): guard reaper's UUID annotation + defensive attr access The stale-claim-reaped notification hook added a runtime-unquoted `UUID` type annotation (only imported under TYPE_CHECKING, so the module raised NameError on import) and a direct `t.assigned_to` attribute access that crashes against the minimal test doubles the existing reaper test suite uses. Quote the annotation and switch to getattr-defensive access, matching `_assignee_is_provider_parked`'s existing convention in the same file. * [13d03d5c] test(notification): unit coverage for 5 coordination-event producers One test per new send_* method (reassignment, collision-sequencing, unblock, dependency-revival, stale-claim-reaped) following the existing _FakeDb/_patch_db_context pattern, asserting subject/body/ related_task_id/priority/recipient-count, plus a no-recipients no-op case for reassignment. * [13d03d5c] test(task): prove reassign + unblock don't double-fire notifications Two chokepoint-level tests mocking NotificationService at its defining module: a repeated reassign() to the same already-current target skips the notification (guarded by comparing against the pre-mutation assignee), and a repeated unblock() on the same task only notifies once since the second call short-circuits on the status!=BLOCKED guard. * [13d03d5c] style(task): ruff format the collision-sequencing wiring block No behavior change — reflows the newly-added _notify_collision_sequencing call site to satisfy ruff format's line-length rules. * [13d03d5c] docs(backend): add coordination-event notification producers guide Documented the 5 new NotificationService producers (reassignment, collision-sequencing, unblock, dependency-revival, stale-claim-reaped) with fire conditions, double-fire prevention mechanisms, and implementation patterns. Updated backend README to link the new services guide for developers integrating new coordination events. --------- Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> Co-authored-by: Backend Documenter <be-doc@roboco.tech> * [3ee8150b] Frontend: render coordination-event notifications + e2e smoke coverage (#475) * [69777c3a] test(e2e-smoke): add coverage for soft-block + unblock coordination notifications (#471) Co-authored-by: Frontend Developer 2 <fe-dev-2@roboco.tech> * [8eb82639] Render 5 coordination-event notification types with task deep-links (#470) * [8eb82639] feat(notifications): add APPROVAL type icon and deep-link component test Add missing APPROVAL member to the frontend NotificationType enum to match backend roboco/models/base.py, wire its icon into the existing typeIcons Record in the notifications page, and add a component test covering type rendering and the task deep-link. * [8eb82639] docs(notifications): document 5 coordination-event types and APPROVAL enum addition Added comprehensive reference guide explaining the 5 notification types (TASK_ASSIGNMENT, BLOCKER_ESCALATION, REVIEW_REQUEST, DOCUMENTATION_REQUEST, APPROVAL), their visual identities (icon + color), use cases, and deep-linking behavior to related tasks. Updated panel README with quick reference table. TypeScript Record pattern ensures exhaustive type coverage at build time. --------- Co-authored-by: Frontend Developer 1 <fe-dev-1@roboco.tech> Co-authored-by: Frontend Documenter <fe-doc@roboco.tech> --------- Co-authored-by: Frontend Developer 2 <fe-dev-2@roboco.tech> Co-authored-by: Frontend Developer 1 <fe-dev-1@roboco.tech> Co-authored-by: Frontend Documenter <fe-doc@roboco.tech> * [a27de2a8] fix(docs): reflow hard-wrapped notification-types.md to pass markdown gate (#479) (#481) The Python quality gate on assembled PR #477 was red because the newly added docs/frontend/components/notification-types.md (introduced by the frontend coordination-event rendering commit) had manually wrapped prose paragraphs, which scripts/reflow_md.py --check rejects as part of make quality. Reflowed the file with scripts/reflow_md.py --apply (whitespace only, no content change) so the check passes. ruff format/check, mypy, xenon, vulture, bandit, and the full pytest suite (10284 passed) all confirmed green on this commit; notification.py, task.py, and orchestrator.py are untouched. Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> * [705419d5] Remove duplicate unblock notification and fix its dependent tests (#485) (#488) * [705419d5] fix(notifications): remove duplicate unblock notification, fix its tests The /unblock route was still calling delivery.notify_assignee_of_unblock() (TASK_ASSIGNMENT) after TaskService.unblock() already sent the send_unblock_notification() ALERT wired in by an earlier task — a real duplicate notification on every unblock. Delete the route-layer call and the now-dead NotificationDeliveryService.notify_assignee_of_unblock method, fix the integration test that mocked it, and fix/extend the e2e notification-coordination-events test to assert the persisted ALERT rows (exact subjects) for both the direct-unblock and dependency-revival producers instead of the old TASK_ASSIGNMENT assertion. * [705419d5] docs(backend): update coordination-events doc for unblock duplicate removal --------- Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> Co-authored-by: Backend Documenter <be-doc@roboco.tech> * [6c142a73] docs(changelog): document restored coordination-event notification producers and add collision-sequencing double-fire test (#489) (#490) Co-authored-by: Backend Developer 2 <be-dev-2@roboco.tech> * [77719d3f] Seed system agent in e2e harness to fix unblock/dependency-revival notifications The e2e harness's seed_company omitted the system sentinel agent that production seeds via initial_data.py. The unblock and dependency-revival notification producers default to from_agent="system", which _resolve_agent_uuid looks up by slug in the DB. With no system row the resolver returns None and _create_notification silently skips the notification, so the two ALERT assertions got 0 rows instead of 1. The soft-block test passed because it uses NotificationDeliveryService which creates the notification directly with a real agent UUID as from_agent, bypassing the slug resolution path entirely. * [77719d3f] Use foundation UUID for system agent to avoid slug collision The first attempt seeded the system agent with a random UUID. Other tests (_seed_system_and_secretary, _seed_video_agents) check by the fixed foundation UUID via session.get(AgentTable, uuid); not finding it they INSERT their own system row, hitting ix_agents_slug. Using the foundation UUID makes their check find the seed_company row and skip. * [77719d3f] Fix dependency-revival notification event loop mismatch The dependency-revival test calls _unblock_dependents directly via stack.run_db, which creates a new asyncio event loop. Inside, _notify_dependency_revival -> NotificationService._create_notification opened its own session via get_db_context(), which reuses the singleton _DbHolder engine — bound to the FastAPI server's event loop. The asyncpg connection raised 'Future attached to a different loop' and the exception was silently caught + logged as a warning, so the notification never persisted and the test saw 0 rows. Fix: add an optional db_session parameter to _create_notification and the two send methods. When provided, use the caller's session directly and skip the internal commit (the caller owns the transaction). The TaskService's _notify_unblock and _notify_dependency_revival now pass self.session, keeping the notification in the same event loop + session as the task transition. * [77719d3f] Scope system-agent seeding to notification tests only Seeding the system sentinel in seed_company (commits 3bba7b32/617b7890) fixed the 0-notification bug but caused 3 i_documented gateway_timeout failures: every e2e test now paid notification-creation latency for system-origin notifications that were previously silently skipped, pushing the already-slow i_documented verb past its 120s timeout. Move system-agent seeding out of seed_company and into a scoped _seed_system_agent helper called only by the two coordination-event tests that exercise send_unblock_notification / send_dependency_revival_notification (both resolve from_agent='system' via DB lookup). dev_lifecycle and state_machine tests revert to the pre-fix behavior (system-origin notifications silently skipped, no extra latency). The event-loop fix (commit7b95d77d: pass db_session=self.session to _create_notification) is unchanged — dependency_revival still needs it because stack.run_db creates a new event loop while _DbHolder.engine is bound to the FastAPI server loop. * [77719d3f] Fix reassignment notification deadlock + suppressed-notification commit regression Two fixes in notification.py / task.py: 1. Cross-session self-deadlock in send_reassignment_notification: TaskService.reassign() flushes an uncommitted row lock on the task, then calls _notify_reassignment -> send_reassignment_notification -> _create_notification(db_session=None) which opens a SEPARATE session via get_db_context() and INSERTs a notification with related_task_id FK -> tasks.id. The FK key-share lock blocks on the request session's uncommitted exclusive lock, but the request can't commit until the notify returns -> 120s verb hard-cut. Fix: pass db_session=self.session so the notification joins the verb's own transaction, same pattern as the unblock/dependency-revival fix in7b95d77d. 2. Suppressed-notification commit regression: the7b95d77drefactor moved await db.commit() out of _create_notification_with_session into _create_notification's db_session=None branch, where it ran unconditionally — even when _create_notification_with_session returned early (suppressed: unresolvable from_agent / no recipients / refire-guard / dedup-hit). Fix: _create_notification_with_session now returns bool (False at each early return, True after delivery); _create_notification commits only when created is True. --------- Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> Co-authored-by: Backend Documenter <be-doc@roboco.tech> Co-authored-by: Frontend Developer 2 <fe-dev-2@roboco.tech> Co-authored-by: Frontend Developer 1 <fe-dev-1@roboco.tech> Co-authored-by: Frontend Documenter <fe-doc@roboco.tech> Co-authored-by: Backend Developer 2 <be-dev-2@roboco.tech> Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
co-authored by
Backend Developer 1
Backend Documenter
Frontend Developer 2
Frontend Developer 1
Frontend Documenter
Backend Developer 2
Renn F
parent
acb4d567d2
commit
1114ee5ea0
@@ -4,6 +4,12 @@ All notable changes to RoboCo are documented in this file.
|
|||||||
|
|
||||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||||
|
|
||||||
|
## [Unreleased]
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- **Restored five coordination-event notification producers with double-fire guards.** Reassignment, collision-sequencing, unblock, dependency-revival, and stale-claim-reaped notifications are now wired at their lifecycle chokepoints in `TaskService` and the orchestrator reaper, each with an idempotent upstream guard preventing duplicate ALERT rows. The duplicate route-level `notify_assignee_of_unblock` call in `POST /api/tasks/{id}/unblock` was removed so unblock fires exactly one notification. Added `docs/backend/services/coordination-events.md` and `tests/e2e_smoke/test_notification_coordination_events.py` covering the restored producers.
|
||||||
|
|
||||||
## [0.23.0] - 2026-07-11
|
## [0.23.0] - 2026-07-11
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
@@ -11,6 +11,8 @@ Documentation for the Backend Cell team.
|
|||||||
|
|
||||||
- `/api/` - API documentation
|
- `/api/` - API documentation
|
||||||
- `/qa/` - QA-related docs
|
- `/qa/` - QA-related docs
|
||||||
|
- `/services/` - Internal service architecture & patterns
|
||||||
|
- `coordination-events.md` - 5 coordination-event notification producers: reassignment, collision-sequencing, unblock, dependency-revival, stale-claim-reaped
|
||||||
|
|
||||||
## Contributing
|
## Contributing
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,171 @@
|
|||||||
|
# Coordination-Event Notifications
|
||||||
|
|
||||||
|
The NotificationService provides five typed producers for coordination events—state transitions that affect multiple agents across a task lifecycle. Each producer fires at a specific chokepoint and is guarded against double-firing through idempotent upstream conditions.
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
Coordination events differ from generic task-state notifications: they signal changes that require coordination between agents (reassignments, dependency unblocks, sequencing blocks) or escalations (stale claims). Every producer follows the same pattern:
|
||||||
|
|
||||||
|
1. **Typed producer method** in `NotificationService` (e.g., `send_reassignment_notification`)
|
||||||
|
2. **Best-effort wiring** at the chokepoint via a helper method (e.g., `_notify_reassignment` in TaskService)
|
||||||
|
3. **Try/except guard** — a notification failure never breaks the underlying state transition
|
||||||
|
4. **Double-fire prevention** — idempotent upstream guards ensure no duplicate notifications
|
||||||
|
|
||||||
|
## The Five Producers
|
||||||
|
|
||||||
|
### 1. Reassignment: `send_reassignment_notification`
|
||||||
|
|
||||||
|
**Fired from:** `TaskService.reassign()` → `_notify_reassignment()`
|
||||||
|
|
||||||
|
**Trigger:** A task is reassigned to a different agent.
|
||||||
|
|
||||||
|
**Recipients:** Previous assignee + new assignee + CEO
|
||||||
|
|
||||||
|
**Double-fire guard:** `_notify_reassignment` checks `new_assignee == previous_assignee` and returns early if nothing changed. TaskService only captures the assignee once (pre-mutation) so if `reassign` is called twice in succession, the second call operates on an already-updated assignee and the guard catches it.
|
||||||
|
|
||||||
|
**Subject & body:**
|
||||||
|
```
|
||||||
|
Subject: Task {task_id} reassigned
|
||||||
|
Body: Task {task_id} was reassigned from {previous} to {new}.
|
||||||
|
```
|
||||||
|
|
||||||
|
**Example scenario:** A PM delegates a task from developer A to developer B. Both developers and the CEO receive notification of the change.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 2. Collision Sequencing: `send_collision_sequencing_notification`
|
||||||
|
|
||||||
|
**Fired from:** `TaskService.wire_sibling_collision_dag()` → `_notify_collision_sequencing()`
|
||||||
|
|
||||||
|
**Trigger:** A file/migration/shared-surface collision causes the collision-sequencing analyzer to add a dependency edge, holding back one task behind a blocking sibling.
|
||||||
|
|
||||||
|
**Recipients:** Held-back task's assignee + CEO
|
||||||
|
|
||||||
|
**Double-fire guard:** The wiring only fires the notification when `add_dependency` returns `True` (a freshly-inserted edge). Subsequent calls to `wire_sibling_collision_dag` over the same sibling pair contribute no new edge and therefore trigger no notification.
|
||||||
|
|
||||||
|
**Subject & body:**
|
||||||
|
```
|
||||||
|
Subject: Task {held_back_task_id} sequenced behind a sibling
|
||||||
|
Body: Task {held_back_task_id} was held back by the collision-sequencing
|
||||||
|
analyzer: it now depends on task {blocking_task_id}, which surfaced
|
||||||
|
an overlapping file/migration/shared-surface collision. It will
|
||||||
|
resume once that task reaches a terminal state.
|
||||||
|
```
|
||||||
|
|
||||||
|
**Example scenario:** Two backend dev tasks both touch `roboco/models/task.py` and one adds a migration. The analyzer detects a collision and adds a sequencing edge, notifying the developer of the held-back task.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 3. Unblock: `send_unblock_notification`
|
||||||
|
|
||||||
|
**Fired from:** `TaskService.unblock()` / `unblock_with_restore()` → `_notify_unblock()`
|
||||||
|
|
||||||
|
**Trigger:** A PM or resolver explicitly unblocks a task that was in BLOCKED status.
|
||||||
|
|
||||||
|
**Recipients:** Restored owner + CEO
|
||||||
|
|
||||||
|
**Double-fire guard:** Both `unblock` and `unblock_with_restore` are guarded by a status check (`status != BLOCKED` short-circuits early). A repeated call against an already-unblocked task is a no-op upstream and never reaches the notification handler.
|
||||||
|
|
||||||
|
**Route-layer duplicate removed:** The `POST /api/tasks/{id}/unblock` route previously called `NotificationDeliveryService.notify_assignee_of_unblock()` (a `TASK_ASSIGNMENT` notification) after `TaskService.unblock()` had already sent the ALERT above. That route-layer call and the now-dead delivery method have been removed, so the ALERT from the service chokepoint is the only notification fired for an unblock.
|
||||||
|
|
||||||
|
**Subject & body:**
|
||||||
|
```
|
||||||
|
Subject: Task {task_id} unblocked
|
||||||
|
Body: Task {task_id} has been unblocked and handed back to {owner}.
|
||||||
|
It is ready to resume.
|
||||||
|
```
|
||||||
|
|
||||||
|
**Example scenario:** A task was blocked by an external dependency. The dependency resolves, the PM calls `unblock()`, and the task owner is notified it's ready to resume.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 4. Dependency Revival: `send_dependency_revival_notification`
|
||||||
|
|
||||||
|
**Fired from:** `TaskService._unblock_dependents()` → `_notify_dependency_revival()`
|
||||||
|
|
||||||
|
**Trigger:** A task's **last outstanding dependency completes**, automatically reviving the task.
|
||||||
|
|
||||||
|
**Recipients:** Revived task's assignee + CEO
|
||||||
|
|
||||||
|
**Double-fire guard:** `_unblock_dependents` prunes the `dependency_ids` list **before** firing the notification. A repeated call for the same completed dependency finds no matching dependent and never reaches the notification handler.
|
||||||
|
|
||||||
|
**Distinct from `send_unblock_notification`:** This fires when a dependency completes automatically (no resolver acted). `send_unblock_notification` fires when a PM explicitly calls unblock on a task blocked by escalation. The notification names which dependency unblocked it rather than who resolved it.
|
||||||
|
|
||||||
|
**Subject & body:**
|
||||||
|
```
|
||||||
|
Subject: Task {task_id} revived by dependency completion
|
||||||
|
Body: Task {task_id} was revived: its dependency {completed_dependency_id}
|
||||||
|
just completed and no other dependencies remain. It is ready to resume.
|
||||||
|
```
|
||||||
|
|
||||||
|
**Example scenario:** A task was blocked waiting on three dependencies. The first two complete and unblock nothing (others remain). The third completes, the last dependency clears, and the task is auto-revived with a notification.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 5. Stale Claim Reaped: `send_stale_claim_reaped_notification`
|
||||||
|
|
||||||
|
**Fired from:** Orchestrator's `_reap_with_service()` → `_notify_stale_claim_reaped()`
|
||||||
|
|
||||||
|
**Trigger:** The reaper detects a stale claim (no heartbeat updates) and releases it back to PENDING.
|
||||||
|
|
||||||
|
**Recipients:** Reaped agent + CEO
|
||||||
|
|
||||||
|
**Priority:** HIGH (higher than other coordination events; stale claims are operational issues)
|
||||||
|
|
||||||
|
**Double-fire guard:** A reaped task leaves `list_in_progress_or_claimed` once released to PENDING. A subsequent reaper tick never re-considers the same claim and cannot re-fire the notification.
|
||||||
|
|
||||||
|
**Subject & body:**
|
||||||
|
```
|
||||||
|
Subject: Task {task_id}: stale claim reaped
|
||||||
|
Body: Task {task_id}'s claim went stale (last heartbeat: {timestamp})
|
||||||
|
and was reaped back to pending, releasing it from {reaped_agent}.
|
||||||
|
```
|
||||||
|
|
||||||
|
**Example scenario:** An agent crashed or became unresponsive while holding a task claim. The reaper detects the stale heartbeat, releases the claim, and notifies both the agent and CEO of the forced release.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Implementation Pattern
|
||||||
|
|
||||||
|
Every producer follows a consistent best-effort pattern at its call site:
|
||||||
|
|
||||||
|
```python
|
||||||
|
async def _notify_<event>(self, ...) -> None:
|
||||||
|
"""Best-effort coordination notification for a <event>."""
|
||||||
|
if <guard condition>:
|
||||||
|
return
|
||||||
|
try:
|
||||||
|
from roboco.services.notification import NotificationService
|
||||||
|
await NotificationService().send_<event>_notification(...)
|
||||||
|
except Exception as e:
|
||||||
|
self.log.warning(
|
||||||
|
"<Event> notify failed", task_id=str(...), error=str(e)
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Why this pattern:**
|
||||||
|
- **Localized guards** in the helper prevent unnecessary notification attempts
|
||||||
|
- **Try/except** ensures a notification failure never breaks the state transition
|
||||||
|
- **Logged & swallowed** — operational visibility without crashing the flow
|
||||||
|
- **Lazy import** avoids circular dependencies at the service layer
|
||||||
|
|
||||||
|
## Adding a New Coordination Event
|
||||||
|
|
||||||
|
When a new coordination event arises:
|
||||||
|
|
||||||
|
1. **Add a new producer method** to `NotificationService` following the existing signature (typed params, docstring describing fire condition + double-fire guard, calls `_create_notification` with `related_task_id` set)
|
||||||
|
2. **Add a helper** in the originating service (TaskService, Orchestrator, etc.) following the best-effort pattern
|
||||||
|
3. **Wire at the chokepoint** — the single place the state transition happens
|
||||||
|
4. **Document the guard** in the helper's docstring so reviewers understand why no duplicate can fire
|
||||||
|
5. **Test the guard** — add a chokepoint-level test proving a repeated call doesn't fire twice
|
||||||
|
|
||||||
|
See `test_notification.py` and `test_task.py` for worked examples of producer-level and chokepoint-level tests.
|
||||||
|
|
||||||
|
## Related Files
|
||||||
|
|
||||||
|
- **Implementation:** `roboco/services/notification.py` (producers)
|
||||||
|
- **Wiring:** `roboco/services/task.py` (TaskService helpers), `roboco/runtime/orchestrator.py` (reaper hook)
|
||||||
|
- **Route:** `roboco/api/routes/tasks.py` (`unblock` endpoint; service-layer ALERT only, no route-level duplicate)
|
||||||
|
- **Unit tests:** `tests/unit/services/test_notification.py` (producer unit tests), `tests/unit/services/test_task.py` (chokepoint double-fire proofs)
|
||||||
|
- **Route/chokepoint integration tests:** `tests/integration/test_tasks_routes.py` (unblock returns 200 and leaves `blocked` with no duplicate notification), `tests/e2e_smoke/test_notification_coordination_events.py` (DB-truth checks for unblock and dependency-revival ALERT rows)
|
||||||
|
- **Data model:** `roboco/models/notification.py` (CreateNotificationParams)
|
||||||
@@ -0,0 +1,83 @@
|
|||||||
|
# Notification Types Reference
|
||||||
|
|
||||||
|
The RoboCo panel renders five core **coordination-event notification types** — formal signals between agents tied to task lifecycle events. Each type has a visual identity (icon + color) and a semantic meaning. All types carry optional deep-links to related tasks.
|
||||||
|
|
||||||
|
## The 5 Coordination-Event Types
|
||||||
|
|
||||||
|
### 1. TASK_ASSIGNMENT
|
||||||
|
**Icon:** ListTodo (green) **Use case:** An agent has been assigned a new task. **When sent:** Via `NotificationService.send_task_assignment` when a PM assigns work. **Related task:** Usually carries `related_task_id` linking to the assigned task.
|
||||||
|
|
||||||
|
### 2. BLOCKER_ESCALATION
|
||||||
|
**Icon:** AlertTriangle (red) **Use case:** A developer is blocked and has escalated the issue to the PM. **When sent:** Via `NotificationDeliveryService.escalate_and_notify` when an agent calls `i_am_blocked`. **Related task:** Links to the task that is blocked.
|
||||||
|
|
||||||
|
### 3. REVIEW_REQUEST
|
||||||
|
**Icon:** Check (purple) **Use case:** QA has been asked to review a developer's work. **When sent:** Via `NotificationService.send_qa_ready` when a dev submits `i_am_done`. **Related task:** Links to the task under review.
|
||||||
|
|
||||||
|
### 4. DOCUMENTATION_REQUEST
|
||||||
|
**Icon:** Info (blue) **Use case:** A documenter has been asked to write docs for a code change. **When sent:** Via `NotificationService.send_docs_ready` when QA passes a task. **Related task:** Links to the task whose code needs documenting.
|
||||||
|
|
||||||
|
### 5. APPROVAL *(New in this release)*
|
||||||
|
**Icon:** ShieldCheck (emerald) **Use case:** A Board member (Product Owner, Head of Marketing, or Main PM) is requested to approve or provide feedback on escalated work. **When sent:** Via `NotificationDeliveryService.notify_ceo_of_escalation` when a task escalates to the Board. **Related task:** Links to the task awaiting approval. **Backend reference:** Matches `roboco/models/base.py` `NotificationType.APPROVAL`.
|
||||||
|
|
||||||
|
## Implementation Details
|
||||||
|
|
||||||
|
All types are defined in `panel/src/types/index.ts` under the `NotificationType` enum:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
export enum NotificationType {
|
||||||
|
TASK_ASSIGNMENT = "task_assignment",
|
||||||
|
BLOCKER_ESCALATION = "blocker_escalation",
|
||||||
|
REVIEW_REQUEST = "review_request",
|
||||||
|
DOCUMENTATION_REQUEST = "documentation_request",
|
||||||
|
APPROVAL = "approval", // Board-level approval requests (PO/HM/Main PM)
|
||||||
|
// ... other types (ALERT, BROADCAST, etc.)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Icon Mapping
|
||||||
|
Icons are rendered in `panel/src/app/(dashboard)/notifications/page.tsx` via the `typeIcons` Record — a TypeScript-enforced exhaustive mapping that ensures every enum member has a visual representation:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
const typeIcons: Record<NotificationType, React.ReactNode> = {
|
||||||
|
[NotificationType.TASK_ASSIGNMENT]: <ListTodo className="h-4 w-4 text-green-500" />,
|
||||||
|
[NotificationType.BLOCKER_ESCALATION]: <AlertTriangle className="h-4 w-4 text-red-500" />,
|
||||||
|
[NotificationType.REVIEW_REQUEST]: <Check className="h-4 w-4 text-purple-500" />,
|
||||||
|
[NotificationType.DOCUMENTATION_REQUEST]: <Info className="h-4 w-4 text-blue-500" />,
|
||||||
|
[NotificationType.APPROVAL]: <ShieldCheck className="h-4 w-4 text-emerald-500" />,
|
||||||
|
// ...
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
TypeScript's `Record<K, V>` type ensures that if a new `NotificationType` enum member is added without a corresponding icon entry, the code will not compile — preventing the "missing icon" bug at build time.
|
||||||
|
|
||||||
|
### Deep-Linking to Tasks
|
||||||
|
When a notification carries a `related_task_id`, the notifications page renders a Next.js `Link` component:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
{notification.related_task_id && (
|
||||||
|
<Link href={`/tasks/${notification.related_task_id}`} className="text-primary hover:underline">
|
||||||
|
Task #{notification.related_task_id.substring(0, 8)}
|
||||||
|
</Link>
|
||||||
|
)}
|
||||||
|
```
|
||||||
|
|
||||||
|
This allows agents to navigate directly from the notification inbox to the related task's detail view.
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
The 5 coordination-event types are covered by component tests in `panel/src/app/(dashboard)/notifications/__tests__/page.test.tsx`:
|
||||||
|
|
||||||
|
1. **Deep-link test:** Verifies that a TASK_ASSIGNMENT notification renders a working `<Link>` to `/tasks/{task_id}`.
|
||||||
|
2. **Icon test:** Verifies that all 5 types render and are queryable by their subject text.
|
||||||
|
|
||||||
|
## Adding a New Coordination-Event Type
|
||||||
|
|
||||||
|
To add a new coordination-event notification type:
|
||||||
|
|
||||||
|
1. Add the enum member to `NotificationType` in `panel/src/types/index.ts`
|
||||||
|
2. Add a corresponding icon entry to the `typeIcons` Record in `notifications/page.tsx`
|
||||||
|
3. Add a test case to the component test file
|
||||||
|
4. Update backend `roboco/models/base.py` `NotificationType` to match
|
||||||
|
5. Wire the notification send/delivery method in the backend service
|
||||||
|
|
||||||
|
The TypeScript exhaustiveness check will catch any missing icon entry at build time.
|
||||||
+16
-1
@@ -50,8 +50,9 @@ That gives you Next dev-server on `localhost:3000`, but you still need the orche
|
|||||||
- `src/components/` — React components (organized by feature: tasks, agents, channels, …)
|
- `src/components/` — React components (organized by feature: tasks, agents, channels, …)
|
||||||
- `src/lib/api/` — typed API client (thin wrappers over `fetch`)
|
- `src/lib/api/` — typed API client (thin wrappers over `fetch`)
|
||||||
- `src/lib/` — constants, utilities, WebSocket hooks
|
- `src/lib/` — constants, utilities, WebSocket hooks
|
||||||
- `src/types/` — shared TypeScript types mirroring backend schemas
|
- `src/types/` — shared TypeScript types mirroring backend schemas (includes `NotificationType` enum)
|
||||||
- `src/hooks/` — reusable React hooks (see [Frontend hooks](../docs/frontend/hooks.md))
|
- `src/hooks/` — reusable React hooks (see [Frontend hooks](../docs/frontend/hooks.md))
|
||||||
|
- `src/app/(dashboard)/notifications/` — notifications inbox page and components
|
||||||
|
|
||||||
## Hooks
|
## Hooks
|
||||||
|
|
||||||
@@ -73,6 +74,20 @@ const { register, unregister, refresh, loading, disabled } = usePageRefresh();
|
|||||||
|
|
||||||
Wrap your page or layout in `PageRefreshProvider` from `@/components/providers` before consuming the hook. Dashboard pages should register their refetch callbacks and avoid adding inline "Refresh" buttons; see [`docs/frontend/components/page-refresh-provider.md`](../docs/frontend/components/page-refresh-provider.md) for the full wiring list and examples.
|
Wrap your page or layout in `PageRefreshProvider` from `@/components/providers` before consuming the hook. Dashboard pages should register their refetch callbacks and avoid adding inline "Refresh" buttons; see [`docs/frontend/components/page-refresh-provider.md`](../docs/frontend/components/page-refresh-provider.md) for the full wiring list and examples.
|
||||||
|
|
||||||
|
## Notifications
|
||||||
|
|
||||||
|
The panel renders five core **coordination-event notification types** that signal task lifecycle transitions between agents:
|
||||||
|
|
||||||
|
| Type | Icon | Color | Meaning |
|
||||||
|
|------|------|-------|---------|
|
||||||
|
| `TASK_ASSIGNMENT` | ListTodo | green | A task has been assigned to you |
|
||||||
|
| `BLOCKER_ESCALATION` | AlertTriangle | red | A developer is blocked and escalated |
|
||||||
|
| `REVIEW_REQUEST` | Check | purple | Your review is needed |
|
||||||
|
| `DOCUMENTATION_REQUEST` | Info | blue | Documentation is needed |
|
||||||
|
| `APPROVAL` | ShieldCheck | emerald | Board-level approval requested |
|
||||||
|
|
||||||
|
Each notification optionally carries a `related_task_id` rendered as a deep-link to `/tasks/{id}`. For full details on types, icons, and adding new types, see [`docs/frontend/components/notification-types.md`](../docs/frontend/components/notification-types.md).
|
||||||
|
|
||||||
## Dependency Management
|
## Dependency Management
|
||||||
|
|
||||||
### Version Alignment
|
### Version Alignment
|
||||||
|
|||||||
@@ -0,0 +1,129 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
|
import { render, screen } from "@testing-library/react";
|
||||||
|
import type { ReactNode } from "react";
|
||||||
|
import { PageRefreshProvider } from "@/components/providers";
|
||||||
|
import { NotificationType, NotificationPriority, type Notification } from "@/types";
|
||||||
|
|
||||||
|
const {
|
||||||
|
useNotifications,
|
||||||
|
useMarkNotificationRead,
|
||||||
|
useAcknowledgeNotification,
|
||||||
|
useMarkAllNotificationsRead,
|
||||||
|
} = vi.hoisted(() => ({
|
||||||
|
useNotifications: vi.fn(),
|
||||||
|
useMarkNotificationRead: vi.fn(),
|
||||||
|
useAcknowledgeNotification: vi.fn(),
|
||||||
|
useMarkAllNotificationsRead: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("next/navigation", () => ({
|
||||||
|
useRouter: () => ({ push: vi.fn() }),
|
||||||
|
useSearchParams: () => new URLSearchParams("tab=all"),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/hooks/use-notifications", () => ({
|
||||||
|
useNotifications,
|
||||||
|
useMarkNotificationRead,
|
||||||
|
useAcknowledgeNotification,
|
||||||
|
useMarkAllNotificationsRead,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/components/ui/markdown", () => ({
|
||||||
|
Markdown: ({ children }: { children: string }) => <div>{children}</div>,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("sonner", () => ({
|
||||||
|
toast: { success: vi.fn(), error: vi.fn() },
|
||||||
|
}));
|
||||||
|
|
||||||
|
import NotificationsPage from "../page";
|
||||||
|
|
||||||
|
function withPageRefresh(ui: ReactNode) {
|
||||||
|
return <PageRefreshProvider>{ui}</PageRefreshProvider>;
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildNotification(overrides: Partial<Notification> = {}): Notification {
|
||||||
|
return {
|
||||||
|
id: "notif-1",
|
||||||
|
type: NotificationType.TASK_ASSIGNMENT,
|
||||||
|
priority: NotificationPriority.NORMAL,
|
||||||
|
from_agent: "fe-pm-00000000",
|
||||||
|
to_agents: ["fe-dev-1"],
|
||||||
|
subject: "New task assigned",
|
||||||
|
body: "You have been assigned a new task.",
|
||||||
|
requires_ack: false,
|
||||||
|
is_acknowledged: false,
|
||||||
|
is_fully_acknowledged: false,
|
||||||
|
is_read: false,
|
||||||
|
related_task_id: "11111111-2222-3333-4444-555555555555",
|
||||||
|
related_message_ids: [],
|
||||||
|
timestamp: "2026-07-11T09:00:00Z",
|
||||||
|
expires_at: null,
|
||||||
|
acked_by: [],
|
||||||
|
acked_at: {},
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("NotificationsPage", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
useMarkNotificationRead.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
useAcknowledgeNotification.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
useMarkAllNotificationsRead.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders a TASK_ASSIGNMENT notification with a working deep-link to its task", () => {
|
||||||
|
useNotifications.mockReturnValue({
|
||||||
|
data: {
|
||||||
|
items: [buildNotification()],
|
||||||
|
total: 1,
|
||||||
|
unread_count: 1,
|
||||||
|
pending_ack_count: 0,
|
||||||
|
},
|
||||||
|
isLoading: false,
|
||||||
|
error: null,
|
||||||
|
refetch: vi.fn(),
|
||||||
|
});
|
||||||
|
|
||||||
|
render(withPageRefresh(<NotificationsPage />));
|
||||||
|
|
||||||
|
expect(screen.getByText("New task assigned")).toBeInTheDocument();
|
||||||
|
|
||||||
|
const taskLink = screen.getByRole("link", { name: /Task #11111111/i });
|
||||||
|
expect(taskLink).toHaveAttribute(
|
||||||
|
"href",
|
||||||
|
"/tasks/11111111-2222-3333-4444-555555555555",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders each of the 5 coordination-event notification types with a distinguishing icon", () => {
|
||||||
|
const types = [
|
||||||
|
NotificationType.TASK_ASSIGNMENT,
|
||||||
|
NotificationType.BLOCKER_ESCALATION,
|
||||||
|
NotificationType.REVIEW_REQUEST,
|
||||||
|
NotificationType.DOCUMENTATION_REQUEST,
|
||||||
|
NotificationType.APPROVAL,
|
||||||
|
];
|
||||||
|
const items = types.map((type, idx) =>
|
||||||
|
buildNotification({
|
||||||
|
id: `notif-${idx}`,
|
||||||
|
type,
|
||||||
|
subject: `Subject for ${type}`,
|
||||||
|
related_task_id: null,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
useNotifications.mockReturnValue({
|
||||||
|
data: { items, total: items.length, unread_count: 0, pending_ack_count: 0 },
|
||||||
|
isLoading: false,
|
||||||
|
error: null,
|
||||||
|
refetch: vi.fn(),
|
||||||
|
});
|
||||||
|
|
||||||
|
render(withPageRefresh(<NotificationsPage />));
|
||||||
|
|
||||||
|
for (const type of types) {
|
||||||
|
expect(screen.getByText(`Subject for ${type}`)).toBeInTheDocument();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -29,6 +29,7 @@ import {
|
|||||||
MailOpen,
|
MailOpen,
|
||||||
BookOpen,
|
BookOpen,
|
||||||
AtSign,
|
AtSign,
|
||||||
|
ShieldCheck,
|
||||||
} from "lucide-react";
|
} from "lucide-react";
|
||||||
import { formatDistanceToNow } from "date-fns";
|
import { formatDistanceToNow } from "date-fns";
|
||||||
import { toast } from "sonner";
|
import { toast } from "sonner";
|
||||||
@@ -50,6 +51,9 @@ const typeIcons: Record<NotificationType, React.ReactNode> = {
|
|||||||
[NotificationType.DOCUMENTATION_REQUEST]: (
|
[NotificationType.DOCUMENTATION_REQUEST]: (
|
||||||
<Info className="h-4 w-4 text-blue-500" />
|
<Info className="h-4 w-4 text-blue-500" />
|
||||||
),
|
),
|
||||||
|
[NotificationType.APPROVAL]: (
|
||||||
|
<ShieldCheck className="h-4 w-4 text-emerald-500" />
|
||||||
|
),
|
||||||
[NotificationType.ALERT]: (
|
[NotificationType.ALERT]: (
|
||||||
<AlertTriangle className="h-4 w-4 text-yellow-500" />
|
<AlertTriangle className="h-4 w-4 text-yellow-500" />
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -120,6 +120,7 @@ export enum NotificationType {
|
|||||||
BLOCKER_ESCALATION = "blocker_escalation",
|
BLOCKER_ESCALATION = "blocker_escalation",
|
||||||
REVIEW_REQUEST = "review_request",
|
REVIEW_REQUEST = "review_request",
|
||||||
DOCUMENTATION_REQUEST = "documentation_request",
|
DOCUMENTATION_REQUEST = "documentation_request",
|
||||||
|
APPROVAL = "approval", // Board-level approval requests (PO/HM/Main PM)
|
||||||
ALERT = "alert",
|
ALERT = "alert",
|
||||||
BROADCAST = "broadcast",
|
BROADCAST = "broadcast",
|
||||||
KNOWLEDGE_SHARE = "knowledge_share", // Cross-agent learning notification
|
KNOWLEDGE_SHARE = "knowledge_share", // Cross-agent learning notification
|
||||||
|
|||||||
@@ -154,6 +154,10 @@ select = [
|
|||||||
# agent_id, project_ids, route, session_id) — same >5-kwarg rationale as the
|
# agent_id, project_ids, route, session_id) — same >5-kwarg rationale as the
|
||||||
# gateway verb surfaces below.
|
# gateway verb surfaces below.
|
||||||
"roboco/services/prompter.py" = ["PLR0913"]
|
"roboco/services/prompter.py" = ["PLR0913"]
|
||||||
|
# send_dependency_revival_notification carries the coordination-event contract
|
||||||
|
# (task_id, assignee, completed_dependency_id, from_agent, to_ceo, db_session) —
|
||||||
|
# db_session is the caller's session for event-loop-safe notification creation.
|
||||||
|
"roboco/services/notification.py" = ["PLR0913"]
|
||||||
# open_video_task's kwargs (occasion, script, platforms, brief,
|
# open_video_task's kwargs (occasion, script, platforms, brief,
|
||||||
# suggested_input_props, project_id) are the authoring-task contract shared
|
# suggested_input_props, project_id) are the authoring-task contract shared
|
||||||
# by the release/spotlight/on-demand callers — same "bundling would just
|
# by the release/spotlight/on-demand callers — same "bundling would just
|
||||||
|
|||||||
@@ -1527,9 +1527,6 @@ async def unblock_task(
|
|||||||
detail="Not authorized to unblock this task",
|
detail="Not authorized to unblock this task",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Remember the assigned agent before unblocking
|
|
||||||
assigned_agent_id = task.assigned_to
|
|
||||||
|
|
||||||
task = await service.unblock(task_id, agent.role)
|
task = await service.unblock(task_id, agent.role)
|
||||||
if not task:
|
if not task:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@@ -1537,16 +1534,9 @@ async def unblock_task(
|
|||||||
detail="Cannot unblock task - not blocked",
|
detail="Cannot unblock task - not blocked",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Notify the assigned agent that the task is unblocked
|
# TaskService.unblock() already sends the ALERT unblock notification
|
||||||
if assigned_agent_id and assigned_agent_id != agent.agent_id:
|
# (send_unblock_notification) to the restored owner + CEO — do not
|
||||||
delivery = get_notification_delivery_service(db)
|
# duplicate it here.
|
||||||
await delivery.notify_assignee_of_unblock(
|
|
||||||
task=task,
|
|
||||||
task_id=task_id,
|
|
||||||
from_agent_id=agent.agent_id,
|
|
||||||
assignee_agent_id=require_uuid(assigned_agent_id),
|
|
||||||
)
|
|
||||||
|
|
||||||
await db.commit()
|
await db.commit()
|
||||||
return task_to_response(task)
|
return task_to_response(task)
|
||||||
|
|
||||||
|
|||||||
@@ -10998,6 +10998,9 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
if self._assignee_is_provider_parked(t):
|
if self._assignee_is_provider_parked(t):
|
||||||
continue
|
continue
|
||||||
task_id = require_uuid(t.id)
|
task_id = require_uuid(t.id)
|
||||||
|
reaped_agent = getattr(t, "assigned_to", None) or getattr(
|
||||||
|
t, "claimed_by", None
|
||||||
|
)
|
||||||
try:
|
try:
|
||||||
await svc.unclaim_for_reaper(task_id)
|
await svc.unclaim_for_reaper(task_id)
|
||||||
logger.warning(
|
logger.warning(
|
||||||
@@ -11011,6 +11014,35 @@ Start now: evidence(task_id="{task_id}")
|
|||||||
task_id=str(task_id),
|
task_id=str(task_id),
|
||||||
error=str(exc),
|
error=str(exc),
|
||||||
)
|
)
|
||||||
|
else:
|
||||||
|
await self._notify_stale_claim_reaped(task_id, reaped_agent, ts)
|
||||||
|
|
||||||
|
async def _notify_stale_claim_reaped(
|
||||||
|
self, task_id: "UUID", reaped_agent: Any, last_heartbeat: datetime | None
|
||||||
|
) -> None:
|
||||||
|
"""Best-effort coordination notification for a reaped stale claim.
|
||||||
|
|
||||||
|
Best-effort: a notification failure must not wedge the reaper tick,
|
||||||
|
so any error is logged and swallowed. A reaped task leaves
|
||||||
|
``list_in_progress_or_claimed`` once released to pending, so a later
|
||||||
|
reaper tick never re-considers the same claim and cannot double-fire.
|
||||||
|
"""
|
||||||
|
if reaped_agent is None:
|
||||||
|
return
|
||||||
|
from roboco.services.notification import NotificationService
|
||||||
|
|
||||||
|
try:
|
||||||
|
await NotificationService().send_stale_claim_reaped_notification(
|
||||||
|
task_id=str(task_id),
|
||||||
|
reaped_agent=str(reaped_agent),
|
||||||
|
last_heartbeat=last_heartbeat.isoformat() if last_heartbeat else None,
|
||||||
|
)
|
||||||
|
except Exception as exc:
|
||||||
|
logger.warning(
|
||||||
|
"Failed to send stale-claim-reaped notification",
|
||||||
|
task_id=str(task_id),
|
||||||
|
error=str(exc),
|
||||||
|
)
|
||||||
|
|
||||||
async def _dispatch_all_work(self) -> None:
|
async def _dispatch_all_work(self) -> None:
|
||||||
"""Run all dispatchers to check for and assign work.
|
"""Run all dispatchers to check for and assign work.
|
||||||
|
|||||||
+316
-82
@@ -355,6 +355,218 @@ class NotificationService:
|
|||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
async def send_reassignment_notification(
|
||||||
|
self,
|
||||||
|
task_id: str,
|
||||||
|
previous_assignee: str | None,
|
||||||
|
new_assignee: str | None,
|
||||||
|
from_agent: str | None = None,
|
||||||
|
to_ceo: str = "ceo",
|
||||||
|
db_session: AsyncSession | None = None,
|
||||||
|
) -> None:
|
||||||
|
"""Tell the outgoing + incoming owner (and the CEO) a task moved.
|
||||||
|
|
||||||
|
Skipped by the caller when ``new_assignee == previous_assignee`` —
|
||||||
|
``TaskService.reassign`` runs even on a no-op redirect, and a
|
||||||
|
same-owner "reassignment" is not a coordination event.
|
||||||
|
"""
|
||||||
|
recipients = list(
|
||||||
|
dict.fromkeys(r for r in (previous_assignee, new_assignee, to_ceo) if r)
|
||||||
|
)
|
||||||
|
if not recipients:
|
||||||
|
return
|
||||||
|
logger.info(
|
||||||
|
"Sending reassignment notification",
|
||||||
|
task_id=task_id,
|
||||||
|
previous_assignee=previous_assignee,
|
||||||
|
new_assignee=new_assignee,
|
||||||
|
)
|
||||||
|
body = (
|
||||||
|
f"Task {task_id} was reassigned from "
|
||||||
|
f"{previous_assignee or 'unassigned'} to {new_assignee or 'unassigned'}."
|
||||||
|
)
|
||||||
|
await self._create_notification(
|
||||||
|
CreateNotificationParams(
|
||||||
|
notification_type=NotificationType.ALERT,
|
||||||
|
priority=NotificationPriority.NORMAL,
|
||||||
|
from_agent=from_agent or "system",
|
||||||
|
to_agents=recipients,
|
||||||
|
subject=f"Task {task_id} reassigned",
|
||||||
|
body=body,
|
||||||
|
related_task_id=task_id,
|
||||||
|
),
|
||||||
|
db_session=db_session,
|
||||||
|
)
|
||||||
|
|
||||||
|
async def send_collision_sequencing_notification(
|
||||||
|
self,
|
||||||
|
held_back_task_id: str,
|
||||||
|
blocking_task_id: str,
|
||||||
|
held_back_assignee: str | None,
|
||||||
|
from_agent: str | None = None,
|
||||||
|
to_ceo: str = "ceo",
|
||||||
|
) -> None:
|
||||||
|
"""Tell the held-back task's owner (+ CEO) it now waits on a sibling.
|
||||||
|
|
||||||
|
Fired only for a newly-created collision-sequencing edge (see
|
||||||
|
``wire_sibling_collision_dag`` — a repeat wiring pass over an
|
||||||
|
already-wired pair contributes no edge, so this cannot double-fire).
|
||||||
|
"""
|
||||||
|
recipients = list(dict.fromkeys(r for r in (held_back_assignee, to_ceo) if r))
|
||||||
|
if not recipients:
|
||||||
|
return
|
||||||
|
logger.info(
|
||||||
|
"Sending collision-sequencing notification",
|
||||||
|
held_back_task_id=held_back_task_id,
|
||||||
|
blocking_task_id=blocking_task_id,
|
||||||
|
)
|
||||||
|
body = (
|
||||||
|
f"Task {held_back_task_id} was held back by the collision-sequencing "
|
||||||
|
f"analyzer: it now depends on task {blocking_task_id}, which surfaced "
|
||||||
|
"an overlapping file/migration/shared-surface collision. It will "
|
||||||
|
"resume once that task reaches a terminal state."
|
||||||
|
)
|
||||||
|
await self._create_notification(
|
||||||
|
CreateNotificationParams(
|
||||||
|
notification_type=NotificationType.ALERT,
|
||||||
|
priority=NotificationPriority.NORMAL,
|
||||||
|
from_agent=from_agent or "system",
|
||||||
|
to_agents=recipients,
|
||||||
|
subject=f"Task {held_back_task_id} sequenced behind a sibling",
|
||||||
|
body=body,
|
||||||
|
related_task_id=held_back_task_id,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
async def send_unblock_notification(
|
||||||
|
self,
|
||||||
|
task_id: str,
|
||||||
|
restored_owner: str | None,
|
||||||
|
from_agent: str | None = None,
|
||||||
|
to_ceo: str = "ceo",
|
||||||
|
db_session: AsyncSession | None = None,
|
||||||
|
) -> None:
|
||||||
|
"""Tell the restored owner (+ CEO) a blocked task is workable again.
|
||||||
|
|
||||||
|
Fired from ``TaskService.unblock`` / ``unblock_with_restore`` — both
|
||||||
|
only act on a task whose status is currently ``BLOCKED``, so a
|
||||||
|
repeated call against the same (already-unblocked) task is a no-op
|
||||||
|
upstream and this cannot double-fire.
|
||||||
|
"""
|
||||||
|
recipients = list(dict.fromkeys(r for r in (restored_owner, to_ceo) if r))
|
||||||
|
if not recipients:
|
||||||
|
return
|
||||||
|
logger.info(
|
||||||
|
"Sending unblock notification",
|
||||||
|
task_id=task_id,
|
||||||
|
restored_owner=restored_owner,
|
||||||
|
)
|
||||||
|
body = (
|
||||||
|
f"Task {task_id} has been unblocked and handed back to "
|
||||||
|
f"{restored_owner or 'its owner'}. It is ready to resume."
|
||||||
|
)
|
||||||
|
await self._create_notification(
|
||||||
|
CreateNotificationParams(
|
||||||
|
notification_type=NotificationType.ALERT,
|
||||||
|
priority=NotificationPriority.NORMAL,
|
||||||
|
from_agent=from_agent or "system",
|
||||||
|
to_agents=recipients,
|
||||||
|
subject=f"Task {task_id} unblocked",
|
||||||
|
body=body,
|
||||||
|
related_task_id=task_id,
|
||||||
|
),
|
||||||
|
db_session=db_session,
|
||||||
|
)
|
||||||
|
|
||||||
|
async def send_dependency_revival_notification(
|
||||||
|
self,
|
||||||
|
task_id: str,
|
||||||
|
assignee: str | None,
|
||||||
|
completed_dependency_id: str,
|
||||||
|
from_agent: str | None = None,
|
||||||
|
to_ceo: str = "ceo",
|
||||||
|
db_session: AsyncSession | None = None,
|
||||||
|
) -> None:
|
||||||
|
"""Tell the revived task's owner (+ CEO) its last dependency landed.
|
||||||
|
|
||||||
|
Distinct event from ``send_unblock_notification``: that one fires
|
||||||
|
when a resolver explicitly calls ``unblock``/``unblock_with_restore``
|
||||||
|
on a task blocked by escalation. This one fires from
|
||||||
|
``TaskService._unblock_dependents`` when the LAST outstanding
|
||||||
|
dependency of a task blocked ON THAT DEPENDENCY completes — no
|
||||||
|
resolver acted, the trigger is upstream task completion, so the
|
||||||
|
notification names which dependency unblocked it rather than who
|
||||||
|
resolved it. ``_unblock_dependents`` prunes ``dependency_ids`` before
|
||||||
|
this fires, so a repeated call for the same completed dependency
|
||||||
|
finds no matching dependent and cannot double-fire.
|
||||||
|
"""
|
||||||
|
recipients = list(dict.fromkeys(r for r in (assignee, to_ceo) if r))
|
||||||
|
if not recipients:
|
||||||
|
return
|
||||||
|
logger.info(
|
||||||
|
"Sending dependency-revival notification",
|
||||||
|
task_id=task_id,
|
||||||
|
completed_dependency_id=completed_dependency_id,
|
||||||
|
)
|
||||||
|
body = (
|
||||||
|
f"Task {task_id} was revived: its dependency "
|
||||||
|
f"{completed_dependency_id} just completed and no other "
|
||||||
|
"dependencies remain. It is ready to resume."
|
||||||
|
)
|
||||||
|
await self._create_notification(
|
||||||
|
CreateNotificationParams(
|
||||||
|
notification_type=NotificationType.ALERT,
|
||||||
|
priority=NotificationPriority.NORMAL,
|
||||||
|
from_agent=from_agent or "system",
|
||||||
|
to_agents=recipients,
|
||||||
|
subject=f"Task {task_id} revived by dependency completion",
|
||||||
|
body=body,
|
||||||
|
related_task_id=task_id,
|
||||||
|
),
|
||||||
|
db_session=db_session,
|
||||||
|
)
|
||||||
|
|
||||||
|
async def send_stale_claim_reaped_notification(
|
||||||
|
self,
|
||||||
|
task_id: str,
|
||||||
|
reaped_agent: str | None,
|
||||||
|
last_heartbeat: str | None = None,
|
||||||
|
from_agent: str = "system",
|
||||||
|
to_ceo: str = "ceo",
|
||||||
|
) -> None:
|
||||||
|
"""Tell the reaped agent (+ CEO) its stale claim was released.
|
||||||
|
|
||||||
|
Fired from the orchestrator's ``_reap_with_service`` alongside
|
||||||
|
``unclaim_for_reaper``. A reaped task leaves
|
||||||
|
``list_in_progress_or_claimed`` once released to pending, so a
|
||||||
|
subsequent reaper tick never re-considers the same claim and this
|
||||||
|
cannot double-fire.
|
||||||
|
"""
|
||||||
|
recipients = list(dict.fromkeys(r for r in (reaped_agent, to_ceo) if r))
|
||||||
|
if not recipients:
|
||||||
|
return
|
||||||
|
logger.info(
|
||||||
|
"Sending stale-claim-reaped notification",
|
||||||
|
task_id=task_id,
|
||||||
|
reaped_agent=reaped_agent,
|
||||||
|
)
|
||||||
|
body = (
|
||||||
|
f"Task {task_id}'s claim went stale "
|
||||||
|
f"(last heartbeat: {last_heartbeat or 'unknown'}) and was reaped "
|
||||||
|
f"back to pending, releasing it from {reaped_agent or 'its holder'}."
|
||||||
|
)
|
||||||
|
await self._create_notification(
|
||||||
|
CreateNotificationParams(
|
||||||
|
notification_type=NotificationType.ALERT,
|
||||||
|
priority=NotificationPriority.HIGH,
|
||||||
|
from_agent=from_agent,
|
||||||
|
to_agents=recipients,
|
||||||
|
subject=f"Task {task_id}: stale claim reaped",
|
||||||
|
body=body,
|
||||||
|
related_task_id=task_id,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
async def send_ack_notification(
|
async def send_ack_notification(
|
||||||
self,
|
self,
|
||||||
*,
|
*,
|
||||||
@@ -567,90 +779,112 @@ class NotificationService:
|
|||||||
return True
|
return True
|
||||||
return False
|
return False
|
||||||
|
|
||||||
async def _create_notification(self, params: CreateNotificationParams) -> None:
|
async def _create_notification(
|
||||||
"""Create a notification via the database and deliver it."""
|
self,
|
||||||
async with get_db_context() as db:
|
params: CreateNotificationParams,
|
||||||
from_agent_uuid = await _resolve_agent_uuid(db, params.from_agent)
|
db_session: AsyncSession | None = None,
|
||||||
if from_agent_uuid is None:
|
) -> None:
|
||||||
# notifications.from_agent is NOT NULL + FK to agents.id, so
|
"""Create a notification via the database and deliver it.
|
||||||
# we cannot insert. Skip-with-warn rather than crash the
|
|
||||||
# upstream request.
|
When ``db_session`` is provided, use it directly and skip the
|
||||||
logger.warning(
|
internal commit — the caller owns the transaction. This is required
|
||||||
"Skipping notification: from_agent unresolvable",
|
when the caller runs on a different event loop than the singleton
|
||||||
from_agent_input=str(params.from_agent),
|
``_DbHolder`` engine (e.g. ``TaskService`` called outside the FastAPI
|
||||||
type=self._notification_type_label(params),
|
request loop); opening ``get_db_context()`` there reuses an engine
|
||||||
subject=params.subject[:80],
|
whose asyncpg connections are bound to the server's loop, raising
|
||||||
to_agents=[str(a) for a in params.to_agents],
|
``Future attached to a different loop``.
|
||||||
)
|
"""
|
||||||
return
|
if db_session is not None:
|
||||||
to_agents_uuids = await self._resolve_recipients(db, params)
|
await self._create_notification_with_session(params, db_session)
|
||||||
if not to_agents_uuids:
|
else:
|
||||||
logger.warning(
|
async with get_db_context() as db:
|
||||||
"Skipping notification: no resolvable recipients",
|
created = await self._create_notification_with_session(params, db)
|
||||||
to_agents_input=[str(a) for a in params.to_agents],
|
if created:
|
||||||
type=self._notification_type_label(params),
|
await db.commit()
|
||||||
subject=params.subject[:80],
|
|
||||||
)
|
async def _create_notification_with_session(
|
||||||
return
|
self, params: CreateNotificationParams, db: AsyncSession
|
||||||
# Re-fire guard for loop-prone types: a 60s Redis SET-NX window
|
) -> bool:
|
||||||
# coalesces the per-tick re-notify storm the DB dedup below skips
|
from_agent_uuid = await _resolve_agent_uuid(db, params.from_agent)
|
||||||
# (these types are requires_ack=False). Fail-open on Redis down.
|
if from_agent_uuid is None:
|
||||||
if await all_recipients_recently_notified(
|
# notifications.from_agent is NOT NULL + FK to agents.id, so
|
||||||
ntype=params.notification_type,
|
# we cannot insert. Skip-with-warn rather than crash the
|
||||||
from_agent=from_agent_uuid,
|
# upstream request.
|
||||||
recipients=to_agents_uuids,
|
logger.warning(
|
||||||
related_task_id=params.related_task_id,
|
"Skipping notification: from_agent unresolvable",
|
||||||
subject=params.subject,
|
from_agent_input=str(params.from_agent),
|
||||||
):
|
type=self._notification_type_label(params),
|
||||||
logger.info(
|
subject=params.subject[:80],
|
||||||
"Suppressed re-fire notification (loop-prone, recent window)",
|
to_agents=[str(a) for a in params.to_agents],
|
||||||
from_agent=str(from_agent_uuid),
|
|
||||||
type=params.notification_type.value,
|
|
||||||
related_task_id=str(params.related_task_id)
|
|
||||||
if params.related_task_id is not None
|
|
||||||
else None,
|
|
||||||
to_agents=[str(a) for a in to_agents_uuids],
|
|
||||||
)
|
|
||||||
return
|
|
||||||
# Purpose-based dedup (CEO directive, 2026-06-10): suppress a second
|
|
||||||
# notification for the SAME purpose while a prior one is unacked. See
|
|
||||||
# ``_duplicate_unacked_exists`` for the rationale + the action-only
|
|
||||||
# scope (informational types carry distinct content per send).
|
|
||||||
if await self._duplicate_unacked_exists(
|
|
||||||
db,
|
|
||||||
from_agent_uuid=from_agent_uuid,
|
|
||||||
params=params,
|
|
||||||
to_agents_uuids=to_agents_uuids,
|
|
||||||
):
|
|
||||||
return
|
|
||||||
notification = NotificationTable(
|
|
||||||
type=params.notification_type,
|
|
||||||
priority=params.priority,
|
|
||||||
from_agent=from_agent_uuid,
|
|
||||||
to_agents=to_agents_uuids,
|
|
||||||
subject=params.subject,
|
|
||||||
body=params.body,
|
|
||||||
related_task_id=params.related_task_id,
|
|
||||||
# requires_ack follows ACK_REQUIRED_BY_TYPE (action-required vs
|
|
||||||
# informational), not the column's True default; default True
|
|
||||||
# for an unmapped type preserves the safe action-required bias.
|
|
||||||
requires_ack=ACK_REQUIRED_BY_TYPE.get(params.notification_type, True),
|
|
||||||
)
|
)
|
||||||
db.add(notification)
|
return False
|
||||||
await db.flush()
|
to_agents_uuids = await self._resolve_recipients(db, params)
|
||||||
|
if not to_agents_uuids:
|
||||||
# Deliver via Redis Streams for real-time push
|
logger.warning(
|
||||||
from roboco.services.notification_delivery import (
|
"Skipping notification: no resolvable recipients",
|
||||||
get_notification_delivery_service,
|
to_agents_input=[str(a) for a in params.to_agents],
|
||||||
|
type=self._notification_type_label(params),
|
||||||
|
subject=params.subject[:80],
|
||||||
)
|
)
|
||||||
|
return False
|
||||||
delivery_service = get_notification_delivery_service(db)
|
# Re-fire guard for loop-prone types: a 60s Redis SET-NX window
|
||||||
await delivery_service.deliver(require_uuid(notification.id))
|
# coalesces the per-tick re-notify storm the DB dedup below skips
|
||||||
|
# (these types are requires_ack=False). Fail-open on Redis down.
|
||||||
await db.commit()
|
if await all_recipients_recently_notified(
|
||||||
|
ntype=params.notification_type,
|
||||||
|
from_agent=from_agent_uuid,
|
||||||
|
recipients=to_agents_uuids,
|
||||||
|
related_task_id=params.related_task_id,
|
||||||
|
subject=params.subject,
|
||||||
|
):
|
||||||
logger.info(
|
logger.info(
|
||||||
"Notification created and delivered",
|
"Suppressed re-fire notification (loop-prone, recent window)",
|
||||||
notification_id=str(notification.id),
|
from_agent=str(from_agent_uuid),
|
||||||
type=params.notification_type.value,
|
type=params.notification_type.value,
|
||||||
|
related_task_id=str(params.related_task_id)
|
||||||
|
if params.related_task_id is not None
|
||||||
|
else None,
|
||||||
|
to_agents=[str(a) for a in to_agents_uuids],
|
||||||
)
|
)
|
||||||
|
return False
|
||||||
|
# Purpose-based dedup (CEO directive, 2026-06-10): suppress a second
|
||||||
|
# notification for the SAME purpose while a prior one is unacked. See
|
||||||
|
# ``_duplicate_unacked_exists`` for the rationale + the action-only
|
||||||
|
# scope (informational types carry distinct content per send).
|
||||||
|
if await self._duplicate_unacked_exists(
|
||||||
|
db,
|
||||||
|
from_agent_uuid=from_agent_uuid,
|
||||||
|
params=params,
|
||||||
|
to_agents_uuids=to_agents_uuids,
|
||||||
|
):
|
||||||
|
return False
|
||||||
|
notification = NotificationTable(
|
||||||
|
type=params.notification_type,
|
||||||
|
priority=params.priority,
|
||||||
|
from_agent=from_agent_uuid,
|
||||||
|
to_agents=to_agents_uuids,
|
||||||
|
subject=params.subject,
|
||||||
|
body=params.body,
|
||||||
|
related_task_id=params.related_task_id,
|
||||||
|
# requires_ack follows ACK_REQUIRED_BY_TYPE (action-required vs
|
||||||
|
# informational), not the column's True default; default True
|
||||||
|
# for an unmapped type preserves the safe action-required bias.
|
||||||
|
requires_ack=ACK_REQUIRED_BY_TYPE.get(params.notification_type, True),
|
||||||
|
)
|
||||||
|
db.add(notification)
|
||||||
|
await db.flush()
|
||||||
|
|
||||||
|
# Deliver via Redis Streams for real-time push
|
||||||
|
from roboco.services.notification_delivery import (
|
||||||
|
get_notification_delivery_service,
|
||||||
|
)
|
||||||
|
|
||||||
|
delivery_service = get_notification_delivery_service(db)
|
||||||
|
await delivery_service.deliver(require_uuid(notification.id))
|
||||||
|
|
||||||
|
logger.info(
|
||||||
|
"Notification created and delivered",
|
||||||
|
notification_id=str(notification.id),
|
||||||
|
type=params.notification_type.value,
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|||||||
@@ -794,30 +794,6 @@ class NotificationDeliveryService(BaseService):
|
|||||||
)
|
)
|
||||||
await self._persist_and_deliver(notification)
|
await self._persist_and_deliver(notification)
|
||||||
|
|
||||||
async def notify_assignee_of_unblock(
|
|
||||||
self,
|
|
||||||
*,
|
|
||||||
task: TaskTable,
|
|
||||||
task_id: UUID,
|
|
||||||
from_agent_id: UUID,
|
|
||||||
assignee_agent_id: UUID,
|
|
||||||
) -> None:
|
|
||||||
"""Notify the task's assigned agent that their task is unblocked."""
|
|
||||||
notification = NotificationTable(
|
|
||||||
type=NotificationType.TASK_ASSIGNMENT,
|
|
||||||
priority=NotificationPriority.HIGH,
|
|
||||||
from_agent=from_agent_id,
|
|
||||||
to_agents=[assignee_agent_id],
|
|
||||||
subject=f"Task unblocked: {task.title or 'Unknown task'}",
|
|
||||||
body=(
|
|
||||||
f"Task {task_id} has been unblocked and is ready to resume.\n\n"
|
|
||||||
"Review the task details in your briefing and continue work."
|
|
||||||
),
|
|
||||||
related_task_id=task_id,
|
|
||||||
requires_ack=ACK_REQUIRED_BY_TYPE[NotificationType.TASK_ASSIGNMENT],
|
|
||||||
)
|
|
||||||
await self._persist_and_deliver(notification)
|
|
||||||
|
|
||||||
async def notify_assignee_of_ceo_rejection(
|
async def notify_assignee_of_ceo_rejection(
|
||||||
self,
|
self,
|
||||||
*,
|
*,
|
||||||
|
|||||||
+119
-5
@@ -4739,8 +4739,33 @@ 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)
|
||||||
|
|
||||||
return task
|
return task
|
||||||
|
|
||||||
|
async def _notify_unblock(self, task_id: UUID, restored_owner: Any) -> None:
|
||||||
|
"""Best-effort coordination notification when a blocked task resumes.
|
||||||
|
|
||||||
|
Called from both `unblock` and `unblock_with_restore`'s restore=True
|
||||||
|
success path. `unblock` only acts on a task whose status is
|
||||||
|
currently BLOCKED (guarded above), so a repeated call against an
|
||||||
|
already-unblocked task never re-fires this.
|
||||||
|
"""
|
||||||
|
if restored_owner is None:
|
||||||
|
return
|
||||||
|
try:
|
||||||
|
from roboco.services.notification import NotificationService
|
||||||
|
|
||||||
|
await NotificationService().send_unblock_notification(
|
||||||
|
task_id=str(task_id),
|
||||||
|
restored_owner=str(restored_owner),
|
||||||
|
db_session=self.session,
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
self.log.warning(
|
||||||
|
"Unblock notify failed", task_id=str(task_id), error=str(e)
|
||||||
|
)
|
||||||
|
|
||||||
async def pause(
|
async def pause(
|
||||||
self, task_id: UUID, agent_role: str | None = None
|
self, task_id: UUID, agent_role: str | None = None
|
||||||
) -> TaskTable | None:
|
) -> TaskTable | None:
|
||||||
@@ -6943,6 +6968,14 @@ class TaskService(BaseService):
|
|||||||
task_id=str(task.id),
|
task_id=str(task.id),
|
||||||
completed_dependency=str(completed_task_id),
|
completed_dependency=str(completed_task_id),
|
||||||
)
|
)
|
||||||
|
# Distinct coordination event from `_notify_unblock`: THIS fires
|
||||||
|
# automatically when a dependency's completion clears the last
|
||||||
|
# blocker, not from a resolver explicitly calling `unblock` — no
|
||||||
|
# human/PM acted, so the notification names the dependency that
|
||||||
|
# unblocked it rather than a resolver. Dependency_ids was already
|
||||||
|
# pruned above, so a repeated pass over this completed_task_id
|
||||||
|
# finds no matching dependent and cannot double-fire.
|
||||||
|
await self._notify_dependency_revival(task, completed_task_id)
|
||||||
|
|
||||||
if blocked_tasks:
|
if blocked_tasks:
|
||||||
# Metrics (blocked-others): record how many downstream tasks this
|
# Metrics (blocked-others): record how many downstream tasks this
|
||||||
@@ -6966,6 +6999,27 @@ class TaskService(BaseService):
|
|||||||
|
|
||||||
await self.session.flush()
|
await self.session.flush()
|
||||||
|
|
||||||
|
async def _notify_dependency_revival(
|
||||||
|
self, task: TaskTable, completed_dependency_id: UUID
|
||||||
|
) -> None:
|
||||||
|
"""Best-effort coordination notification for an auto-revived dependent."""
|
||||||
|
owner = cast("Any", task.claimed_by or task.assigned_to)
|
||||||
|
if owner is None:
|
||||||
|
return
|
||||||
|
try:
|
||||||
|
from roboco.services.notification import NotificationService
|
||||||
|
|
||||||
|
await NotificationService().send_dependency_revival_notification(
|
||||||
|
task_id=str(task.id),
|
||||||
|
assignee=str(owner),
|
||||||
|
completed_dependency_id=str(completed_dependency_id),
|
||||||
|
db_session=self.session,
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
self.log.warning(
|
||||||
|
"Dependency-revival notify failed", task_id=str(task.id), error=str(e)
|
||||||
|
)
|
||||||
|
|
||||||
async def _revive_unblocked_dependent(self, task: TaskTable) -> None:
|
async def _revive_unblocked_dependent(self, task: TaskTable) -> None:
|
||||||
"""Resume — or re-home — a task whose last dependency just cleared.
|
"""Resume — or re-home — a task whose last dependency just cleared.
|
||||||
|
|
||||||
@@ -7491,7 +7545,7 @@ class TaskService(BaseService):
|
|||||||
result = await self.session.execute(query)
|
result = await self.session.execute(query)
|
||||||
return list(result.scalars().all())
|
return list(result.scalars().all())
|
||||||
|
|
||||||
async def add_dependency(self, task_id: UUID, depends_on_id: UUID) -> None:
|
async def add_dependency(self, task_id: UUID, depends_on_id: UUID) -> bool:
|
||||||
"""Append a dependency to a task WITHOUT changing its status.
|
"""Append a dependency to a task WITHOUT changing its status.
|
||||||
|
|
||||||
A PENDING task with unmet dependencies is held back by
|
A PENDING task with unmet dependencies is held back by
|
||||||
@@ -7503,6 +7557,11 @@ class TaskService(BaseService):
|
|||||||
Rejects a self-reference and any edge that would close a cycle (the
|
Rejects a self-reference and any edge that would close a cycle (the
|
||||||
reverse path already reaches the dependent) — a cycle deadlocks both
|
reverse path already reaches the dependent) — a cycle deadlocks both
|
||||||
tasks (`_unblock_dependents` never fires).
|
tasks (`_unblock_dependents` never fires).
|
||||||
|
|
||||||
|
Returns ``True`` only when a new edge was actually inserted, ``False``
|
||||||
|
for an idempotent no-op (missing task, already-present edge) — lets
|
||||||
|
callers like `wire_sibling_collision_dag` tell a fresh wiring pass
|
||||||
|
from a repeat one without a separate membership check.
|
||||||
"""
|
"""
|
||||||
if depends_on_id == task_id:
|
if depends_on_id == task_id:
|
||||||
raise ConflictError(
|
raise ConflictError(
|
||||||
@@ -7511,9 +7570,9 @@ class TaskService(BaseService):
|
|||||||
)
|
)
|
||||||
task = await self.get(task_id)
|
task = await self.get(task_id)
|
||||||
if task is None:
|
if task is None:
|
||||||
return
|
return False
|
||||||
if depends_on_id in task.dependency_ids:
|
if depends_on_id in task.dependency_ids:
|
||||||
return # idempotent re-add
|
return False # idempotent re-add
|
||||||
if await self._would_create_cycle(task_id, depends_on_id):
|
if await self._would_create_cycle(task_id, depends_on_id):
|
||||||
raise ConflictError(
|
raise ConflictError(
|
||||||
f"adding {depends_on_id} as a dependency of {task_id} would "
|
f"adding {depends_on_id} as a dependency of {task_id} would "
|
||||||
@@ -7522,6 +7581,7 @@ class TaskService(BaseService):
|
|||||||
)
|
)
|
||||||
task.dependency_ids = [*task.dependency_ids, depends_on_id]
|
task.dependency_ids = [*task.dependency_ids, depends_on_id]
|
||||||
await self.session.flush()
|
await self.session.flush()
|
||||||
|
return True
|
||||||
|
|
||||||
async def _would_create_cycle(self, task_id: UUID, depends_on_id: UUID) -> bool:
|
async def _would_create_cycle(self, task_id: UUID, depends_on_id: UUID) -> bool:
|
||||||
"""True if ``depends_on_id`` already (transitively) depends on ``task_id``.
|
"""True if ``depends_on_id`` already (transitively) depends on ``task_id``.
|
||||||
@@ -7570,9 +7630,33 @@ class TaskService(BaseService):
|
|||||||
from roboco.services.sequencing import dev_task_collision_edges
|
from roboco.services.sequencing import dev_task_collision_edges
|
||||||
|
|
||||||
siblings = await self.get_subtasks(parent_task_id)
|
siblings = await self.get_subtasks(parent_task_id)
|
||||||
|
siblings_by_id = {UUID(str(s.id)): s for s in siblings}
|
||||||
edges = dev_task_collision_edges(siblings)
|
edges = dev_task_collision_edges(siblings)
|
||||||
for depends_on_id, task_id in edges:
|
for depends_on_id, task_id in edges:
|
||||||
await self.add_dependency(UUID(str(task_id)), UUID(str(depends_on_id)))
|
held_back_id = UUID(str(task_id))
|
||||||
|
blocking_id = UUID(str(depends_on_id))
|
||||||
|
created = await self.add_dependency(held_back_id, blocking_id)
|
||||||
|
if created:
|
||||||
|
held_back = siblings_by_id.get(held_back_id)
|
||||||
|
owner = getattr(held_back, "assigned_to", None) if held_back else None
|
||||||
|
await self._notify_collision_sequencing(
|
||||||
|
held_back_id, blocking_id, owner
|
||||||
|
)
|
||||||
|
|
||||||
|
async def _notify_collision_sequencing(
|
||||||
|
self, held_back_task_id: UUID, blocking_task_id: UUID, owner: Any
|
||||||
|
) -> None:
|
||||||
|
"""Best-effort coordination notification for a newly-wired collision edge."""
|
||||||
|
try:
|
||||||
|
from roboco.services.notification import NotificationService
|
||||||
|
|
||||||
|
await NotificationService().send_collision_sequencing_notification(
|
||||||
|
held_back_task_id=str(held_back_task_id),
|
||||||
|
blocking_task_id=str(blocking_task_id),
|
||||||
|
held_back_assignee=str(owner) if owner is not None else None,
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
self.log.warning("Collision-sequencing notify failed", error=str(e))
|
||||||
|
|
||||||
async def wire_cell_task_wave_chain(self, cell_task_id: UUID) -> None:
|
async def wire_cell_task_wave_chain(self, cell_task_id: UUID) -> None:
|
||||||
"""Wire the cell-task wave chain (multi-level sequencing edge kind 2).
|
"""Wire the cell-task wave chain (multi-level sequencing edge kind 2).
|
||||||
@@ -9168,6 +9252,7 @@ class TaskService(BaseService):
|
|||||||
task = await self.get(task_id)
|
task = await self.get(task_id)
|
||||||
if task is None:
|
if task is None:
|
||||||
return None
|
return None
|
||||||
|
previous_assignee = cast("Any", task.assigned_to)
|
||||||
# Board/advisory → cell-task hand-off is diverted to the pool (see
|
# Board/advisory → cell-task hand-off is diverted to the pool (see
|
||||||
# `_maybe_divert_board_advisory_reassign`); otherwise proceed with the
|
# `_maybe_divert_board_advisory_reassign`); otherwise proceed with the
|
||||||
# normal handoff + cell-PM redirect.
|
# normal handoff + cell-PM redirect.
|
||||||
@@ -9212,8 +9297,35 @@ 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)
|
||||||
return task
|
return task
|
||||||
|
|
||||||
|
async def _notify_reassignment(
|
||||||
|
self, task_id: UUID, previous_assignee: Any, new_assignee: Any
|
||||||
|
) -> None:
|
||||||
|
"""Best-effort coordination notification for a real ownership change.
|
||||||
|
|
||||||
|
Skipped when nothing actually changed — `reassign` runs even on a
|
||||||
|
no-op redirect (the effective assignee already matched the request).
|
||||||
|
"""
|
||||||
|
if new_assignee == previous_assignee:
|
||||||
|
return
|
||||||
|
try:
|
||||||
|
from roboco.services.notification import NotificationService
|
||||||
|
|
||||||
|
await NotificationService().send_reassignment_notification(
|
||||||
|
task_id=str(task_id),
|
||||||
|
previous_assignee=str(previous_assignee)
|
||||||
|
if previous_assignee is not None
|
||||||
|
else None,
|
||||||
|
new_assignee=str(new_assignee) if new_assignee is not None else None,
|
||||||
|
db_session=self.session,
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
self.log.warning(
|
||||||
|
"Reassignment notify failed", task_id=str(task_id), error=str(e)
|
||||||
|
)
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class _CellPmRedirect:
|
class _CellPmRedirect:
|
||||||
"""Outcome of an `_resolve_cell_pm_redirect` call.
|
"""Outcome of an `_resolve_cell_pm_redirect` call.
|
||||||
@@ -9820,7 +9932,9 @@ class TaskService(BaseService):
|
|||||||
except ValueError:
|
except ValueError:
|
||||||
return await self.unblock(task_id, agent_role="cell_pm")
|
return await self.unblock(task_id, agent_role="cell_pm")
|
||||||
|
|
||||||
return 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)
|
||||||
|
return restored
|
||||||
|
|
||||||
async def _apply_pre_block_restore(
|
async def _apply_pre_block_restore(
|
||||||
self,
|
self,
|
||||||
|
|||||||
@@ -0,0 +1,301 @@
|
|||||||
|
"""Scenario: coordination-event notification producers land real DB rows.
|
||||||
|
|
||||||
|
Drives three of the coordination-event notification producers wired at
|
||||||
|
``TaskService``'s transition chokepoints (``roboco/services/task.py``) —
|
||||||
|
soft-block (BLOCKER_ESCALATION to the cell PM), unblock (ALERT to the
|
||||||
|
restored owner + CEO, ``send_unblock_notification``), and dependency-revival
|
||||||
|
(ALERT to the revived owner + CEO, ``send_dependency_revival_notification``)
|
||||||
|
— through the real REST task surface mounted at ``/api/tasks`` in this
|
||||||
|
harness (the same surface scenario 3's CEO approve-and-merge call uses), plus
|
||||||
|
one direct ``TaskService`` chokepoint call for the dependency-revival
|
||||||
|
producer (mirrors ``arcs.wire_dependency``'s pattern of driving
|
||||||
|
``TaskService`` directly for setup that has no bespoke REST endpoint). Real
|
||||||
|
in-process API, real ``NotificationService``/``NotificationDeliveryService``,
|
||||||
|
real ephemeral Postgres — no mocks. Each assertion reads the persisted
|
||||||
|
``NotificationTable`` row back out of the DB via ``E2EStack.run_db``, exactly
|
||||||
|
as the harness's other DB-truth checks do.
|
||||||
|
|
||||||
|
The unblock route (``POST /api/tasks/{id}/unblock``) used to ALSO send a
|
||||||
|
second, duplicate TASK_ASSIGNMENT notification from the route handler
|
||||||
|
itself (``notify_assignee_of_unblock``) on top of the ALERT
|
||||||
|
``TaskService.unblock()`` already sends. That duplicate route-layer call has
|
||||||
|
been removed — unblock fires exactly one notification now, the ALERT below.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from http import HTTPStatus
|
||||||
|
from typing import TYPE_CHECKING, Any
|
||||||
|
|
||||||
|
import httpx
|
||||||
|
from tests.e2e_smoke.arcs import seed_company, seed_project, seed_task
|
||||||
|
|
||||||
|
if TYPE_CHECKING:
|
||||||
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
from tests.e2e_smoke.harness import E2EStack
|
||||||
|
|
||||||
|
|
||||||
|
def _agent_headers(agent_id: Any, role: str) -> dict[str, str]:
|
||||||
|
return {"X-Agent-ID": str(agent_id), "X-Agent-Role": role}
|
||||||
|
|
||||||
|
|
||||||
|
def _seed_system_agent(stack: E2EStack) -> None:
|
||||||
|
"""Seed the ``system`` sentinel at its fixed foundation UUID.
|
||||||
|
|
||||||
|
Production seeds it via ``initial_data.py``; the e2e harness's
|
||||||
|
``seed_company`` deliberately does NOT (seeding it globally adds
|
||||||
|
notification-creation latency to every test, pushing ``i_documented``
|
||||||
|
past its 120 s verb timeout). Only the coordination-event tests that
|
||||||
|
exercise ``send_unblock_notification`` / ``send_dependency_revival_notification``
|
||||||
|
need it — both resolve ``from_agent="system"`` to a UUID via DB lookup.
|
||||||
|
"""
|
||||||
|
from roboco.db.tables import AgentTable
|
||||||
|
from roboco.foundation import identity as _foundation
|
||||||
|
from roboco.models import AgentRole, AgentStatus
|
||||||
|
|
||||||
|
async def _run(session: AsyncSession) -> None:
|
||||||
|
session.add(
|
||||||
|
AgentTable(
|
||||||
|
id=_foundation.AGENTS["system"].uuid,
|
||||||
|
name="system",
|
||||||
|
slug="system",
|
||||||
|
role=AgentRole.SYSTEM,
|
||||||
|
team=None,
|
||||||
|
status=AgentStatus.ACTIVE,
|
||||||
|
model_config={},
|
||||||
|
system_prompt="system",
|
||||||
|
capabilities=[],
|
||||||
|
permissions={},
|
||||||
|
metrics={},
|
||||||
|
)
|
||||||
|
)
|
||||||
|
await session.flush()
|
||||||
|
|
||||||
|
stack.run_db(_run)
|
||||||
|
|
||||||
|
|
||||||
|
def _notifications_for_task(
|
||||||
|
stack: E2EStack, task_id: Any, notification_type: Any
|
||||||
|
) -> list[dict[str, Any]]:
|
||||||
|
from roboco.db.tables import NotificationTable
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
async def _run(session: AsyncSession) -> list[dict[str, Any]]:
|
||||||
|
rows = (
|
||||||
|
(
|
||||||
|
await session.execute(
|
||||||
|
select(NotificationTable).where(
|
||||||
|
NotificationTable.related_task_id == task_id,
|
||||||
|
NotificationTable.type == notification_type,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
)
|
||||||
|
.scalars()
|
||||||
|
.all()
|
||||||
|
)
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
"type": str(r.type),
|
||||||
|
"related_task_id": r.related_task_id,
|
||||||
|
"subject": r.subject,
|
||||||
|
"priority": str(r.priority),
|
||||||
|
"to_agents": list(r.to_agents),
|
||||||
|
}
|
||||||
|
for r in rows
|
||||||
|
]
|
||||||
|
|
||||||
|
rows: list[dict[str, Any]] = stack.run_db(_run)
|
||||||
|
return rows
|
||||||
|
|
||||||
|
|
||||||
|
def test_soft_block_persists_blocker_escalation_notification(
|
||||||
|
e2e_stack: E2EStack,
|
||||||
|
) -> None:
|
||||||
|
"""soft-block chokepoint: notify_pm_of_block -> BLOCKER_ESCALATION."""
|
||||||
|
stack = e2e_stack
|
||||||
|
company = seed_company(stack)
|
||||||
|
project_id, _project_slug = seed_project(stack, company)
|
||||||
|
|
||||||
|
from roboco.models.base import TaskStatus
|
||||||
|
|
||||||
|
task_id = seed_task(
|
||||||
|
stack,
|
||||||
|
title="Investigate flaky upstream API",
|
||||||
|
description=(
|
||||||
|
"Dev is mid-work and hits an external dependency outage; "
|
||||||
|
"soft-blocking so the cell PM is paged for resolution."
|
||||||
|
),
|
||||||
|
acceptance_criteria=["the upstream API responds reliably again"],
|
||||||
|
project_id=project_id,
|
||||||
|
created_by=company.cell_pm_id,
|
||||||
|
assigned_to=company.dev_id,
|
||||||
|
claimed_by=company.dev_id,
|
||||||
|
active_claimant_id=company.dev_id,
|
||||||
|
status=TaskStatus.IN_PROGRESS,
|
||||||
|
branch_name="feature/backend/e2e-soft-block-notify",
|
||||||
|
)
|
||||||
|
|
||||||
|
resp = httpx.post(
|
||||||
|
f"{stack.base_url}/api/tasks/{task_id}/soft-block",
|
||||||
|
json={
|
||||||
|
"reason": "The upstream payments API is returning 503s.",
|
||||||
|
"blocker_type": "external",
|
||||||
|
"what_needed": "Wait for the upstream provider to recover.",
|
||||||
|
"resolver_type": "agent",
|
||||||
|
},
|
||||||
|
headers=_agent_headers(company.dev_id, "developer"),
|
||||||
|
timeout=30,
|
||||||
|
)
|
||||||
|
assert resp.status_code == HTTPStatus.OK, (
|
||||||
|
f"soft-block: {resp.status_code} {resp.text[:1500]}"
|
||||||
|
)
|
||||||
|
|
||||||
|
from roboco.models import NotificationType
|
||||||
|
|
||||||
|
notifications = _notifications_for_task(
|
||||||
|
stack, task_id, NotificationType.BLOCKER_ESCALATION
|
||||||
|
)
|
||||||
|
assert len(notifications) == 1, notifications
|
||||||
|
note = notifications[0]
|
||||||
|
assert "blocker_escalation" in note["type"].lower(), note
|
||||||
|
assert note["related_task_id"] == task_id, note
|
||||||
|
assert note["subject"], "subject must be populated"
|
||||||
|
assert note["priority"], "priority must be populated"
|
||||||
|
assert company.cell_pm_id in note["to_agents"], note
|
||||||
|
|
||||||
|
|
||||||
|
def test_unblock_persists_alert_notification(e2e_stack: E2EStack) -> None:
|
||||||
|
"""unblock chokepoint: TaskService.unblock -> send_unblock_notification -> ALERT.
|
||||||
|
|
||||||
|
Exactly one notification fires for unblock (the route-layer
|
||||||
|
``notify_assignee_of_unblock`` TASK_ASSIGNMENT duplicate was removed).
|
||||||
|
"""
|
||||||
|
stack = e2e_stack
|
||||||
|
company = seed_company(stack)
|
||||||
|
project_id, _project_slug = seed_project(stack, company)
|
||||||
|
_seed_system_agent(stack)
|
||||||
|
|
||||||
|
from roboco.models.base import TaskStatus
|
||||||
|
|
||||||
|
task_id = seed_task(
|
||||||
|
stack,
|
||||||
|
title="Rotate the expired staging credential",
|
||||||
|
description=(
|
||||||
|
"Dev soft-blocked waiting on a credential rotation; the cell "
|
||||||
|
"PM resolves it and unblocks the task so the dev resumes."
|
||||||
|
),
|
||||||
|
acceptance_criteria=["the staging credential is valid again"],
|
||||||
|
project_id=project_id,
|
||||||
|
created_by=company.cell_pm_id,
|
||||||
|
assigned_to=company.dev_id,
|
||||||
|
claimed_by=company.dev_id,
|
||||||
|
active_claimant_id=company.dev_id,
|
||||||
|
status=TaskStatus.IN_PROGRESS,
|
||||||
|
branch_name="feature/backend/e2e-unblock-notify",
|
||||||
|
)
|
||||||
|
|
||||||
|
block_resp = httpx.post(
|
||||||
|
f"{stack.base_url}/api/tasks/{task_id}/soft-block",
|
||||||
|
json={
|
||||||
|
"reason": "The staging DB credential expired overnight.",
|
||||||
|
"blocker_type": "external",
|
||||||
|
"what_needed": "A rotated staging credential from the cell PM.",
|
||||||
|
"resolver_type": "agent",
|
||||||
|
},
|
||||||
|
headers=_agent_headers(company.dev_id, "developer"),
|
||||||
|
timeout=30,
|
||||||
|
)
|
||||||
|
assert block_resp.status_code == HTTPStatus.OK, (
|
||||||
|
f"soft-block: {block_resp.status_code} {block_resp.text[:1500]}"
|
||||||
|
)
|
||||||
|
|
||||||
|
unblock_resp = httpx.post(
|
||||||
|
f"{stack.base_url}/api/tasks/{task_id}/unblock",
|
||||||
|
headers=_agent_headers(company.cell_pm_id, "cell_pm"),
|
||||||
|
timeout=30,
|
||||||
|
)
|
||||||
|
assert unblock_resp.status_code == HTTPStatus.OK, (
|
||||||
|
f"unblock: {unblock_resp.status_code} {unblock_resp.text[:1500]}"
|
||||||
|
)
|
||||||
|
|
||||||
|
from roboco.models import NotificationType
|
||||||
|
|
||||||
|
notifications = _notifications_for_task(stack, task_id, NotificationType.ALERT)
|
||||||
|
assert len(notifications) == 1, notifications
|
||||||
|
note = notifications[0]
|
||||||
|
assert "alert" in note["type"].lower(), note
|
||||||
|
assert note["related_task_id"] == task_id, note
|
||||||
|
assert note["subject"] == f"Task {task_id} unblocked", note
|
||||||
|
assert note["priority"], "priority must be populated"
|
||||||
|
assert company.dev_id in note["to_agents"], note
|
||||||
|
|
||||||
|
# The deleted route-layer TASK_ASSIGNMENT duplicate must not reappear.
|
||||||
|
stale = _notifications_for_task(stack, task_id, NotificationType.TASK_ASSIGNMENT)
|
||||||
|
assert stale == [], stale
|
||||||
|
|
||||||
|
|
||||||
|
def test_dependency_revival_persists_alert_notification(e2e_stack: E2EStack) -> None:
|
||||||
|
"""dependency-revival chokepoint: _unblock_dependents ->
|
||||||
|
send_dependency_revival_notification -> ALERT.
|
||||||
|
|
||||||
|
No resolver calls unblock here — a dependent task blocked on another
|
||||||
|
task auto-resumes the moment that dependency's completion clears the
|
||||||
|
last outstanding dependency, at the same ``_unblock_dependents``
|
||||||
|
chokepoint ``TaskService.complete``/``ceo_approve`` call in production.
|
||||||
|
Driven directly against ``TaskService`` (mirrors
|
||||||
|
``arcs.wire_dependency``'s pattern) since there is no bespoke REST
|
||||||
|
endpoint for "a dependency just completed".
|
||||||
|
"""
|
||||||
|
stack = e2e_stack
|
||||||
|
company = seed_company(stack)
|
||||||
|
project_id, _project_slug = seed_project(stack, company)
|
||||||
|
_seed_system_agent(stack)
|
||||||
|
|
||||||
|
from roboco.models.base import TaskStatus
|
||||||
|
|
||||||
|
dependency_id = seed_task(
|
||||||
|
stack,
|
||||||
|
title="Ship the shared auth helper",
|
||||||
|
description="Upstream task the dependent below is blocked on.",
|
||||||
|
acceptance_criteria=["the shared auth helper is merged"],
|
||||||
|
project_id=project_id,
|
||||||
|
created_by=company.cell_pm_id,
|
||||||
|
assigned_to=company.dev_id,
|
||||||
|
status=TaskStatus.IN_PROGRESS,
|
||||||
|
)
|
||||||
|
dependent_id = seed_task(
|
||||||
|
stack,
|
||||||
|
title="Wire the new endpoint to the shared auth helper",
|
||||||
|
description=(
|
||||||
|
"Blocked on the shared auth helper landing; should auto-resume "
|
||||||
|
"the moment that dependency completes, with no resolver acting."
|
||||||
|
),
|
||||||
|
acceptance_criteria=["the endpoint uses the shared auth helper"],
|
||||||
|
project_id=project_id,
|
||||||
|
created_by=company.cell_pm_id,
|
||||||
|
assigned_to=company.dev_id,
|
||||||
|
claimed_by=company.dev_id,
|
||||||
|
status=TaskStatus.BLOCKED,
|
||||||
|
dependency_ids=[dependency_id],
|
||||||
|
branch_name="feature/backend/e2e-dependency-revival",
|
||||||
|
)
|
||||||
|
|
||||||
|
from roboco.services.task import get_task_service
|
||||||
|
|
||||||
|
async def _complete_dependency(session: AsyncSession) -> None:
|
||||||
|
await get_task_service(session)._unblock_dependents(dependency_id)
|
||||||
|
|
||||||
|
stack.run_db(_complete_dependency)
|
||||||
|
|
||||||
|
from roboco.models import NotificationType
|
||||||
|
|
||||||
|
notifications = _notifications_for_task(stack, dependent_id, NotificationType.ALERT)
|
||||||
|
assert len(notifications) == 1, notifications
|
||||||
|
note = notifications[0]
|
||||||
|
assert "alert" in note["type"].lower(), note
|
||||||
|
assert note["related_task_id"] == dependent_id, note
|
||||||
|
assert note["subject"] == f"Task {dependent_id} revived by dependency completion", (
|
||||||
|
note
|
||||||
|
)
|
||||||
|
assert note["priority"], "priority must be populated"
|
||||||
|
assert company.dev_id in note["to_agents"], note
|
||||||
@@ -2023,15 +2023,21 @@ async def test_soft_block_task_success(task_client: dict) -> None:
|
|||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# unblock: success notifies assignee + commits
|
# unblock: success leaves blocked status
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_unblock_task_success_notifies_assignee(
|
async def test_unblock_task_success(
|
||||||
task_client: dict,
|
task_client: dict,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Unblock a blocked task assigned to a different agent → notification path."""
|
"""Unblock a blocked task assigned to a different agent → 200, leaves BLOCKED.
|
||||||
|
|
||||||
|
The unblock notification (ALERT) is sent from TaskService.unblock() itself
|
||||||
|
(roboco/services/notification.py send_unblock_notification) — the route no
|
||||||
|
longer sends a second, duplicate notification, so there is nothing to mock
|
||||||
|
here.
|
||||||
|
"""
|
||||||
other = await _seed_agent(task_client)
|
other = await _seed_agent(task_client)
|
||||||
task = _seed_task(
|
task = _seed_task(
|
||||||
task_client,
|
task_client,
|
||||||
@@ -2040,18 +2046,11 @@ async def test_unblock_task_success_notifies_assignee(
|
|||||||
)
|
)
|
||||||
await task_client["db"].flush()
|
await task_client["db"].flush()
|
||||||
|
|
||||||
with patch(
|
response = await task_client["client"].post(
|
||||||
"roboco.api.routes.tasks.get_notification_delivery_service"
|
f"/api/tasks/{task.id}/unblock", headers=_HDR
|
||||||
) as mock_delivery:
|
)
|
||||||
delivery_instance = AsyncMock()
|
|
||||||
delivery_instance.notify_assignee_of_unblock = AsyncMock(return_value=None)
|
|
||||||
mock_delivery.return_value = delivery_instance
|
|
||||||
response = await task_client["client"].post(
|
|
||||||
f"/api/tasks/{task.id}/unblock", headers=_HDR
|
|
||||||
)
|
|
||||||
assert response.status_code == HTTPStatus.OK
|
assert response.status_code == HTTPStatus.OK
|
||||||
assert response.json()["status"] != "blocked"
|
assert response.json()["status"] != "blocked"
|
||||||
delivery_instance.notify_assignee_of_unblock.assert_awaited_once()
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -412,4 +412,108 @@ async def test_create_notification_requires_ack_derives_from_type(
|
|||||||
assert (
|
assert (
|
||||||
rows[0].requires_ack is ACK_REQUIRED_BY_TYPE[NotificationType.KNOWLEDGE_SHARE]
|
rows[0].requires_ack is ACK_REQUIRED_BY_TYPE[NotificationType.KNOWLEDGE_SHARE]
|
||||||
)
|
)
|
||||||
assert rows[0].requires_ack is False
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Coordination-event producers (reassignment / collision / unblock /
|
||||||
|
# dependency-revival / stale-claim-reaped)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
# previous_assignee + new_assignee + ceo, resolved to UUIDs pre-insert.
|
||||||
|
_REASSIGN_RECIPIENT_COUNT = 3
|
||||||
|
# {task-owner, ceo} for the other four coordination producers.
|
||||||
|
_TWO_RECIPIENT_COUNT = 2
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_reassignment_notification(svc: NotificationService) -> None:
|
||||||
|
aid = uuid4()
|
||||||
|
db = _FakeDb(agent_uuid=aid)
|
||||||
|
with _patch_db_context(db):
|
||||||
|
await svc.send_reassignment_notification(
|
||||||
|
task_id="t1", previous_assignee="be-dev-1", new_assignee="be-dev-2"
|
||||||
|
)
|
||||||
|
rows = [r for r in db.added if r.related_task_id == "t1"]
|
||||||
|
assert rows
|
||||||
|
assert all(len(r.to_agents) == _REASSIGN_RECIPIENT_COUNT for r in rows)
|
||||||
|
assert any("reassigned" in r.subject for r in rows)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_reassignment_notification_no_recipients_is_noop(
|
||||||
|
svc: NotificationService,
|
||||||
|
) -> None:
|
||||||
|
"""All three recipients falsy ⇒ nothing is created (no crash)."""
|
||||||
|
db = _FakeDb()
|
||||||
|
with _patch_db_context(db):
|
||||||
|
await svc.send_reassignment_notification(
|
||||||
|
task_id="t1",
|
||||||
|
previous_assignee=None,
|
||||||
|
new_assignee=None,
|
||||||
|
to_ceo="",
|
||||||
|
)
|
||||||
|
assert db.added == []
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_collision_sequencing_notification(
|
||||||
|
svc: NotificationService,
|
||||||
|
) -> None:
|
||||||
|
aid = uuid4()
|
||||||
|
db = _FakeDb(agent_uuid=aid)
|
||||||
|
with _patch_db_context(db):
|
||||||
|
await svc.send_collision_sequencing_notification(
|
||||||
|
held_back_task_id="t2",
|
||||||
|
blocking_task_id="t1",
|
||||||
|
held_back_assignee="be-dev-1",
|
||||||
|
)
|
||||||
|
rows = [r for r in db.added if r.related_task_id == "t2"]
|
||||||
|
assert rows
|
||||||
|
assert all(len(r.to_agents) == _TWO_RECIPIENT_COUNT for r in rows)
|
||||||
|
assert any("sequenced behind" in r.subject for r in rows)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_unblock_notification(svc: NotificationService) -> None:
|
||||||
|
aid = uuid4()
|
||||||
|
db = _FakeDb(agent_uuid=aid)
|
||||||
|
with _patch_db_context(db):
|
||||||
|
await svc.send_unblock_notification(task_id="t1", restored_owner="be-dev-1")
|
||||||
|
rows = [r for r in db.added if r.related_task_id == "t1"]
|
||||||
|
assert rows
|
||||||
|
assert all(len(r.to_agents) == _TWO_RECIPIENT_COUNT for r in rows)
|
||||||
|
assert any("unblocked" in r.subject for r in rows)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_dependency_revival_notification(
|
||||||
|
svc: NotificationService,
|
||||||
|
) -> None:
|
||||||
|
aid = uuid4()
|
||||||
|
db = _FakeDb(agent_uuid=aid)
|
||||||
|
with _patch_db_context(db):
|
||||||
|
await svc.send_dependency_revival_notification(
|
||||||
|
task_id="t1", assignee="be-dev-1", completed_dependency_id="dep1"
|
||||||
|
)
|
||||||
|
rows = [r for r in db.added if r.related_task_id == "t1"]
|
||||||
|
assert rows
|
||||||
|
assert all(len(r.to_agents) == _TWO_RECIPIENT_COUNT for r in rows)
|
||||||
|
assert any("revived" in r.subject for r in rows)
|
||||||
|
assert any("dep1" in r.body for r in rows)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_stale_claim_reaped_notification(
|
||||||
|
svc: NotificationService,
|
||||||
|
) -> None:
|
||||||
|
aid = uuid4()
|
||||||
|
db = _FakeDb(agent_uuid=aid)
|
||||||
|
with _patch_db_context(db):
|
||||||
|
await svc.send_stale_claim_reaped_notification(
|
||||||
|
task_id="t1", reaped_agent="be-dev-1", last_heartbeat="2026-07-11T00:00:00"
|
||||||
|
)
|
||||||
|
rows = [r for r in db.added if r.related_task_id == "t1"]
|
||||||
|
assert rows
|
||||||
|
assert all(len(r.to_agents) == _TWO_RECIPIENT_COUNT for r in rows)
|
||||||
|
assert any(r.priority == NotificationPriority.HIGH for r in rows)
|
||||||
|
assert any("stale claim reaped" in r.subject for r in rows)
|
||||||
|
|||||||
@@ -461,6 +461,93 @@ async def test_reassign_returns_none_when_task_missing() -> None:
|
|||||||
assert out is None
|
assert out is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_reassign_notifies_once_not_twice_for_same_target() -> None:
|
||||||
|
"""Repeated reassign to the SAME target must not double-fire.
|
||||||
|
|
||||||
|
`reassign` runs every time it's called (even a no-op redirect); the
|
||||||
|
coordination notification is guarded separately by comparing the new
|
||||||
|
assignee against the assignee captured before the mutation, so a second
|
||||||
|
call with an already-current target must skip the notification.
|
||||||
|
"""
|
||||||
|
task = _build_task(assigned_to=None, claimed_by=None)
|
||||||
|
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||||
|
_bind(svc, "get", AsyncMock(return_value=task))
|
||||||
|
new_assignee = uuid4()
|
||||||
|
mock_ns = MagicMock()
|
||||||
|
mock_ns.send_reassignment_notification = AsyncMock()
|
||||||
|
with patch(
|
||||||
|
"roboco.services.notification.NotificationService", return_value=mock_ns
|
||||||
|
):
|
||||||
|
await svc.reassign(task.id, new_assignee)
|
||||||
|
await svc.reassign(task.id, new_assignee)
|
||||||
|
mock_ns.send_reassignment_notification.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_unblock_notifies_once_not_twice_on_repeated_call() -> None:
|
||||||
|
"""Repeated unblock() against the same task must not double-fire.
|
||||||
|
|
||||||
|
`unblock` only mutates + notifies a task whose status is currently
|
||||||
|
BLOCKED; the first call flips it to IN_PROGRESS/PENDING, so a second
|
||||||
|
call against the same task_id short-circuits on the status guard and
|
||||||
|
must not send a second notification.
|
||||||
|
"""
|
||||||
|
raiser = uuid4()
|
||||||
|
task = _build_task(
|
||||||
|
status=TaskStatus.BLOCKED, branch_name=None, blocker_raised_by=raiser
|
||||||
|
)
|
||||||
|
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||||
|
_bind(svc, "get", AsyncMock(return_value=task))
|
||||||
|
_bind(svc, "_index_lifecycle_event_background", AsyncMock())
|
||||||
|
mock_ns = MagicMock()
|
||||||
|
mock_ns.send_unblock_notification = AsyncMock()
|
||||||
|
with patch(
|
||||||
|
"roboco.services.notification.NotificationService", return_value=mock_ns
|
||||||
|
):
|
||||||
|
first = await svc.unblock(task.id)
|
||||||
|
second = await svc.unblock(task.id)
|
||||||
|
assert first is task
|
||||||
|
assert second is None
|
||||||
|
mock_ns.send_unblock_notification.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_wire_sibling_collision_dag_notifies_only_for_new_edges() -> None:
|
||||||
|
"""Collision-sequencing notification fires only for freshly-added edges.
|
||||||
|
|
||||||
|
`add_dependency` returns True only on a new edge; a subsequent wiring
|
||||||
|
pass over the same pair returns False and must skip the notification,
|
||||||
|
so the coordination ALERT cannot double-fire.
|
||||||
|
"""
|
||||||
|
parent_id = uuid4()
|
||||||
|
held_back_id = uuid4()
|
||||||
|
blocking_id = uuid4()
|
||||||
|
held_back = _build_task(id=held_back_id, assigned_to=uuid4())
|
||||||
|
blocking = _build_task(id=blocking_id)
|
||||||
|
svc = TaskService(MagicMock(flush=AsyncMock()))
|
||||||
|
_bind(svc, "get_subtasks", AsyncMock(return_value=[held_back, blocking]))
|
||||||
|
add_dep_mock = AsyncMock(side_effect=[True, False])
|
||||||
|
_bind(svc, "add_dependency", add_dep_mock)
|
||||||
|
mock_ns = MagicMock()
|
||||||
|
mock_ns.send_collision_sequencing_notification = AsyncMock()
|
||||||
|
wiring_passes = 2
|
||||||
|
with (
|
||||||
|
patch(
|
||||||
|
"roboco.services.sequencing.dev_task_collision_edges",
|
||||||
|
return_value=[(blocking_id, held_back_id)],
|
||||||
|
),
|
||||||
|
patch(
|
||||||
|
"roboco.services.notification.NotificationService",
|
||||||
|
return_value=mock_ns,
|
||||||
|
),
|
||||||
|
):
|
||||||
|
for _ in range(wiring_passes):
|
||||||
|
await svc.wire_sibling_collision_dag(parent_id)
|
||||||
|
mock_ns.send_collision_sequencing_notification.assert_awaited_once()
|
||||||
|
assert add_dep_mock.await_count == wiring_passes
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_mark_agent_idle_sets_status_idle() -> None:
|
async def test_mark_agent_idle_sets_status_idle() -> None:
|
||||||
agent = MagicMock(id=uuid4(), status=AgentStatus.ACTIVE)
|
agent = MagicMock(id=uuid4(), status=AgentStatus.ACTIVE)
|
||||||
|
|||||||
Reference in New Issue
Block a user