Totoro b57fa6fb9e
feat(scheduler): push scheduled task results to IM channels via a notification outbox (#6135)
* feat(persistence): add notification delivery outbox for scheduled task IM push

New notification_deliveries table and repository (migration 0012) backing
the outbox pattern for issue #4254: idempotent enqueue keyed on task_run_id
plus event plus provider plus target, atomic claim of due rows, and
exponential-backoff retry state moving pending to sending to sent or failed.
Execution status stays in scheduled_task_runs; delivery status lives here.

* feat(scheduler): enqueue and deliver scheduled-task notifications over IM channels

Closes the loop for issue #4254. On run completion the scheduler writes
outbox rows for the task owner's connected channel identities (success and
failure events; interrupts stay silent), and a new NotificationDeliveryWorker
polls due rows and pushes them through the channel's proactive send path.
Channel gains send_notification; WeCom implements it via the WS client's
send_message with errcode checking, the base class raises for providers
without proactive push.

Delivery is best-effort and never shadows execution status: enqueue and send
failures are logged and recorded in the outbox, which retries with backoff.
Channel liveness is re-checked at delivery time, and the worker stops before
the channel service on shutdown. Feature activates only when channel
connections are enabled; no new dependencies.

Verified end to end against a live WeCom bot: scheduled run completion
produced an outbox row that was delivered in under one second.

* feat(scheduler): include scheduled run result summary in IM notifications

The v1 notification only carried the run outcome skeleton (status, task id,
run id). Completed runs now include the agent's final answer so the IM push
is actually useful headlessly.

- The delivery worker accepts an optional resolve_run_summary(run_id,
  user_id) resolver; for run_completed events it enriches the rendered text
  with a "Result:" line (bounded to 1000 chars). The lookup is best-effort:
  resolver failures or missing summaries degrade to the skeleton text and
  never block or fail the delivery.
- The summary is read at delivery time from runs.last_ai_message (the
  convenience column written on run completion), not snapshotted into the
  outbox payload; retried deliveries re-read the freshest value and the
  outbox row payload stays skeleton-only. The claimed outbox row is never
  mutated in place.
- Gateway wiring scopes the read to the outbox row's owner_user_id so a
  stale row cannot pull another user's run content.
- Failed runs keep the error-only skeleton: a partial answer on a failed
  run would be misleading.

Tests: 9 new worker cases (rendering incl. truncation, sync/async
resolvers, failure/missing fallbacks, no resolver call for failed runs).
Full scheduled + outbox + channels regression green (441 passed), ruff
check + format clean.

* fix(scheduler): address review feedback on notification delivery reliability

Review follow-ups for the scheduled-task IM notification outbox:

1. Crash recovery for "sending" rows: claim now stamps updated_at and the
   worker reconciles rows stuck in "sending" past a 10-minute lease back to
   "pending" before every claim. Per-row try/except isolates a crashing
   delivery from the rest of the claimed batch.
2. Double-send race under multiple workers: claim returns exactly the rows
   its own UPDATE...RETURNING flipped (guarded by status='pending'), so a
   rival worker's concurrent claim can no longer be handed out twice.
3. Restore non-ASCII comments and drop the BOM accidentally introduced in
   test_scheduled_task_service.py.
4. Manual "run now" completions no longer enqueue IM notifications; the
   user is already watching the UI. Missing trigger metadata fails safe
   towards notifying.
5. Channel-outage failures no longer consume the retry budget: they are
   parked with count_attempt=False on a flat 15-minute backoff so an
   hours-long outage cannot exhaust retries and drop the notification.

Tests: repository 16, worker 21, scheduled-task service 31; targeted
regression across notification/scheduler/channel suites: 511 passed.

* fix(scheduler): address round-2 review on IM notification delivery

Park WeCom transport/outage failures without burning retries, bound worker stop, include task titles, align enqueue with delivery, and document the IM push behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(scheduler): complete IM notification docs and drop unused list API

Add backend/channels/migrations AGENTS notes, CHANGELOG and config.example hints for the outbox push path, and remove the unused list_by_task_run repository method flagged in review.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scheduler): address round-3 review on IM notification delivery

Cap channel-down parking by row age and parked attempts, redact IM egress
content, narrow WeCom transport exceptions, add lifespan wiring tests, and
run ruff format so lint-backend passes.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scheduler): redact entire PEM blocks in IM egress text

The previous BEGIN-only pattern left key material and the END marker in notifications.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(persistence): align 0014 parked_attempts with bootstrap schema tests

Backfill existing rows with a temporary server default, then drop it so create_all matches Alembic, and move HEAD anchors to 0014.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(persistence): format notification tests and shorten alembic revision id

ruff format --check failed on test_notification_delivery_worker.py.
0015_notification_delivery_parked_attempts exceeds alembic_version.version_num
(VARCHAR 32) and truncates on Postgres CI; use 0015_parked_attempts instead.

* test: fix post-merge CI failures after migration renumber

Update migration 0015 test HEAD expectation to 0017_parked_attempts and add queue_timeout_seconds to the lifespan notification worker mock scheduler config.

* fix(persistence): chain the notification outbox migrations after 0026

Main's head moved from 0016 to 0026_mcp_task_lease_tokens, so the two
outbox revisions are renumbered and re-parented:

- 0017_notification_deliveries -> 0027_notification_deliveries
- 0018_parked_attempts -> 0028_parked_attempts

Only the revision ids, parents and the migration guide change.

* fix(scheduler): adapt the notification hook and its docs to main

- tests: the completion tests assert on the atomic complete_run record;
  new cases cover a completion that was not recorded, a task deleted
  meanwhile, a failed task read, and the outbox being off.
- AGENTS.md: drop the root note and shorten the backend note to a pointer;
  the two notes pushed two effective guidance chains past the 96 KiB limit.
- CHANGELOG: move the entry from the released 2.1.0 section to Unreleased
  and add its link references.

* test(persistence): move the chain-head pin to the outbox migrations

test_migration_0026 pinned 0026 as the head, so it failed once the outbox
revisions chained after it. It now checks chain membership like the older
migration tests, and a new test file owns the head pin, checks the parents
and the 32-character revision id limit, and runs the upgrade and downgrade.

* fix(gateway): wire scheduled-task notifications after the channel service starts

The outbox wiring sat in the scheduler block and asked for the running
channel service there. #5035 moved the channel service start to after the
scheduler, and the two changes merged without a conflict, so on every
startup the lookup returned None and the feature disabled itself with the
"write-only outbox" warning. The lifespan tests patched the lookup to a
constant and kept passing.

The wiring now runs in its own step after the channel service start. The
scheduler is built as on main, and ScheduledTaskService gains
attach_notification_outbox() so the enqueue side is enabled once the
delivery worker is running. The guard, the worker and the shutdown order
are unchanged.

Tests: the lifespan helper can model the real accessor (None until the
channel service has started); a new case fails without this fix.

* fix(scheduler): enqueue from scheduler start and keep the push when the title read fails

Follow-up to the startup-order fix, from review.

- The enqueue side is wired with the scheduler again (it only needs the
  durable table), and only the delivery worker waits for the channel
  service. Attaching it later left a window after the scheduler started in
  which a finished run wrote no outbox row. If the channel service is
  missing or the worker cannot start, the enqueue side is detached, so the
  outbox still never becomes write-only.
- A failed task read after complete_run no longer drops the notification:
  the title is optional and the message falls back to the task id. A task
  deleted meanwhile still stays silent.

* fix(channels): park WeCom notifications while the SDK socket is closed

The WeCom SDK raises a plain RuntimeError when its socket is not open, and
send_notification only mapped ConnectionError, TimeoutError and OSError to
ChannelUnavailable. A disconnect therefore spent the counted retry budget
and settled the row as failed after about 15 minutes, although the docs
say transport outages park without consuming retries.

send_notification now asks the client's is_connected first. The new test
uses a real, never-connected SDK client: it raised RuntimeError after three
retries before, and parks now.

* docs(scheduler): describe which outcomes notify; mirror the changelog entry

- README (en/zh) and CONFIGURATION.md: only runs that finish as success or
  failed notify; occurrences that end without a finished run (launch error,
  queue timeout, restart recovery) do not. Parking lasts up to about a day.
- CHANGELOG: cite the issue as text like the rest of the file, and add the
  entry and link reference to CHANGELOG_zh.md.
- AGENTS.md: drop the backend pointer so this branch adds nothing to the
  guidance chains closest to their size limit; the scheduler-side rules
  and the lifespan order now live in app/channels/AGENTS.md.

* docs(changelog): cite #6135 next to #4843 for the notification outbox entry

* fix(scheduler,channels): re-check the IM binding at delivery time; park WeCom transport errors, count its rejections

Review round 1 on #6135.

The delivery worker now takes resolve_connections (the Gateway wires
ChannelConnectionRepository.list_connections) and, before the channel
liveness check and the summary read, requires the row's owner to still
hold a connected (provider, external_account_id). A disconnected target
is finalized with mark_failed(terminal=True); a lookup failure or an
unexpected result shape parks the row. Terminal outbox rows ignore
later status writes.

WeCom: the SDK raises RuntimeError from send_message for a socket that
is not open and for a dropped connection, but also for a platform
rejection ("Reply ack error: errcode=..."). Only that prefix keeps
counting; other RuntimeErrors, ConnectionError/TimeoutError/OSError and
websockets' ConnectionClosed park the row as ChannelUnavailable.

* fix(channels): treat a WeCom ACK rejection as proof the transport is up; pin the rejection prefix through the retry branch and the real connection-row shape

---------

Co-authored-by: GaoTianyuanhr <75788039+GaoTianyuanhr@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: GaoTianyuanhr <GaoTianyuanhr@users.noreply.github.com>
Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com>
2026-10-01 17:59:30 +08:00

422 lines
17 KiB
Python

"""Abstract base class for IM channels."""
from __future__ import annotations
import asyncio
import logging
import threading
from abc import ABC, abstractmethod
from collections.abc import Awaitable, Callable, Coroutine
from concurrent.futures import CancelledError as FutureCancelledError
from concurrent.futures import Future
from dataclasses import dataclass
from typing import Any, TypeVar
from app.channels.commands import extract_connect_code
from app.channels.message_bus import (
InboundMessage,
InboundMessageType,
InboundQueueClosedError,
InboundQueueFullError,
InboundReservation,
InboundReservationExpiredError,
MessageBus,
OutboundMessage,
ResolvedAttachment,
)
logger = logging.getLogger(__name__)
T = TypeVar("T")
class ChannelUnavailable(Exception):
"""Local connectivity failure: the channel cannot accept traffic right now.
Distinct from a platform rejection of a well-formed send. Delivery workers
park outbox rows with ``count_attempt=False`` on this error so a transport
outage cannot exhaust the retry budget.
"""
@dataclass(eq=False, slots=True)
class _ThreadsafeSubmission:
coroutine: Coroutine[Any, Any, Any]
loop: asyncio.AbstractEventLoop
name: str
msg_id: Any
reservation: InboundReservation | None
completion: Future[Any]
task: asyncio.Task[Any] | None = None
cancel_requested: bool = False
class Channel(ABC):
"""Base class for all IM channel implementations.
Each channel connects to an external messaging platform and:
1. Receives messages, wraps them as InboundMessage, publishes to the bus.
2. Subscribes to outbound messages and sends replies back to the platform.
Subclasses must implement ``start``, ``stop``, and ``send``.
"""
def __init__(self, name: str, bus: MessageBus, config: dict[str, Any]) -> None:
self.name = name
self.bus = bus
self.config = config
self._running = False
self._connection_repo: Any = config.get("connection_repo")
# Provider SDK callbacks often run on a dedicated thread and submit
# preparation work to the Gateway loop. Submission and shutdown share
# this lock so stop() cannot miss a future created concurrently.
self._threadsafe_submissions: set[_ThreadsafeSubmission] = set()
self._threadsafe_submissions_lock = threading.Lock()
self._threadsafe_submission_intake_open = True
@property
def is_running(self) -> bool:
return self._running
@property
def supports_streaming(self) -> bool:
return False
# -- lifecycle ---------------------------------------------------------
@abstractmethod
async def start(self) -> None:
"""Start listening for messages from the external platform."""
@abstractmethod
async def stop(self) -> None:
"""Gracefully stop the channel."""
# -- outbound ----------------------------------------------------------
@abstractmethod
async def send(self, msg: OutboundMessage) -> None:
"""Send a message back to the external platform.
The implementation should use ``msg.chat_id`` and ``msg.thread_ts``
to route the reply to the correct conversation/thread.
"""
async def send_file(self, msg: OutboundMessage, attachment: ResolvedAttachment) -> bool:
"""Upload a single file attachment to the platform.
Returns True if the upload succeeded, False otherwise.
Default implementation returns False (no file upload support).
"""
return False
async def send_notification(self, *, target: str, text_markdown: str) -> None:
"""Push a proactive notification to an external identity (issue #4254).
Unlike ``send``, there is no inbound frame to reply to: *target* is a
platform identity (e.g. the ``external_account_id`` recorded by the
bind flow). Used by the notification delivery worker to deliver
scheduled-task outcomes. Channels without proactive push support keep
the default, which raises so the delivery outbox records a failure
instead of silently dropping the notification.
Connectivity preconditions should raise :class:`ChannelUnavailable`
(not a generic ``RuntimeError``) so the outbox can park the row without
consuming its retry budget. Platform rejections of a well-formed send
remain ordinary exceptions that count as attempts.
"""
if not self.is_running:
raise ChannelUnavailable(f"channel '{self.name}' is not running")
raise NotImplementedError(f"channel '{self.name}' does not support proactive notifications")
# -- helpers -----------------------------------------------------------
async def _send_with_retry(
self,
operation: Callable[[], Awaitable[T]],
*,
max_retries: int,
log_prefix: str | None = None,
operation_name: str = "send",
) -> T:
"""Run an outbound send operation with the shared channel retry policy."""
prefix = log_prefix or f"[{self.name}]"
last_exc: Exception | None = None
for attempt in range(max_retries):
try:
return await operation()
except Exception as exc:
last_exc = exc
if attempt < max_retries - 1:
delay = 2**attempt
logger.warning(
"%s %s failed (attempt %d/%d), retrying in %ds: %s",
prefix,
operation_name,
attempt + 1,
max_retries,
delay,
exc,
)
await asyncio.sleep(delay)
logger.error("%s %s failed after %d attempts: %s", prefix, operation_name, max_retries, last_exc)
if last_exc is None:
raise RuntimeError(f"{self.name} {operation_name} failed without an exception from any attempt")
raise last_exc
def _log_future_error(self, fut: Any, name: str, msg_id: Any) -> None:
"""Callback for concurrent futures scheduled from channel worker threads."""
try:
exc = fut.exception()
except (asyncio.CancelledError, FutureCancelledError, asyncio.InvalidStateError):
return
except Exception:
logger.exception("[%s] failed to inspect future for %s (msg_id=%s)", self.name, name, msg_id)
return
if exc:
logger.error("[%s] %s failed for msg_id=%s: %s", self.name, name, msg_id, exc)
def _open_threadsafe_future_intake(self) -> None:
"""Allow a newly started provider to submit work to its main loop."""
with self._threadsafe_submissions_lock:
if self._threadsafe_submissions:
raise RuntimeError(f"cannot restart {self.name} while cross-thread work is still running")
self._threadsafe_submission_intake_open = True
def _submit_threadsafe_coroutine(
self,
coroutine: Coroutine[Any, Any, T],
loop: asyncio.AbstractEventLoop | None,
*,
name: str,
msg_id: Any,
reservation: InboundReservation | None = None,
) -> bool:
"""Submit provider-thread work while retaining its real asyncio Task."""
with self._threadsafe_submissions_lock:
if not self._threadsafe_submission_intake_open or loop is None or not loop.is_running():
coroutine.close()
if reservation is not None:
reservation.release()
return False
submission = _ThreadsafeSubmission(
coroutine=coroutine,
loop=loop,
name=name,
msg_id=msg_id,
reservation=reservation,
completion=Future(),
)
self._threadsafe_submissions.add(submission)
try:
loop.call_soon_threadsafe(self._start_threadsafe_submission, submission)
except RuntimeError:
self._threadsafe_submissions.discard(submission)
coroutine.close()
if reservation is not None:
reservation.release()
return False
return True
def _start_threadsafe_submission(self, submission: _ThreadsafeSubmission) -> None:
"""Create the owned Task on its event loop or finish a pre-start cancel."""
task: asyncio.Task[Any] | None = None
startup_error: BaseException | None = None
with self._threadsafe_submissions_lock:
if submission.cancel_requested:
self._threadsafe_submissions.discard(submission)
cancelled_before_start = True
else:
cancelled_before_start = False
try:
task = submission.loop.create_task(submission.coroutine)
except BaseException as exc:
self._threadsafe_submissions.discard(submission)
startup_error = exc
else:
submission.task = task
if cancelled_before_start:
submission.coroutine.close()
if submission.reservation is not None:
submission.reservation.release()
submission.completion.cancel()
return
if startup_error is not None:
submission.coroutine.close()
if submission.reservation is not None:
submission.reservation.release()
submission.completion.set_exception(startup_error)
self._log_future_error(submission.completion, submission.name, submission.msg_id)
return
assert task is not None
task.add_done_callback(lambda completed: self._finalize_threadsafe_submission(submission, completed))
def _finalize_threadsafe_submission(
self,
submission: _ThreadsafeSubmission,
task: asyncio.Task[Any],
) -> None:
with self._threadsafe_submissions_lock:
self._threadsafe_submissions.discard(submission)
if submission.reservation is not None:
submission.reservation.release()
if task.cancelled():
submission.completion.cancel()
else:
try:
submission.completion.set_result(task.result())
except BaseException as exc:
submission.completion.set_exception(exc)
self._log_future_error(submission.completion, submission.name, submission.msg_id)
async def _close_and_drain_threadsafe_futures(self) -> None:
"""Close submission, then cancel and await the owned asyncio Tasks."""
with self._threadsafe_submissions_lock:
self._threadsafe_submission_intake_open = False
submissions = tuple(self._threadsafe_submissions)
tasks: list[tuple[asyncio.AbstractEventLoop, asyncio.Task[Any]]] = []
for submission in submissions:
submission.cancel_requested = True
if submission.task is not None:
tasks.append((submission.loop, submission.task))
for loop, task in tasks:
try:
loop.call_soon_threadsafe(task.cancel)
except RuntimeError:
logger.warning("[%s] event loop closed before cross-thread task cancellation", self.name)
if submissions:
await asyncio.gather(
*(asyncio.shield(asyncio.wrap_future(submission.completion)) for submission in submissions),
return_exceptions=True,
)
def _pending_connect_code(self, text: str) -> str | None:
"""Return the one-time bind code if *text* is a ``/connect <code>`` command
and channel connections are configured, else ``None``.
Adapters MUST consult this **before** applying their ``allowed_users`` /
``_check_user`` gate, so a browser-initiated bind can bootstrap an external
identity that the platform bot has never seen and is therefore not yet
authorized. (Telegram uses its deep-link ``/start <token>`` flow instead.)
"""
if self._connection_repo is None:
return None
return extract_connect_code(text)
def _make_inbound(
self,
chat_id: str,
user_id: str,
text: str,
*,
msg_type: InboundMessageType = InboundMessageType.CHAT,
thread_ts: str | None = None,
files: list[dict[str, Any]] | None = None,
metadata: dict[str, Any] | None = None,
) -> InboundMessage:
"""Convenience factory for creating InboundMessage instances."""
return InboundMessage(
channel_name=self.name,
chat_id=chat_id,
user_id=user_id,
text=text,
msg_type=msg_type,
thread_ts=thread_ts,
files=files or [],
metadata=metadata or {},
)
def _reserve_inbound(self, msg: InboundMessage) -> InboundReservation | None:
"""Reserve bounded intake capacity or explicitly drop under overload.
Real-time socket/polling providers do not expose a reliable delivery
retry contract to this adapter layer. They therefore drop a message
that cannot be admitted immediately. ``MessageBus`` emits a
rate-limited warning with the cumulative rejection count.
"""
try:
return self.bus.reserve_inbound(msg)
except InboundQueueFullError:
return None
except (InboundQueueClosedError, InboundReservationExpiredError):
logger.debug("[%s] inbound ignored because channel intake is closed", self.name)
return None
def _commit_reserved_inbound(
self,
reservation: InboundReservation,
msg: InboundMessage,
) -> bool:
"""Commit a reservation on the MessageBus loop, releasing on failure."""
try:
reservation.commit(msg)
return True
except (InboundQueueClosedError, InboundReservationExpiredError):
logger.debug("[%s] inbound reservation expired during shutdown", self.name)
return False
finally:
reservation.release()
async def _publish_inbound_or_drop(self, msg: InboundMessage) -> bool:
"""Publish from an already-serialized provider loop without waiting."""
try:
await self.bus.publish_inbound(msg)
return True
except InboundQueueFullError:
return False
except InboundQueueClosedError:
logger.debug("[%s] inbound ignored because channel intake is closed", self.name)
return False
async def _on_outbound(self, msg: OutboundMessage) -> None:
"""Outbound callback registered with the bus.
Only forwards messages targeted at this channel.
Sends the text message first, then uploads any file attachments.
File uploads are skipped entirely when the text send fails to avoid
partial deliveries (files without accompanying text).
"""
if msg.channel_name == self.name:
try:
await self.send(msg)
except Exception:
logger.exception("Failed to send outbound message on channel %s", self.name)
return # Do not attempt file uploads when the text message failed
for attachment in msg.attachments:
try:
success = await self.send_file(msg, attachment)
if not success:
logger.warning("[%s] file upload skipped for %s", self.name, attachment.filename)
except Exception:
logger.exception("[%s] failed to upload file %s", self.name, attachment.filename)
async def receive_file(self, msg: InboundMessage, thread_id: str, *, user_id: str | None = None) -> InboundMessage:
"""
Optionally process and materialize inbound file attachments for this channel.
By default, this method does nothing and simply returns the original message.
Subclasses (e.g. FeishuChannel) may override this to download files (images, documents, etc)
referenced in msg.files, save them to the sandbox, and update msg.text to include
the sandbox file paths for downstream model consumption.
Args:
msg: The inbound message, possibly containing file metadata in msg.files.
thread_id: The resolved DeerFlow thread ID for sandbox path context.
user_id: Optional DeerFlow storage user ID for user-scoped channel workers.
Returns:
The (possibly modified) InboundMessage, with text and/or files updated as needed.
"""
del user_id
return msg