feat: Telegram V2 — inbound commands + actionable approve/reject from chat (#551)

* feat(telegram): V2 inbound — command router, actionable approve/reject keyboards, chat-gated poll loop

* fix(release,x,video,telegram): terminal-state guards on approve/reject; sender-identity check

* docs(map,rag): Telegram V2 inbound surfaces and terminal-state approve/reject guards

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
Renzo F
2026-07-18 00:47:09 +02:00
committed by GitHub
co-authored by Renn F
parent c9c1f62981
commit 9fbec78126
30 changed files with 2732 additions and 68 deletions
+28 -3
View File
@@ -1,5 +1,5 @@
## Purpose
This slice implements RoboCo's formal-notification backbone: NotificationService is the typed notification factory (blocker, QA-ready, A2A, board-review, ack), NotificationDeliveryService handles delivery (transactional-outbox bus publish), ACK tracking, expiry sweeps, and PM/CEO task-handoff notifications, and notification_dedup is a bounded Redis SET-NX re-fire guard for loop-prone notification types. Together they turn lifecycle events into both a durable DB record and a real-time push, with multiple dedup layers (Redis re-fire window + DB purpose-dedup) to keep agent inboxes from flooding under coordinator loops.
This slice implements RoboCo's formal-notification backbone: NotificationService is the typed notification factory (blocker, QA-ready, A2A, board-review, ack), NotificationDeliveryService handles delivery (transactional-outbox bus publish), ACK tracking, expiry sweeps, PM/CEO task-handoff notifications, and the best-effort Telegram DM bridge (`_notify_telegram`), and notification_dedup is a bounded Redis SET-NX re-fire guard for loop-prone notification types. Together they turn lifecycle events into both a durable DB record and a real-time push, with multiple dedup layers (Redis re-fire window + DB purpose-dedup) to keep agent inboxes from flooding under coordinator loops. The Telegram side has grown into its own two-way bridge: V1 (outbound-only DMs on escalation/completion) plus V2's `TelegramInboundEngine` (`telegram_inbound.py`) — a poll loop that turns the CEO's Telegram replies/button-taps into the same CEO-gated service calls the HTTP routes make.
## Files
@@ -7,7 +7,10 @@ This slice implements RoboCo's formal-notification backbone: NotificationService
|---|---|---|
| roboco/services/notification.py | Typed notification factory (blocker/QA/docs/handoff/A2A/board-review/ack) with slug→UUID recipient resolution, DB purpose-dedup + Redis re-fire guard, owns its own DB context and commit | 943 |
| roboco/services/notification_dedup.py | Bounded Redis SET-NX re-fire guard for loop-prone notification types (TASK_ASSIGNMENT/REVIEW_REQUEST/DOCUMENTATION_REQUEST/BROADCAST); 60s TTL, fail-open | 91 |
| roboco/services/notification_delivery.py | Delivery (transactional-outbox deferred bus publish), ACK/read tracking, expiry sweep, PM/CEO task-handoff notifications (notify_pm_of_block, escalate_and_notify, etc.), API-facing list/CRUD | 1034 |
| roboco/services/notification_delivery.py | Delivery (transactional-outbox deferred bus publish), ACK/read tracking, expiry sweep, PM/CEO task-handoff notifications (notify_pm_of_block, escalate_and_notify, etc.), API-facing list/CRUD, `_notify_telegram` best-effort CEO DM fan-out (V2: `actionable=True` on escalation attaches an Approve/Reject/Open inline keyboard) | 1305 |
| roboco/services/telegram_client.py | Bot API client ABC + `NullTelegramClient` (unconfigured, never egresses) + `LiveTelegramClient`: `send_message` (reply_markup/reply_to_message_id), V2 additions `get_updates` (long-poll), `answer_callback_query`, `edit_message_reply_markup`, `edit_message_text` | 247 |
| roboco/services/telegram_credentials.py | Singleton Fernet-encrypted `bot_token`/`chat_id` CRUD (mirrors `x_credentials.py`); decrypts server-side only, API returns `has_credentials` only | 109 |
| roboco/services/telegram_inbound.py | V2: `TelegramInboundEngine` — getUpdates poll cycle (offset persisted as `telegram_last_update_id` in system_settings), chat-id AND sender-id authorization, `/status` `/queue` `/task` command router, `apv|rej:<kind>:<id8>` callback codec, force_reply reject/approve-notes state machine (in-memory `_PENDING_REPLIES`, TTL), per-kind dispatch to the SAME service methods the CEO-gated HTTP routes call (task/release/xpost/video/roadmap), `via=telegram` audit rows | 858 |
## Data Flow
Two create-and-deliver paths exist. (A) NotificationService._create_notification (notification.py) opens its OWN get_db_context, resolves sender + recipients to UUIDs via _resolve_agent_uuid, runs the Redis re-fire guard (all_recipients_recently_notified), then DB purpose-dedup (ack-required types only, same sender+type+task+overlapping recipients not yet acked), builds NotificationTable with requires_ack from ACK_REQUIRED_BY_TYPE, flushes, calls NotificationDeliveryService.deliver (which defers NOTIFICATION_SENT bus events to after_commit), and finally commits — the commit triggers the deferred bus drain. (B) NotificationDeliveryService._persist_and_deliver (notification_delivery.py) is used by the task-handoff helpers (notify_pm_of_block, escalate_and_notify, etc.): it runs inside the CALLER's open transaction, applies only the Redis re-fire guard (no DB purpose-dedup), adds+flushes+delivers, and leaves the commit to the caller (api/routes/tasks.py). Sweeper loops in the orchestrator call sweep_expired_notifications periodically. Real-time push: deliver defers per-recipient NOTIFICATION_SENT events; the after_commit listener schedules _drain_pending_publishes which publishes to the StreamEventBus; websocket_bridge forwards to /ws/notifications/{id} sockets. ACKs flow acknowledge → acked_by/read_by mutation + NOTIFICATION_ACKED event.
@@ -80,12 +83,25 @@ notification
│ ├── notify_auditor_of_rework (ALERT to the auditor agent on needs_revision)
│ └── _persist_and_deliver (re-fire guard only, caller commits)
├── Recipient helpers: _resolve_team_pm / _resolve_pm_for_agent_or_team / _get_agent_by_id/slug / _get_ceo_agent / _get_auditor_agent
── API-facing: list_system_notifications / list_for_agent / get_for_recipient_and_mark_read / acknowledge_for_recipient / mark_read_for_recipient
── API-facing: list_system_notifications / list_for_agent / get_for_recipient_and_mark_read / acknowledge_for_recipient / mark_read_for_recipient
└── _notify_telegram (best-effort CEO DM; actionable=True on escalation attaches build_action_keyboard)
telegram_client.py (TelegramClient ABC / NullTelegramClient / LiveTelegramClient)
├── V1: send_message (reply_markup, reply_to_message_id)
└── V2: get_updates (long-poll) / answer_callback_query / edit_message_reply_markup / edit_message_text
telegram_inbound.py (TelegramInboundEngine, V2)
├── run_cycle (getUpdates offset cursor, dispatch each update, advance+persist offset)
├── _handle_message (chat+sender auth, force_reply pending-consume, command dispatch)
├── _dispatch_command (/status /queue /task /help)
├── _handle_callback (chat+sender auth, parse_callback, needs_reply branch → _prompt_for_reply, else _dispatch_approve)
├── _dispatch_approve / _dispatch_reject (per-kind handler dict: task/release/xpost/video/roadmap)
│ └── each handler calls the SAME service method the CEO-gated HTTP route calls; _mark_audit stamps a via=telegram AuditLogTable row
└── _finish_action (clears the buttoned message's keyboard, stamps the outcome)
```
## Dependencies
- Internal: roboco.config.settings (redis_url), roboco.db.tables (NotificationTable, AgentTable, TaskTable), roboco.db.base.get_db_context, roboco.events (Event, EventType, get_event_bus), roboco.foundation.policy.communications.ACK_REQUIRED_BY_TYPE, roboco.models.base (NotificationPriority, NotificationType, AgentRole), roboco.models.notification.CreateNotificationParams, roboco.services.base (BaseService, ConflictError, NotFoundError), roboco.services.permissions.has_privileged_access, roboco.services.repositories.get_agent_slug, roboco.agents_config (get_escalation_target, get_pm_for_agent, get_pm_for_team), roboco.utils.converters (require_uuid, to_python_uuid)
- External: sqlalchemy (select, and_, or_, func, event, joinedload, selectinload, with_for_update, IntegrityError, AsyncSession), redis.asyncio (from_url, set NX EX, aclose), asyncio (create_task, get_running_loop), structlog, datetime (UTC, datetime, timedelta), uuid.UUID, dataclasses
- telegram_inbound.py additionally imports: roboco.services.release_proposal (dispatch_approve, get_release_proposal_service, TaskAlreadyCompletedError), roboco.services.roadmap_service, roboco.services.task.get_task_service, roboco.services.telegram_credentials, roboco.services.tiktok_client/tiktok_credentials, roboco.services.video_post_service (VideoPostService, TaskAlreadyCompletedError, VideoCaptionTooLongError), roboco.services.x_credentials, roboco.services.x_post_service (TaskAlreadyCompletedError, XPostBodyTooLongError), roboco.services.x_video_client, roboco.foundation.policy.content.validators.reject_trivial, roboco.seeds.initial_data.AGENT_UUIDS
## Entry Points
@@ -96,9 +112,13 @@ notification
| NotificationDeliveryService.notify_pm_of_block / escalate_and_notify / notify_ceo_of_escalation / notify_auditor_of_rework | roboco/services/notification_delivery.py | api/routes/tasks.py i_am_blocked / escalate / ceo-approval routes; TaskService._alert_auditor_of_rework at QA-fail / rework chokepoints |
| NotificationDeliveryService.acknowledge / list_for_agent / get_for_recipient_and_mark_read | roboco/services/notification_delivery.py | api/routes/notifications.py ACK + list endpoints |
| sweep_expired_notifications | roboco/services/notification_delivery.py | orchestrator periodic loop (orchestrator.py:5780) |
| TelegramInboundEngine.run_cycle | roboco/services/telegram_inbound.py | orchestrator `_telegram_poll_loop` (default off, `telegram_enabled` AND `telegram_inbound_enabled`) |
## Config Flags
- settings.redis_url — Redis URL used by notification_dedup for the SET-NX re-fire guard (derived from ROBOCO_REDIS_HOST/_PORT)
- `telegram_enabled` (default off) — V1 master switch; `_notify_telegram` no-ops without it AND stored credentials.
- `telegram_inbound_enabled` (default off, sub-switch on top of `telegram_enabled`) — V2: arms `TelegramInboundEngine.run_cycle` (the poll loop) and makes escalation DMs carry an actionable keyboard; with it off the bot only sends, never listens, and any inline button on an old message is inert.
- `telegram_poll_interval_seconds` (5.0) / `telegram_poll_timeout_seconds` (25, Bot API long-poll `timeout`) / `telegram_max_updates_per_cycle` (50) / `telegram_pending_reply_ttl_seconds` (300) — V2 poll-loop tuning.
## Gotchas
@@ -112,6 +132,10 @@ notification
- acknowledge publishes NOTIFICATION_ACKED directly to the bus (NOT deferred via after_commit) — unlike deliver. An ACK that is rolled back after publish could emit a phantom ACK event. The ACK path does not use the transactional outbox.
- list_system_notifications filters pending_ack_only POST-fetch because 'not fully acked' is not SQL-friendly on PostgreSQL array columns. For pending_ack_only=True the SQL `limit` is NOT applied — applying it before the Python filter let a window of newer fully-acked rows mask older unacked ones the operator still needs to act on (correctness bug fixed in 115061f3); the full ack-required set is fetched ordered newest-first, Python-filtered to unacked, then sliced to `limit`. The non-pending branch retains the SQL limit.
- get_notification_count loads ALL notifications for an agent into memory (no SQL count) to compute total/unread/pending_ack — O(n) per call, no pagination.
- `TelegramInboundEngine._PENDING_REPLIES` (a force_reply prompt awaiting the CEO's free-text reply) is a per-process, in-memory dict keyed by `(chat_id, prompt_message_id)` — not durable. An orchestrator restart drops any in-flight prompt; the CEO just taps the button again. TTL-swept both lazily (on the next prompt) and on expiry at consume-time.
- `_authorized_chat` (chat id must equal the stored credentials' chat id) is the ONLY identity check a Telegram update carries — there is no agent/session token — so it stands in for every CEO-gated route's `require_ceo_role`. `_authorized_sender` (added in the same wave that added `_authorized_chat`'s callers) is defense-in-depth on top of it: when the update carries a `from` user, its id must ALSO equal the chat id (the supported deployment is a private 1:1 chat); a present-but-mismatched sender is refused, an absent one keeps chat-id-only behavior.
- The getUpdates offset cursor reuses the existing `system_settings` KV store (`telegram_last_update_id`, validated as a non-negative int) rather than a new table/migration — a restart resumes from the last-committed offset instead of replaying processed updates.
- `_dispatch_approve`'s `_approve_release` handler must pre-check the proposal's terminal state itself before calling `dispatch_approve` — that function fires the ~40min release execute as a background task and returns immediately with nothing to inspect, so a stale Approve on an already-rejected/published proposal would otherwise report a false "dispatched" success while the service's own guard silently no-ops it.
## Drift from CLAUDE.md
@@ -129,6 +153,7 @@ notification
| 3aff6e04 | Chore: Close gaps (#285) — follow-on gap closure touching notification.py / notification_dedup.py / notification_delivery.py | Refinement of the #283 changes (exact hunks not isolated per-file in this merge commit; consolidated the dedup/outbox behavior above) |
> Post-snapshot updates (since 2026-06-29): 115061f3 fixed list_system_notifications pending_ack_only correctness: SQL limit is now dropped for that branch so newer fully-acked rows can't mask older unacked ones (see Gotcha update above). `61e00832` (PR #492) added `notify_auditor_of_rework()` and `_get_auditor_agent()` to power the reactive auditor dispatch path: HIGH-priority ALERT notifications addressed to the auditor agent are emitted when a task enters `needs_revision` via QA/PR/PM rework chokepoints. **Wave 3** (2026-07-17, PR #547): `CreateNotificationParams` gains `requires_ack: bool | None = None`, consulted in `_create_notification` ahead of the `ACK_REQUIRED_BY_TYPE` default; `send_a2a_notification` gains a `requires_ack: bool = False` kwarg (plus an `str | None` `task_id`, for a conversational DM with no task behind it) that threads through — the only caller passing True is `A2AService._maybe_wake_ceo_recipient` (docs/map/a2a-audit-journal-permissions.md), so its wake row is finally visible to the orchestrator's `_dispatch_a2a_work` `pending_ack_only` poll.
> `3b9fd0e0`+`11915f36` (PR #551, Telegram V2): `3b9fd0e0` adds `telegram_inbound.py` (new file, `TelegramInboundEngine`), extends `telegram_client.py` with `get_updates`/`answer_callback_query`/`edit_message_reply_markup`/`edit_message_text`, adds `actionable=True` to `_notify_telegram` (escalation only) so the DM carries an Approve/Reject/Open keyboard, and wires the orchestrator's `_telegram_poll_loop`. `11915f36` closes a live-reproduced approve-after-reject hole reachable via a stale Telegram button (or the pre-existing HTTP routes for X/video): `ReleaseProposalService.approve()` now refuses CANCELLED (`already_rejected`) and COMPLETED (`already_published`) proposals via a new `_approve_precheck`, `.reject()` refuses COMPLETED by raising a new `TaskAlreadyCompletedError`, and `XPostService`/`VideoPostService.approve()` each add a CANCELLED pre-lock-and-under-lock guard returning `already_rejected`. Also adds `_authorized_sender` (chat-id auth is defense-in-depth'd with a sender-id check) and widens `_resolve_task`'s search limit 10→50 so a genuine id-prefix hit can't be pushed out by newer title/description matches.
## Regression Risks