From eb20d88b6bae6fd73b71ee1d2414ee549b8d70c2 Mon Sep 17 00:00:00 2001 From: ahmad-ajmal Date: Thu, 11 Jun 2026 05:51:52 +0100 Subject: [PATCH 1/5] Durable trigger store --- agent_core/core/impl/trigger/listener.py | 40 ++ agent_core/core/impl/trigger/queue.py | 46 ++- agent_core/core/trigger.py | 12 +- app/agent_base.py | 84 +++-- app/scheduler/manager.py | 123 +++++-- app/triggers/__init__.py | 28 ++ app/triggers/service.py | 286 +++++++++++++++ app/triggers/sources.py | 46 +++ app/triggers/store.py | 441 +++++++++++++++++++++++ app/ui_layer/controller/ui_controller.py | 22 +- app/usage/session_storage.py | 4 +- tests/e2e/_harness/helpers.py | 11 +- tests/test_trigger_service.py | 337 +++++++++++++++++ tests/test_trigger_store.py | 189 ++++++++++ 14 files changed, 1610 insertions(+), 59 deletions(-) create mode 100644 agent_core/core/impl/trigger/listener.py create mode 100644 app/triggers/__init__.py create mode 100644 app/triggers/service.py create mode 100644 app/triggers/sources.py create mode 100644 app/triggers/store.py create mode 100644 tests/test_trigger_service.py create mode 100644 tests/test_trigger_store.py diff --git a/agent_core/core/impl/trigger/listener.py b/agent_core/core/impl/trigger/listener.py new file mode 100644 index 00000000..09703675 --- /dev/null +++ b/agent_core/core/impl/trigger/listener.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +""" +core.impl.trigger.listener + +Lifecycle listener protocol for TriggerQueue. + +The queue is an in-memory ordering primitive; when it discards triggers +(same-session "prefer newest" replacement, session removal, full clear) a +durable store layered on top must be told so the corresponding rows are +settled instead of silently resurrecting on the next rehydration. The queue +only knows this protocol — the store implementation lives in the app layer. +""" + +from __future__ import annotations + +from typing import List, Optional, Protocol, runtime_checkable + +from agent_core.core.trigger import Trigger + + +@runtime_checkable +class TriggerLifecycleListener(Protocol): + """Receives notifications when the queue discards triggers. + + Implementations must be synchronous and non-raising; the queue calls + them while holding its internal lock. + """ + + def on_evicted( + self, evicted: List[Trigger], replacement: Optional[Trigger] + ) -> None: + """Triggers were removed from the queue without being consumed. + + Args: + evicted: The discarded triggers. + replacement: The newer same-session trigger that superseded them, + or None when they were removed outright (session cleanup, + queue clear). + """ + ... diff --git a/agent_core/core/impl/trigger/queue.py b/agent_core/core/impl/trigger/queue.py index 54bed65f..ef74aaae 100644 --- a/agent_core/core/impl/trigger/queue.py +++ b/agent_core/core/impl/trigger/queue.py @@ -22,6 +22,7 @@ if TYPE_CHECKING: from agent_core.core.protocols import LLMInterfaceProtocol, TaskManagerProtocol from agent_core.core.task import Task + from agent_core.core.impl.trigger.listener import TriggerLifecycleListener # TaskManager type alias for backwards compatibility TaskManager = TaskManagerProtocol @@ -73,6 +74,32 @@ def __init__( self._route_to_session_prompt = route_to_session_prompt self._task_manager = task_manager self._event_stream_manager = event_stream_manager + self._lifecycle_listener: Optional["TriggerLifecycleListener"] = None + + def set_lifecycle_listener( + self, listener: Optional["TriggerLifecycleListener"] + ) -> None: + """Register a listener notified when triggers are discarded unconsumed. + + Used by the durable trigger store to settle rows for triggers the + queue drops (same-session replacement, session removal, clear) so + they don't rehydrate on the next boot. + + Args: + listener: The listener, or None to detach. + """ + self._lifecycle_listener = listener + + def _notify_evicted( + self, evicted: List[Trigger], replacement: Optional[Trigger] + ) -> None: + """Notify the lifecycle listener, swallowing listener errors.""" + if not self._lifecycle_listener or not evicted: + return + try: + self._lifecycle_listener.on_evicted(evicted, replacement) + except Exception as e: + logger.warning(f"[TRIGGER QUEUE] Lifecycle listener failed: {e}") def set_task_manager(self, task_manager: Optional["TaskManager"]) -> None: """Set the task manager for accessing task details during routing. @@ -169,8 +196,10 @@ async def clear(self) -> None: changed. """ async with self._cv: + discarded = list(self._heap) + list(self._active.values()) self._heap.clear() self._active.clear() + self._notify_evicted(discarded, None) self._cv.notify_all() # ================================================================= @@ -375,6 +404,10 @@ async def put(self, trig: Trigger, skip_merge: bool = False) -> None: # Remove ALL old triggers for this session self._heap = [t for t in self._heap if t.session_id != trig.session_id] + # Tell the durable store the old triggers were superseded so + # their rows are settled (not silently dropped / rehydrated). + self._notify_evicted(same, trig) + # NEW BEHAVIOUR: prefer new → push new trigger only heapq.heappush(self._heap, trig) @@ -560,10 +593,14 @@ async def remove_sessions(self, session_ids: list[str]) -> None: if not session_ids: return async with self._cv: + removed = [t for t in self._heap if t.session_id in session_ids] self._heap = [t for t in self._heap if t.session_id not in session_ids] - # Also remove from active triggers + # Also remove from active triggers. Active triggers are NOT + # reported as evicted — the consumer still holds them and will + # ack/nack when its react cycle finishes. for sid in session_ids: self._active.pop(sid, None) + self._notify_evicted(removed, None) heapq.heapify(self._heap) self._cv.notify_all() @@ -636,6 +673,7 @@ def _merge_trigger_group( combined_payload: Dict[str, Any] = {} combined_desc: OrderedDict[str, None] = OrderedDict() + combined_store_ids: List[int] = [] priority = triggers[0].priority fire_at = triggers[0].fire_at @@ -648,6 +686,9 @@ def _merge_trigger_group( combined_desc[desc] = None combined_payload.update(trig.payload) + # Union store rows so acking the merged trigger settles every + # constituent — a missed id would strand its row in CLAIMED. + combined_store_ids.extend(trig.store_ids) merged_desc = ( "\n\n".join(combined_desc.keys()) or triggers[0].next_action_description @@ -659,6 +700,9 @@ def _merge_trigger_group( next_action_description=merged_desc, payload=combined_payload, session_id=session_id, + id=triggers[0].id, + source=triggers[0].source, + store_ids=combined_store_ids, ) logger.debug( diff --git a/agent_core/core/trigger.py b/agent_core/core/trigger.py index 55d7f532..ea8b07a8 100644 --- a/agent_core/core/trigger.py +++ b/agent_core/core/trigger.py @@ -8,7 +8,7 @@ from __future__ import annotations from dataclasses import dataclass, field -from typing import Dict, Any, Optional +from typing import Dict, Any, List, Optional @dataclass(order=True) @@ -27,6 +27,13 @@ class Trigger: session_id: Optional session identifier for multi-user scenarios. waiting_for_reply: Whether this trigger is waiting for a user response (used by CraftBot for multi-user chat scenarios). + id: Durable-store row id when this trigger is backed by a TriggerStore + row; None for legacy in-memory-only triggers. + source: Typed origin of the trigger (TriggerSource value); empty for + legacy producers that haven't been migrated to TriggerService. + store_ids: All durable-store row ids this trigger represents. A merged + trigger carries the ids of every constituent so acking it settles + all of them. """ fire_at: float @@ -35,3 +42,6 @@ class Trigger: payload: Dict[str, Any] = field(default_factory=dict, compare=False) session_id: Optional[str] = field(default=None, compare=False) waiting_for_reply: bool = field(default=False, compare=False) + id: Optional[int] = field(default=None, compare=False) + source: str = field(default="", compare=False) + store_ids: List[int] = field(default_factory=list, compare=False) diff --git a/app/agent_base.py b/app/agent_base.py index 0fb6bada..76295c84 100644 --- a/app/agent_base.py +++ b/app/agent_base.py @@ -262,6 +262,15 @@ def __init__( route_to_session_prompt=ROUTE_TO_SESSION_PROMPT, ) + # Durable trigger layer (issue #321): triggers are written to + # sessions.db before they can run; the queue stays as the in-memory + # ordering primitive. The service registers itself as the queue's + # lifecycle listener so dropped triggers settle their rows. + from app.triggers import TriggerService, TriggerStore + + self.trigger_store = TriggerStore() + self.trigger_service = TriggerService(self.trigger_store, self.triggers) + # global state self.state_manager = StateManager(self.event_stream_manager) self.context_engine = ContextEngine(state_manager=self.state_manager) @@ -2726,6 +2735,12 @@ async def reset_agent_state(self) -> str: """ # 1. Clear runtime state await self.triggers.clear() + # Wipe the durable trigger rows too — otherwise the next boot's + # rehydration would resurrect the work this reset just cleared. + try: + self.trigger_store.clear_all() + except Exception as e: + logger.warning(f"[RESET] Failed to clear trigger store: {e}") self.task_manager.reset() self.state_manager.reset() self.event_stream_manager.clear_all() @@ -3465,11 +3480,16 @@ async def _schedule_restored_task_triggers(self) -> None: # running agent loop, after the watcher is live, so it surfaces in # the interface just like the resumed tasks' own messages. try: - await self.triggers.put( - Trigger( - fire_at=time.time(), + from app.triggers import TriggerSource, TriggerSpec + + # No dedup key: each boot composes a fresh notice. A stale + # rehydrated notice row from a crashed boot is superseded by + # this emit via the queue's same-session replacement. + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.RESTART_NOTICE, + description="Restart notice", priority=1, # ahead of resumed tasks (priority 5/7) - next_action_description="Restart notice", # Sentinel id so the heap never merges this with another # session-less trigger (e.g. memory-at-startup) and # clobbers the payload. @@ -3479,8 +3499,8 @@ async def _schedule_restored_task_triggers(self) -> None: "message": "\n".join(lines), "gui_mode": STATE.gui_mode, }, - ), - skip_merge=True, + skip_merge=True, + ) ) except Exception as e: logger.warning( @@ -3497,29 +3517,36 @@ async def _schedule_restored_task_triggers(self) -> None: is_simple = getattr(task, "mode", "complex") == "simple" restore_priority = 5 if is_simple else 7 + from app.triggers import ( + TriggerSource, + TriggerSpec, + resume_dedup_key, + ) + if task.waiting_for_user_reply: - await self.triggers.put( - Trigger( - fire_at=time.time(), - priority=restore_priority, - next_action_description=( + result = await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.RESUME, + description=( "Waiting for user reply (resumed after restart)" ), + priority=restore_priority, session_id=task_id, payload={"gui_mode": STATE.gui_mode}, + dedup_key=resume_dedup_key(task_id), waiting_for_reply=True, - ), - skip_merge=True, + skip_merge=True, + ) ) logger.info( - f"[RESTORE] Scheduled waiting trigger for task '{task.name}'" + f"[RESTORE] Scheduled waiting trigger for task " + f"'{task.name}'{' (deduped)' if result.deduped else ''}" ) else: - await self.triggers.put( - Trigger( - fire_at=time.time(), - priority=restore_priority, - next_action_description=( + result = await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.RESUME, + description=( "Resume this task after an app restart. A " "consolidated restart notice has already been " "sent to the user, so do NOT send any " @@ -3528,13 +3555,16 @@ async def _schedule_restored_task_triggers(self) -> None: "it left off based on its todos and recent " "event-stream activity." ), + priority=restore_priority, session_id=task_id, payload={"gui_mode": STATE.gui_mode}, - ), - skip_merge=True, + dedup_key=resume_dedup_key(task_id), + skip_merge=True, + ) ) logger.info( - f"[RESTORE] Scheduled resume trigger for task '{task.name}'" + f"[RESTORE] Scheduled resume trigger for task " + f"'{task.name}'{' (deduped)' if result.deduped else ''}" ) except Exception as e: logger.warning( @@ -3816,6 +3846,7 @@ def step(step_num: int, total: int, message: str) -> None: await self.scheduler.initialize( config_path=scheduler_config_path, trigger_queue=self.triggers, + trigger_service=self.trigger_service, ) await self.scheduler.start() @@ -3824,6 +3855,15 @@ def step(step_num: int, total: int, message: str) -> None: scheduler_config_path, self.scheduler.reload, name="scheduler_config.json" ) + # Rehydrate unfinished durable triggers from the previous run BEFORE + # scheduling restored-task resumes: the resume emits below carry + # dedup keys, so a rehydrated resume row blocks the duplicate instead + # of double-enqueueing. + try: + await self.trigger_service.rehydrate() + except Exception as e: + logger.warning(f"[RESTORE] Trigger rehydration failed: {e}") + # Resume triggers for tasks restored from previous session await self._schedule_restored_task_triggers() diff --git a/app/scheduler/manager.py b/app/scheduler/manager.py index 32fb3be1..c0792fa2 100644 --- a/app/scheduler/manager.py +++ b/app/scheduler/manager.py @@ -40,6 +40,7 @@ def __init__(self): self._scheduler_tasks: Dict[str, asyncio.Task] = {} self._config_path: Optional[Path] = None self._trigger_queue: Optional[TriggerQueue] = None + self._trigger_service = None # Optional[TriggerService] — durable emit path self._is_running: bool = False self._master_enabled: bool = True # Track master enabled state for config saves self._lock = asyncio.Lock() @@ -48,6 +49,7 @@ async def initialize( self, config_path: Path, trigger_queue: TriggerQueue, + trigger_service=None, ) -> None: """ Initialize the scheduler with configuration. @@ -55,9 +57,13 @@ async def initialize( Args: config_path: Path to scheduler_config.json trigger_queue: TriggerQueue to fire triggers into + trigger_service: Optional TriggerService. When provided, fires are + emitted durably with dedup keys (issue #321); when None, falls + back to direct queue puts (legacy behavior, used by old tests). """ self._config_path = Path(config_path) self._trigger_queue = trigger_queue + self._trigger_service = trigger_service # Load configuration config = self._load_config() @@ -331,17 +337,30 @@ async def queue_immediate_trigger( **(payload or {}), } - # Create trigger - trigger = Trigger( - fire_at=time.time(), # Fire immediately - priority=priority, - next_action_description=f"[Immediate] {name}: {instruction}", - payload=trigger_payload, - session_id=session_id, - ) - - # Queue the trigger - await self._trigger_queue.put(trigger) + # Queue the trigger — durably when the service is wired + # No dedup key: each immediate request is intentionally a new fire. + if self._trigger_service is not None: + from app.triggers import TriggerSource, TriggerSpec + + await self._trigger_service.emit( + TriggerSpec( + source=TriggerSource.SCHEDULED_IMMEDIATE, + description=f"[Immediate] {name}: {instruction}", + fire_at=time.time(), # Fire immediately + priority=priority, + session_id=session_id, + payload=trigger_payload, + ) + ) + else: + trigger = Trigger( + fire_at=time.time(), # Fire immediately + priority=priority, + next_action_description=f"[Immediate] {name}: {instruction}", + payload=trigger_payload, + session_id=session_id, + ) + await self._trigger_queue.put(trigger) logger.info( f"[SCHEDULER] Queued immediate trigger: {name} (session: {session_id})" @@ -607,28 +626,72 @@ async def _fire_schedule(self, schedule: ScheduledTask) -> None: f"{overdue_human}; firing as catch-up with agent-judgment note" ) - # One-time tasks: remove from the persisted config BEFORE enqueueing so a - # crash/restart between firing and removal can never re-fire them. The - # in-memory trigger queue is not persisted, so once it's enqueued the - # config entry is no longer needed. - if not schedule.recurring: - self._schedules.pop(schedule.id, None) - self._save_config() - logger.info( - f"[SCHEDULER] One-time task fired, removed from config: {schedule.id}" + if self._trigger_service is not None: + # Durable path: emit FIRST — the dedup key is the + # crash guard now. A crash anywhere after the INSERT can't lose + # the fire, and a re-fire attempt (config not yet saved, or the + # run_count>0 reload skip missed) collides with the active row + # and is a no-op. Then remove one-time tasks from the config. + from app.triggers import ( + TriggerSource, + TriggerSpec, + scheduled_dedup_key, + scheduled_once_dedup_key, ) - # Create trigger - trigger = Trigger( - fire_at=now, - priority=schedule.priority, - next_action_description=description, - payload=payload, - session_id=session_id, - ) + if not schedule.recurring: + source = TriggerSource.SCHEDULED_ONCE + dedup_key = scheduled_once_dedup_key(schedule.id) + else: + source = TriggerSource.SCHEDULED + # Bucket by this fire's scheduled minute (next_run was set to + # this fire's target by the schedule loop) so retrying the + # same fire dedups but the next occurrence does not. + dedup_key = scheduled_dedup_key(schedule.id, schedule.next_run or now) + + result = await self._trigger_service.emit( + TriggerSpec( + source=source, + description=description, + fire_at=now, + priority=schedule.priority, + session_id=session_id, + payload=payload, + dedup_key=dedup_key, + ) + ) + if result.deduped: + logger.info( + f"[SCHEDULER] Fire deduped (already queued/in-flight): " + f"{schedule.id} - {schedule.name}" + ) - # Fire! - await self._trigger_queue.put(trigger) + if not schedule.recurring: + self._schedules.pop(schedule.id, None) + self._save_config() + logger.info( + f"[SCHEDULER] One-time task fired, removed from config: {schedule.id}" + ) + else: + # Legacy path (no durable store wired): keep the Phase 0 ordering — + # remove one-time tasks from the persisted config BEFORE enqueueing + # so a crash/restart between firing and removal can never re-fire + # them. + if not schedule.recurring: + self._schedules.pop(schedule.id, None) + self._save_config() + logger.info( + f"[SCHEDULER] One-time task fired, removed from config: {schedule.id}" + ) + + trigger = Trigger( + fire_at=now, + priority=schedule.priority, + next_action_description=description, + payload=payload, + session_id=session_id, + ) + await self._trigger_queue.put(trigger) logger.info( f"[SCHEDULER] Fired schedule: {schedule.id} - {schedule.name} " diff --git a/app/triggers/__init__.py b/app/triggers/__init__.py new file mode 100644 index 00000000..bdacac1f --- /dev/null +++ b/app/triggers/__init__.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +""" +app.triggers + +Durable trigger execution (issue #321): typed sources, the SQLite-backed +TriggerStore, and the TriggerService producer/consumer front door. +""" + +from app.triggers.sources import ( + TriggerSource, + resume_dedup_key, + scheduled_dedup_key, + scheduled_once_dedup_key, +) +from app.triggers.store import TriggerStore, get_trigger_store +from app.triggers.service import EmitResult, TriggerService, TriggerSpec + +__all__ = [ + "TriggerSource", + "TriggerStore", + "TriggerService", + "TriggerSpec", + "EmitResult", + "get_trigger_store", + "resume_dedup_key", + "scheduled_dedup_key", + "scheduled_once_dedup_key", +] diff --git a/app/triggers/service.py b/app/triggers/service.py new file mode 100644 index 00000000..66fe2dc6 --- /dev/null +++ b/app/triggers/service.py @@ -0,0 +1,286 @@ +# -*- coding: utf-8 -*- +""" +app.triggers.service + +TriggerService — the single producer front door for durable triggers +(issue #321, Primitive A). + +``emit()`` writes the trigger to the store FIRST (no LLM call, sub-ms), then +feeds the in-memory TriggerQueue, which stays as the ordering primitive. The +consumer drives the lifecycle through ``next()`` (claim) and ``ack()``/ +``nack()`` (settle); ``rehydrate()`` re-delivers everything unfinished at +boot. The service also implements the queue's lifecycle-listener protocol so +triggers the queue discards (same-session replacement, session removal, +clear) settle their rows instead of resurrecting on the next boot. + +Legacy producers that still call ``queue.put()`` directly keep working: +their triggers carry no ``store_ids``, so claim/ack are no-ops for them. +""" + +from __future__ import annotations + +import json +import logging +import time +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional, Union + +from agent_core.core.trigger import Trigger +from agent_core.core.impl.trigger.queue import TriggerQueue + +from app.triggers.sources import TriggerSource +from app.triggers.store import STALE_TRIGGER_HOURS, TriggerStore + +try: + from app.logger import logger +except Exception: + logger = logging.getLogger(__name__) + logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") + + +# A trigger rehydrated/fired more than this many seconds late gets a +# catch-up note so the agent can use judgment (generalizes the Phase 0 +# scheduler-only behavior to every source). +CATCHUP_THRESHOLD_SECONDS = 120 + + +@dataclass +class TriggerSpec: + """What a producer asks TriggerService to durably schedule.""" + + source: Union[TriggerSource, str] + description: str + fire_at: Optional[float] = None # None → now + priority: int = 50 + session_id: Optional[str] = None + payload: Dict[str, Any] = field(default_factory=dict) + dedup_key: Optional[str] = None + skip_merge: bool = False + waiting_for_reply: bool = False + + +@dataclass +class EmitResult: + trigger_id: Optional[int] + deduped: bool + + +def _format_duration(seconds: float) -> str: + seconds = int(seconds) + if seconds < 60: + value, unit = seconds, "second" + elif seconds < 3600: + value, unit = seconds // 60, "minute" + elif seconds < 86400: + value, unit = seconds // 3600, "hour" + else: + value, unit = seconds // 86400, "day" + return f"{value} {unit}{'s' if value != 1 else ''}" + + +class TriggerService: + """Durable front door over (TriggerStore, TriggerQueue).""" + + def __init__(self, store: TriggerStore, queue: TriggerQueue) -> None: + self._store = store + self._queue = queue + queue.set_lifecycle_listener(self) + + # ─────────────────────── Producer API ─────────────────────────────────── + + async def emit(self, spec: TriggerSpec) -> EmitResult: + """Durably record a trigger, then enqueue it. + + The store INSERT happens first — from that point a crash anywhere + loses nothing. A dedup_key collision with an active row means this + work is already queued or in flight: no enqueue, no double-fire. + """ + fire_at = spec.fire_at if spec.fire_at is not None else time.time() + source = ( + spec.source.value + if isinstance(spec.source, TriggerSource) + else str(spec.source) + ) + + row_id, created = self._store.insert( + source=source, + description=spec.description, + fire_at=fire_at, + priority=spec.priority, + session_id=spec.session_id, + payload=spec.payload, + dedup_key=spec.dedup_key, + waiting_for_reply=spec.waiting_for_reply, + ) + if not created: + logger.info( + f"[TriggerService] Deduped emit (key={spec.dedup_key!r}, " + f"existing row={row_id})" + ) + return EmitResult(row_id, True) + + trig = Trigger( + fire_at=fire_at, + priority=spec.priority, + next_action_description=spec.description, + payload=dict(spec.payload), + session_id=spec.session_id, + waiting_for_reply=spec.waiting_for_reply, + id=row_id, + source=source, + store_ids=[row_id] if row_id is not None else [], + ) + await self._queue.put(trig, skip_merge=spec.skip_merge) + return EmitResult(row_id, False) + + # ─────────────────────── Consumer API ─────────────────────────────────── + + async def next(self) -> Trigger: + """Wait for the next due trigger and claim its store rows.""" + trig = await self._queue.get() + if trig.store_ids: + self._store.claim(trig.store_ids) + return trig + + async def ack(self, trig: Trigger) -> None: + """The react cycle for this trigger completed.""" + if trig.store_ids: + self._store.ack(trig.store_ids) + + async def nack(self, trig: Trigger, error: str) -> None: + """The react cycle raised before completing.""" + if trig.store_ids: + self._store.fail(trig.store_ids, error=error) + + # ─────────────────────── fire() pass-through ──────────────────────────── + + async def fire( + self, + session_id: str, + *, + message: Optional[str] = None, + platform: Optional[str] = None, + living_ui_id: Optional[str] = None, + ) -> bool: + """Retarget a session's trigger to now, durably mirroring the change. + + The store write happens before the in-memory mutation so an attached + user message survives a crash mid-react (today it would be lost). + """ + patch: Dict[str, Any] = {} + if message: + patch["pending_user_message"] = message + if platform: + patch["pending_platform"] = platform + if living_ui_id: + patch["living_ui_id"] = living_ui_id + try: + self._store.update_for_fire(session_id, time.time(), patch) + except Exception as e: + logger.warning(f"[TriggerService] Failed to mirror fire() to store: {e}") + return await self._queue.fire( + session_id, + message=message, + platform=platform, + living_ui_id=living_ui_id, + ) + + # ─────────────────────── Boot recovery ────────────────────────────────── + + async def rehydrate(self) -> int: + """Re-deliver every unfinished trigger from the previous run. + + 1. CLAIMED orphans (in flight when the process died) → PENDING. + 2. Load PENDING rows into the queue. Stale rows (> 24h past due, + mirroring the task TTL) are settled instead of re-fired; overdue + rows get an agent-judgment catch-up note (generalized Phase 0). + + Must run BEFORE ``_schedule_restored_task_triggers()`` so boot-time + ``resume:{task_id}`` re-emits hit the dedup index instead of + double-enqueueing. + """ + self._store.reclaim_claimed() + + now = time.time() + stale_ids: List[int] = [] + requeued = 0 + + for row in self._store.load_pending(): + overdue = now - row["fire_at"] + + if overdue > STALE_TRIGGER_HOURS * 3600: + stale_ids.append(row["id"]) + logger.info( + f"[TriggerService] Skipping stale trigger {row['id']} " + f"({row['source']}, {_format_duration(overdue)} past due)" + ) + continue + + try: + payload = json.loads(row["payload_json"]) + except (ValueError, TypeError): + payload = {} + description = row["description"] + + if overdue > CATCHUP_THRESHOLD_SECONDS and not payload.get("is_catch_up"): + note = ( + f"NOTE: This trigger was due about " + f"{_format_duration(overdue)} ago but CraftBot was offline " + f"at the time. Use your judgment: if it is only slightly " + f"late and still relevant, carry it out normally. If it is " + f"significantly late, or the action is time-sensitive or " + f"irreversible (e.g. sending a message or email), confirm " + f"with the user before proceeding, or skip it if it is no " + f"longer relevant." + ) + payload["is_catch_up"] = True + payload["overdue_seconds"] = overdue + description = f"{description}\n\n{note}" + + trig = Trigger( + fire_at=row["fire_at"], + priority=row["priority"], + next_action_description=description, + payload=payload, + session_id=row["session_id"], + waiting_for_reply=bool(row["waiting_for_reply"]), + id=row["id"], + source=row["source"] or "", + store_ids=[row["id"]], + ) + await self._queue.put(trig, skip_merge=True) + requeued += 1 + + if stale_ids: + self._store.mark_stale(stale_ids) + if requeued: + logger.info( + f"[TriggerService] Rehydrated {requeued} pending trigger(s) " + "from previous run" + ) + return requeued + + # ─────────────────────── Session / reset cleanup ──────────────────────── + + async def cancel_sessions(self, session_ids: List[str]) -> None: + """Settle a session's rows and drop its queued triggers.""" + self._store.cancel_sessions(session_ids) + await self._queue.remove_sessions(session_ids) + + def clear_all(self) -> None: + """Wipe the store (agent reset path).""" + self._store.clear_all() + + # ─────────────────────── TriggerLifecycleListener ─────────────────────── + + def on_evicted( + self, evicted: List[Trigger], replacement: Optional[Trigger] + ) -> None: + """Queue discarded triggers unconsumed — settle their rows.""" + ids = [row_id for t in evicted for row_id in (t.store_ids or [])] + if not ids: + return + if replacement is not None: + self._store.supersede(ids, replacement.id) + else: + self._store.cancel(ids) diff --git a/app/triggers/sources.py b/app/triggers/sources.py new file mode 100644 index 00000000..2554fac8 --- /dev/null +++ b/app/triggers/sources.py @@ -0,0 +1,46 @@ +# -*- coding: utf-8 -*- +""" +app.triggers.sources + +Typed trigger sources and dedup-key builders (issue #321). + +Phase 1 defines only the sources migrated to TriggerService so far; Phase 2 +extends this enum to every producer and removes the scattered +``payload["type"]`` string branching. +""" + +from __future__ import annotations + +from enum import Enum + + +class TriggerSource(str, Enum): + """Typed origin of a trigger. Stored in the ``triggers.source`` column.""" + + SCHEDULED = "scheduled" + SCHEDULED_ONCE = "scheduled_once" + SCHEDULED_IMMEDIATE = "scheduled_immediate" + RESUME = "resume" + RESTART_NOTICE = "restart_notice" + # Catch-all for producers not yet migrated to TriggerService. + LEGACY = "legacy" + + +def scheduled_dedup_key(schedule_id: str, fire_target: float) -> str: + """Dedup key for one fire of a recurring schedule. + + Bucketed to the scheduled minute: re-emitting the *same* fire (crash + retry) dedups, while the next legitimate occurrence (a different + minute) does not. + """ + return f"scheduled:{schedule_id}:{int(fire_target // 60)}" + + +def scheduled_once_dedup_key(schedule_id: str) -> str: + """Dedup key for a one-time scheduled task — one fire, ever, per id.""" + return f"scheduled-once:{schedule_id}" + + +def resume_dedup_key(task_id: str) -> str: + """Dedup key for a boot-time task resume — double-boot can't double-resume.""" + return f"resume:{task_id}" diff --git a/app/triggers/store.py b/app/triggers/store.py new file mode 100644 index 00000000..8ac0506a --- /dev/null +++ b/app/triggers/store.py @@ -0,0 +1,441 @@ +# -*- coding: utf-8 -*- +""" +app.triggers.store + +Durable trigger store (issue #321, Primitive A). + +SQLite-backed write-ahead store for triggers: a trigger accepted by +TriggerService is INSERTed here as PENDING before it can run, claimed +(CLAIMED) when the consumer picks it up, and settled (DONE/FAILED) when the +react cycle finishes. A boot-time reclaim scan turns orphaned CLAIMED rows +back into PENDING, so a crash anywhere between emit and ack re-delivers +instead of losing work. + +Dedup is enforced by the database, not application code: a partial UNIQUE +index on ``dedup_key`` over *active* rows (PENDING/CLAIMED) makes inserting +the same work twice an atomic no-op, while settled rows don't block +legitimate re-fires (e.g. ``resume:{task_id}`` on a later restart). + +Same file and idioms as ``app/usage/session_storage.py`` (sessions.db, WAL, +short per-operation connections, sync API). +""" + +from __future__ import annotations + +import json +import logging +import sqlite3 +import time +from datetime import datetime, timezone +from pathlib import Path +from typing import Any, Dict, List, Optional, Tuple + +try: + from app.logger import logger +except Exception: + logger = logging.getLogger(__name__) + logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") + + +# Trigger lifecycle states +STATUS_PENDING = "PENDING" +STATUS_CLAIMED = "CLAIMED" +STATUS_DONE = "DONE" +STATUS_FAILED = "FAILED" +STATUS_DEAD = "DEAD" + +ACTIVE_STATUSES = (STATUS_PENDING, STATUS_CLAIMED) + +# How a settled row reached DONE/FAILED — for observability, not logic. +RESOLUTION_COMPLETED = "completed" +RESOLUTION_SUPERSEDED = "superseded" +RESOLUTION_CANCELLED = "cancelled" +RESOLUTION_FAILED = "failed" +RESOLUTION_STALE = "stale" + +# PENDING rows whose fire_at is older than this are not rehydrated (mirrors +# the STALE_TASK_HOURS=24 task TTL in session_storage). +STALE_TRIGGER_HOURS = 24 + + +def _now_iso() -> str: + return datetime.now(timezone.utc).isoformat() + + +class TriggerStore: + """Durable persistence for triggers, layered under TriggerQueue.""" + + def __init__(self, db_path: Optional[str] = None): + if db_path is None: + from app.config import APP_DATA_PATH + + usage_dir = Path(APP_DATA_PATH) / ".usage" + usage_dir.mkdir(parents=True, exist_ok=True) + db_path = str(usage_dir / "sessions.db") + + self._db_path = db_path + self._init_db() + logger.info(f"[TriggerStore] Initialized at {self._db_path}") + + def _connect(self) -> sqlite3.Connection: + conn = sqlite3.connect(self._db_path) + conn.execute("PRAGMA busy_timeout=5000") + return conn + + def _init_db(self) -> None: + """Create the triggers table, replacing any pre-#321 legacy schema.""" + with self._connect() as conn: + conn.execute("PRAGMA journal_mode=WAL") + + # A `triggers` table existed in old CraftBot versions with a + # different schema (session_storage used to DROP it every boot). + # If a table without our dedup_key column is present, it's that + # relic — replace it. + cols = [ + row[1] + for row in conn.execute("PRAGMA table_info(triggers)").fetchall() + ] + # "queue" is the newest column — its absence also catches a table + # created by an earlier pre-release iteration of this branch. + if cols and ("dedup_key" not in cols or "queue" not in cols): + logger.info("[TriggerStore] Dropping legacy triggers table") + conn.execute("DROP TABLE triggers") + + conn.execute(""" + CREATE TABLE IF NOT EXISTS triggers ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + source TEXT NOT NULL DEFAULT '', + dedup_key TEXT, + session_id TEXT, + queue TEXT NOT NULL DEFAULT 'main', + fire_at REAL NOT NULL, + not_before REAL, + priority INTEGER NOT NULL DEFAULT 50, + description TEXT NOT NULL DEFAULT '', + payload_json TEXT NOT NULL DEFAULT '{}', + waiting_for_reply INTEGER NOT NULL DEFAULT 0, + status TEXT NOT NULL DEFAULT 'PENDING', + resolution TEXT, + superseded_by INTEGER, + attempts INTEGER NOT NULL DEFAULT 0, + claimed_by TEXT, + lease_until REAL, + last_error TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL + ) + """) + conn.execute(""" + CREATE INDEX IF NOT EXISTS idx_triggers_status_fire + ON triggers(status, fire_at) + """) + conn.execute(""" + CREATE INDEX IF NOT EXISTS idx_triggers_session + ON triggers(session_id) + """) + # Dedup is active-rows-only: a DONE resume:{task_id} must not + # block a legitimate re-resume after the next restart. + conn.execute(""" + CREATE UNIQUE INDEX IF NOT EXISTS uq_triggers_dedup_active + ON triggers(dedup_key) + WHERE dedup_key IS NOT NULL + AND status IN ('PENDING', 'CLAIMED') + """) + conn.commit() + + # ─────────────────────── Insert / dedup ───────────────────────────────── + + def insert( + self, + *, + source: str, + description: str, + fire_at: float, + priority: int = 50, + session_id: Optional[str] = None, + payload: Optional[Dict[str, Any]] = None, + dedup_key: Optional[str] = None, + not_before: Optional[float] = None, + waiting_for_reply: bool = False, + queue: str = "main", + ) -> Tuple[Optional[int], bool]: + """Durably record a trigger as PENDING. + + Returns: + (row_id, created). When ``dedup_key`` collides with an active row, + the insert is a no-op and the existing row's id is returned with + created=False. + """ + now = _now_iso() + payload_json = json.dumps(payload or {}, default=str) + with self._connect() as conn: + try: + cur = conn.execute( + """ + INSERT INTO triggers ( + source, dedup_key, session_id, queue, fire_at, + not_before, priority, description, payload_json, + waiting_for_reply, status, attempts, created_at, + updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'PENDING', 0, ?, ?) + """, + ( + source, + dedup_key, + session_id, + queue, + fire_at, + not_before, + priority, + description, + payload_json, + 1 if waiting_for_reply else 0, + now, + now, + ), + ) + conn.commit() + return cur.lastrowid, True + except sqlite3.IntegrityError: + row = conn.execute( + """ + SELECT id FROM triggers + WHERE dedup_key = ? AND status IN ('PENDING', 'CLAIMED') + """, + (dedup_key,), + ).fetchone() + return (row[0] if row else None), False + + # ─────────────────────── Lifecycle transitions ─────────────────────────── + + def _set_status( + self, + ids: List[int], + status: str, + *, + resolution: Optional[str] = None, + superseded_by: Optional[int] = None, + last_error: Optional[str] = None, + ) -> None: + if not ids: + return + now = _now_iso() + with self._connect() as conn: + conn.executemany( + """ + UPDATE triggers + SET status = ?, resolution = ?, superseded_by = ?, + last_error = COALESCE(?, last_error), + lease_until = NULL, updated_at = ? + WHERE id = ? + """, + [ + (status, resolution, superseded_by, last_error, now, row_id) + for row_id in ids + ], + ) + conn.commit() + + def claim( + self, + ids: List[int], + lease_seconds: float = 900.0, + claimed_by: str = "main", + ) -> List[int]: + """Atomically mark rows as being worked on (PENDING → CLAIMED). + + Compare-and-swap semantics: a row is claimed only if it is still + PENDING, and the list of actually-claimed ids is returned. With + today's single consumer the claim always wins; with multiple + sub-agent consumers (future) the database arbitrates — exactly one + claimant per row. + + The lease exists for crash recovery: with a single consumer the + boot-time reclaim scan re-delivers orphans; multi-consumer mode adds + a periodic lease-expiry sweep. + """ + if not ids: + return [] + now = _now_iso() + lease_until = time.time() + lease_seconds + claimed: List[int] = [] + with self._connect() as conn: + for row_id in ids: + cur = conn.execute( + """ + UPDATE triggers + SET status = 'CLAIMED', attempts = attempts + 1, + claimed_by = ?, lease_until = ?, updated_at = ? + WHERE id = ? AND status = 'PENDING' + """, + (claimed_by, lease_until, now, row_id), + ) + if cur.rowcount: + claimed.append(row_id) + conn.commit() + return claimed + + def ack(self, ids: List[int]) -> None: + """The react cycle completed — settle the rows (→ DONE).""" + self._set_status(ids, STATUS_DONE, resolution=RESOLUTION_COMPLETED) + + def fail(self, ids: List[int], error: Optional[str] = None) -> None: + """The react cycle raised — settle the rows (→ FAILED). + + Phase 5 turns this into retry-with-backoff; until then FAILED is + terminal (matching today's behavior where the consumer logs and + moves on). + """ + self._set_status( + ids, STATUS_FAILED, resolution=RESOLUTION_FAILED, last_error=error + ) + + def supersede(self, ids: List[int], by_id: Optional[int]) -> None: + """Rows replaced by a newer same-session trigger (→ DONE, marked).""" + self._set_status( + ids, STATUS_DONE, resolution=RESOLUTION_SUPERSEDED, superseded_by=by_id + ) + + def cancel(self, ids: List[int]) -> None: + """Rows removed without execution (session cleanup, reset).""" + self._set_status(ids, STATUS_DONE, resolution=RESOLUTION_CANCELLED) + + def cancel_sessions(self, session_ids: List[str]) -> int: + """Settle every active row belonging to the given sessions.""" + if not session_ids: + return 0 + now = _now_iso() + placeholders = ",".join("?" for _ in session_ids) + with self._connect() as conn: + cur = conn.execute( + f""" + UPDATE triggers + SET status = 'DONE', resolution = 'cancelled', + lease_until = NULL, updated_at = ? + WHERE session_id IN ({placeholders}) + AND status IN ('PENDING', 'CLAIMED') + """, + (now, *session_ids), + ) + conn.commit() + return cur.rowcount + + # ─────────────────────── Boot recovery ────────────────────────────────── + + def reclaim_claimed(self) -> int: + """Boot scan: CLAIMED rows are orphans of a crashed run → PENDING. + + Single consumer means any CLAIMED row at boot was in-flight when the + process died; lease expiry doesn't matter for this scan. + """ + now = _now_iso() + with self._connect() as conn: + cur = conn.execute( + """ + UPDATE triggers + SET status = 'PENDING', claimed_by = NULL, + lease_until = NULL, updated_at = ? + WHERE status = 'CLAIMED' + """, + (now,), + ) + conn.commit() + if cur.rowcount: + logger.info( + f"[TriggerStore] Reclaimed {cur.rowcount} in-flight trigger(s) " + "from previous run" + ) + return cur.rowcount + + def load_pending(self) -> List[Dict[str, Any]]: + """Return all PENDING rows (as dicts) ordered by fire_at.""" + with self._connect() as conn: + conn.row_factory = sqlite3.Row + rows = conn.execute( + "SELECT * FROM triggers WHERE status = 'PENDING' ORDER BY fire_at ASC" + ).fetchall() + return [dict(row) for row in rows] + + def mark_stale(self, ids: List[int]) -> None: + """Settle rows skipped at rehydration for being too old.""" + self._set_status(ids, STATUS_DONE, resolution=RESOLUTION_STALE) + + # ─────────────────────── fire() mirroring ─────────────────────────────── + + def update_for_fire( + self, + session_id: str, + fire_at: float, + payload_patch: Dict[str, Any], + ) -> int: + """Mirror queue.fire() onto the session's active rows. + + Persists the retargeted fire_at and any attached user message BEFORE + the in-memory mutation, so a crash mid-react keeps the message for + redelivery. + """ + now = _now_iso() + updated = 0 + with self._connect() as conn: + conn.row_factory = sqlite3.Row + rows = conn.execute( + """ + SELECT id, payload_json FROM triggers + WHERE session_id = ? AND status IN ('PENDING', 'CLAIMED') + """, + (session_id,), + ).fetchall() + for row in rows: + try: + payload = json.loads(row["payload_json"]) + except (ValueError, TypeError): + payload = {} + payload.update(payload_patch) + conn.execute( + """ + UPDATE triggers + SET fire_at = ?, payload_json = ?, updated_at = ? + WHERE id = ? + """, + (fire_at, json.dumps(payload, default=str), now, row["id"]), + ) + updated += 1 + conn.commit() + return updated + + # ─────────────────────── Utilities ────────────────────────────────────── + + def get(self, row_id: int) -> Optional[Dict[str, Any]]: + with self._connect() as conn: + conn.row_factory = sqlite3.Row + row = conn.execute( + "SELECT * FROM triggers WHERE id = ?", (row_id,) + ).fetchone() + return dict(row) if row else None + + def count_by_status(self) -> Dict[str, int]: + with self._connect() as conn: + rows = conn.execute( + "SELECT status, COUNT(*) FROM triggers GROUP BY status" + ).fetchall() + return {status: count for status, count in rows} + + def clear_all(self) -> None: + """Wipe the table. Part of the agent reset path — without this a + reset followed by a restart would resurrect cleared work.""" + with self._connect() as conn: + conn.execute("DELETE FROM triggers") + conn.commit() + logger.info("[TriggerStore] Cleared all trigger rows") + + +# Global store instance (same pattern as get_session_storage) +_trigger_store: Optional[TriggerStore] = None + + +def get_trigger_store() -> TriggerStore: + """Get the global trigger store instance.""" + global _trigger_store + if _trigger_store is None: + _trigger_store = TriggerStore() + return _trigger_store diff --git a/app/ui_layer/controller/ui_controller.py b/app/ui_layer/controller/ui_controller.py index 55786b90..fa3ba19a 100644 --- a/app/ui_layer/controller/ui_controller.py +++ b/app/ui_layer/controller/ui_controller.py @@ -511,20 +511,38 @@ def _update_state_from_event(self, event: UIEvent) -> None: ) async def _consume_triggers(self) -> None: - """Consume triggers and run agent reactions.""" + """Consume triggers and run agent reactions. + + Durable lifecycle (issue #321): ``next()`` claims the trigger's store + rows (CLAIMED), ``ack()`` settles them when the react cycle completes, + ``nack()`` on an exception. A crash or cancellation mid-react leaves + the rows CLAIMED, and the next boot's reclaim scan re-delivers them — + at-least-once instead of silently lost. + """ logger.info("[CONSUMER] Trigger consumer started") try: while self._running and self._agent.is_running: + trigger = None try: - trigger = await self._agent.triggers.get() + trigger = await self._agent.trigger_service.next() await self._agent.react(trigger) + await self._agent.trigger_service.ack(trigger) except asyncio.CancelledError: + # Shutdown: deliberately no ack/nack — the row stays + # CLAIMED and is reclaimed (re-delivered) on next boot. raise except Exception as e: logger.error( f"[CONSUMER] Exception during trigger processing: {e!r}", exc_info=True, ) + if trigger is not None: + try: + await self._agent.trigger_service.nack(trigger, repr(e)) + except Exception: + logger.error( + "[CONSUMER] Failed to nack trigger", exc_info=True + ) await asyncio.sleep(0.1) except asyncio.CancelledError: logger.info("[CONSUMER] Trigger consumer cancelled") diff --git a/app/usage/session_storage.py b/app/usage/session_storage.py index 4056f3e0..ddecf578 100644 --- a/app/usage/session_storage.py +++ b/app/usage/session_storage.py @@ -100,8 +100,8 @@ def _init_db(self) -> None: ) """) - # Clean up triggers table from previous versions (no longer used) - cursor.execute("DROP TABLE IF EXISTS triggers") + # NOTE: the `triggers` table is owned by app/triggers/store.py + # (durable trigger store, issue #321) — do not touch it here. conn.commit() diff --git a/tests/e2e/_harness/helpers.py b/tests/e2e/_harness/helpers.py index 81b7e7f9..4ee892c1 100644 --- a/tests/e2e/_harness/helpers.py +++ b/tests/e2e/_harness/helpers.py @@ -197,6 +197,12 @@ async def _external_event_spy(payload: dict) -> None: get_session_storage().clear_all() except Exception: pass + # Wipe durable trigger rows too — otherwise rows from a previous test run + # would rehydrate into this one. + try: + agent.trigger_store.clear_all() + except Exception: + pass async with record_llm_calls(agent), record_action_calls(agent): # Poll each requested integration's session status until it's ready. @@ -276,10 +282,13 @@ async def _external_event_spy(payload: dict) -> None: break try: trig = await asyncio.wait_for( - agent.triggers.get(), timeout=per_iter_timeout + agent.trigger_service.next(), timeout=per_iter_timeout ) except asyncio.TimeoutError: break await agent.react(trig) + # Settle the durable rows like the production consumer does — + # without this, claimed rows pile up and rehydrate next run. + await agent.trigger_service.ack(trig) return bridge_statuses diff --git a/tests/test_trigger_service.py b/tests/test_trigger_service.py new file mode 100644 index 00000000..c252a33d --- /dev/null +++ b/tests/test_trigger_service.py @@ -0,0 +1,337 @@ +# -*- coding: utf-8 -*- +"""Integration tests for TriggerService + TriggerQueue + TriggerStore +(issue #321, Phase 1) — including crash/restart simulations.""" + +import asyncio +import heapq +import time + +from agent_core.core.trigger import Trigger +from agent_core.core.impl.trigger.queue import TriggerQueue + +from app.triggers import TriggerService, TriggerSpec, TriggerSource +from app.triggers.store import TriggerStore + + +def run(coro): + return asyncio.run(coro) + + +def make_stack(tmp_path, name="sessions.db"): + """Real store + real queue (dummy LLM — routing never fires because every + spec carries a session_id and the routing prompt is empty).""" + store = TriggerStore(db_path=str(tmp_path / name)) + queue = TriggerQueue(llm=object()) + service = TriggerService(store, queue) + return store, queue, service + + +def spec(**overrides): + kwargs = dict( + source=TriggerSource.SCHEDULED, + description="do the thing", + priority=50, + session_id="s1", + payload={"type": "scheduled"}, + skip_merge=True, + ) + kwargs.update(overrides) + return TriggerSpec(**kwargs) + + +class TestEmitNextAck: + def test_roundtrip_transitions(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + result = await service.emit(spec()) + assert not result.deduped + assert store.get(result.trigger_id)["status"] == "PENDING" + + trig = await asyncio.wait_for(service.next(), timeout=2) + assert trig.store_ids == [result.trigger_id] + assert store.get(result.trigger_id)["status"] == "CLAIMED" + + await service.ack(trig) + assert store.get(result.trigger_id)["status"] == "DONE" + + run(scenario()) + + def test_nack_marks_failed(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + result = await service.emit(spec()) + trig = await asyncio.wait_for(service.next(), timeout=2) + await service.nack(trig, "RuntimeError: kaboom") + row = store.get(result.trigger_id) + assert row["status"] == "FAILED" + assert "kaboom" in row["last_error"] + + run(scenario()) + + def test_dedup_emit_no_double_enqueue(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + r1 = await service.emit( + spec(dedup_key="scheduled-once:abc", session_id="a") + ) + r2 = await service.emit( + spec(dedup_key="scheduled-once:abc", session_id="b") + ) + assert not r1.deduped + assert r2.deduped + assert r2.trigger_id == r1.trigger_id + assert await queue.size() == 1 + + run(scenario()) + + def test_legacy_put_passthrough(self, tmp_path): + # Producers not yet migrated still work; their triggers carry no + # store rows and ack is a no-op. + store, queue, service = make_stack(tmp_path) + + async def scenario(): + await queue.put( + Trigger( + fire_at=time.time(), + priority=3, + next_action_description="legacy", + session_id="legacy-session", + ), + skip_merge=True, + ) + trig = await asyncio.wait_for(service.next(), timeout=2) + assert trig.store_ids == [] + await service.ack(trig) # must not raise + assert store.count_by_status() == {} + + run(scenario()) + + +class TestCrashRecovery: + def test_crash_while_pending_rehydrates_once(self, tmp_path): + async def scenario(): + store, queue, service = make_stack(tmp_path) + result = await service.emit(spec()) + # crash: process dies with the trigger still PENDING in the heap + + store2, queue2, service2 = make_stack(tmp_path) # restart + requeued = await service2.rehydrate() + assert requeued == 1 + assert await queue2.size() == 1 + + trig = await asyncio.wait_for(service2.next(), timeout=2) + assert trig.store_ids == [result.trigger_id] + await service2.ack(trig) + + # a second restart must not re-deliver settled work + store3, queue3, service3 = make_stack(tmp_path) + assert await service3.rehydrate() == 0 + + run(scenario()) + + def test_crash_mid_react_reclaims_claimed(self, tmp_path): + async def scenario(): + store, queue, service = make_stack(tmp_path) + result = await service.emit(spec()) + await asyncio.wait_for(service.next(), timeout=2) + assert store.get(result.trigger_id)["status"] == "CLAIMED" + # crash: no ack — row orphaned CLAIMED + + store2, queue2, service2 = make_stack(tmp_path) # restart + requeued = await service2.rehydrate() + assert requeued == 1 + row = store2.get(result.trigger_id) + assert row["status"] == "CLAIMED" or row["status"] == "PENDING" + trig = await asyncio.wait_for(service2.next(), timeout=2) + await service2.ack(trig) + assert store2.get(result.trigger_id)["status"] == "DONE" + assert store2.get(result.trigger_id)["attempts"] == 2 + + run(scenario()) + + def test_boot_resume_emit_hits_rehydrated_dedup(self, tmp_path): + # Double-boot can't double-resume: the rehydrated resume row blocks + # the boot-time re-emit via the dedup index. + async def scenario(): + store, queue, service = make_stack(tmp_path) + await service.emit( + spec( + source=TriggerSource.RESUME, + dedup_key="resume:task42", + session_id="task42", + ) + ) + # crash before consumption + + store2, queue2, service2 = make_stack(tmp_path) + await service2.rehydrate() + result = await service2.emit( + spec( + source=TriggerSource.RESUME, + dedup_key="resume:task42", + session_id="task42", + ) + ) + assert result.deduped + assert await queue2.size() == 1 + + run(scenario()) + + +class TestEvictionSettlesRows: + def test_same_session_replacement_supersedes(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + r1 = await service.emit(spec(description="old")) + r2 = await service.emit(spec(description="new")) + row = store.get(r1.trigger_id) + assert row["status"] == "DONE" + assert row["resolution"] == "superseded" + assert row["superseded_by"] == r2.trigger_id + assert await queue.size() == 1 + + run(scenario()) + + def test_superseded_rows_do_not_rehydrate(self, tmp_path): + async def scenario(): + store, queue, service = make_stack(tmp_path) + await service.emit(spec(description="old")) + r2 = await service.emit(spec(description="new")) + # crash + + store2, queue2, service2 = make_stack(tmp_path) + requeued = await service2.rehydrate() + assert requeued == 1 + trig = await asyncio.wait_for(service2.next(), timeout=2) + assert trig.store_ids == [r2.trigger_id] + + run(scenario()) + + def test_cancel_sessions_settles_and_dequeues(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + result = await service.emit(spec(session_id="doomed")) + await service.cancel_sessions(["doomed"]) + assert store.get(result.trigger_id)["resolution"] == "cancelled" + assert await queue.size() == 0 + + run(scenario()) + + def test_queue_clear_cancels_rows(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + result = await service.emit(spec()) + await queue.clear() + assert store.get(result.trigger_id)["resolution"] == "cancelled" + + run(scenario()) + + +class TestMergedTriggerAck: + def test_merge_unions_store_ids_and_ack_settles_all(self, tmp_path): + # put() replaces same-session triggers, so two same-session entries + # can only coexist via internal paths — inject directly to verify the + # merge/ack contract (a missed id would strand a row in CLAIMED). + store, queue, service = make_stack(tmp_path) + + async def scenario(): + id1, _ = store.insert( + source="scheduled", description="a", fire_at=time.time() + ) + id2, _ = store.insert( + source="scheduled", description="b", fire_at=time.time() + ) + for row_id, desc in ((id1, "a"), (id2, "b")): + heapq.heappush( + queue._heap, + Trigger( + fire_at=time.time() - 1, + priority=50, + next_action_description=desc, + session_id="s1", + id=row_id, + source="scheduled", + store_ids=[row_id], + ), + ) + + trig = await asyncio.wait_for(service.next(), timeout=2) + assert sorted(trig.store_ids) == sorted([id1, id2]) + await service.ack(trig) + assert store.get(id1)["status"] == "DONE" + assert store.get(id2)["status"] == "DONE" + + run(scenario()) + + +class TestRehydrateEdges: + def test_stale_rows_are_settled_not_refired(self, tmp_path): + async def scenario(): + store, queue, service = make_stack(tmp_path) + old_id, _ = store.insert( + source="scheduled", + description="ancient", + fire_at=time.time() - 25 * 3600, + ) + requeued = await service.rehydrate() + assert requeued == 0 + row = store.get(old_id) + assert row["status"] == "DONE" + assert row["resolution"] == "stale" + + run(scenario()) + + def test_overdue_rows_get_catch_up_note(self, tmp_path): + async def scenario(): + store, queue, service = make_stack(tmp_path) + store.insert( + source="scheduled", + description="late task", + fire_at=time.time() - 600, + session_id="s1", + ) + await service.rehydrate() + trig = await asyncio.wait_for(service.next(), timeout=2) + assert "NOTE:" in trig.next_action_description + assert trig.payload.get("is_catch_up") is True + + run(scenario()) + + def test_waiting_for_reply_round_trips(self, tmp_path): + async def scenario(): + store, queue, service = make_stack(tmp_path) + await service.emit( + spec(waiting_for_reply=True, fire_at=time.time() + 9999) + ) + # crash before it fires + + store2, queue2, service2 = make_stack(tmp_path) + await service2.rehydrate() + triggers = await queue2.list_triggers() + assert len(triggers) == 1 + assert triggers[0].waiting_for_reply is True + + run(scenario()) + + +class TestFireMirroring: + def test_fire_persists_pending_message(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + result = await service.emit(spec(fire_at=time.time() + 9999)) + fired = await service.fire("s1", message="hello", platform="Telegram") + assert fired + row = store.get(result.trigger_id) + assert '"pending_user_message": "hello"' in row["payload_json"] + # in-memory trigger fires now and carries the message + trig = await asyncio.wait_for(service.next(), timeout=2) + assert trig.payload["pending_user_message"] == "hello" + + run(scenario()) diff --git a/tests/test_trigger_store.py b/tests/test_trigger_store.py new file mode 100644 index 00000000..f5a5ae33 --- /dev/null +++ b/tests/test_trigger_store.py @@ -0,0 +1,189 @@ +# -*- coding: utf-8 -*- +"""Unit tests for the durable trigger store (issue #321, Phase 1).""" + +import sqlite3 +import time + +from app.triggers.store import TriggerStore + + +def make_store(tmp_path): + return TriggerStore(db_path=str(tmp_path / "sessions.db")) + + +def insert_basic(store, **overrides): + kwargs = dict( + source="scheduled", + description="do the thing", + fire_at=time.time(), + priority=50, + session_id="s1", + payload={"type": "scheduled"}, + dedup_key=None, + ) + kwargs.update(overrides) + return store.insert(**kwargs) + + +class TestInsertAndDedup: + def test_insert_creates_pending_row(self, tmp_path): + store = make_store(tmp_path) + row_id, created = insert_basic(store) + assert created + row = store.get(row_id) + assert row["status"] == "PENDING" + assert row["source"] == "scheduled" + assert row["session_id"] == "s1" + assert row["attempts"] == 0 + + def test_active_dedup_key_blocks_duplicate(self, tmp_path): + store = make_store(tmp_path) + id1, created1 = insert_basic(store, dedup_key="scheduled-once:abc") + id2, created2 = insert_basic(store, dedup_key="scheduled-once:abc") + assert created1 + assert not created2 + assert id2 == id1 # existing active row returned + + def test_claimed_row_still_blocks_duplicate(self, tmp_path): + store = make_store(tmp_path) + id1, _ = insert_basic(store, dedup_key="k") + store.claim([id1]) + id2, created2 = insert_basic(store, dedup_key="k") + assert not created2 + assert id2 == id1 + + def test_settled_row_does_not_block_refire(self, tmp_path): + # A DONE resume:{task_id} must not block a re-resume after the next + # restart — that's why dedup is a partial index over active rows. + store = make_store(tmp_path) + id1, _ = insert_basic(store, dedup_key="resume:t1") + store.ack([id1]) + id2, created2 = insert_basic(store, dedup_key="resume:t1") + assert created2 + assert id2 != id1 + + def test_null_dedup_keys_never_collide(self, tmp_path): + store = make_store(tmp_path) + _, c1 = insert_basic(store, dedup_key=None) + _, c2 = insert_basic(store, dedup_key=None) + assert c1 and c2 + + +class TestLifecycle: + def test_claim_ack(self, tmp_path): + store = make_store(tmp_path) + row_id, _ = insert_basic(store) + store.claim([row_id]) + row = store.get(row_id) + assert row["status"] == "CLAIMED" + assert row["attempts"] == 1 + assert row["lease_until"] is not None + + store.ack([row_id]) + row = store.get(row_id) + assert row["status"] == "DONE" + assert row["resolution"] == "completed" + assert row["lease_until"] is None + + def test_fail_records_error(self, tmp_path): + store = make_store(tmp_path) + row_id, _ = insert_basic(store) + store.claim([row_id]) + store.fail([row_id], error="ValueError: boom") + row = store.get(row_id) + assert row["status"] == "FAILED" + assert row["resolution"] == "failed" + assert "boom" in row["last_error"] + + def test_supersede_marks_replacement(self, tmp_path): + store = make_store(tmp_path) + old_id, _ = insert_basic(store) + new_id, _ = insert_basic(store) + store.supersede([old_id], by_id=new_id) + row = store.get(old_id) + assert row["status"] == "DONE" + assert row["resolution"] == "superseded" + assert row["superseded_by"] == new_id + + def test_cancel_sessions_settles_only_active(self, tmp_path): + store = make_store(tmp_path) + a, _ = insert_basic(store, session_id="s1") + b, _ = insert_basic(store, session_id="s1") + store.ack([b]) # already settled — must keep resolution "completed" + c, _ = insert_basic(store, session_id="s2") + + count = store.cancel_sessions(["s1"]) + assert count == 1 + assert store.get(a)["resolution"] == "cancelled" + assert store.get(b)["resolution"] == "completed" + assert store.get(c)["status"] == "PENDING" + + +class TestBootRecovery: + def test_reclaim_claimed(self, tmp_path): + store = make_store(tmp_path) + a, _ = insert_basic(store) + b, _ = insert_basic(store) + store.claim([a]) + store.ack([b]) + + reclaimed = store.reclaim_claimed() + assert reclaimed == 1 + assert store.get(a)["status"] == "PENDING" + assert store.get(a)["attempts"] == 1 # attempt history preserved + assert store.get(b)["status"] == "DONE" + + def test_load_pending_ordered_and_filtered(self, tmp_path): + store = make_store(tmp_path) + late, _ = insert_basic(store, fire_at=2000.0) + early, _ = insert_basic(store, fire_at=1000.0) + done, _ = insert_basic(store, fire_at=500.0) + store.ack([done]) + + rows = store.load_pending() + assert [r["id"] for r in rows] == [early, late] + + +class TestFireMirroring: + def test_update_for_fire_patches_active_rows(self, tmp_path): + store = make_store(tmp_path) + active, _ = insert_basic(store, session_id="s1", fire_at=9999999999.0) + settled, _ = insert_basic(store, session_id="s1") + store.ack([settled]) + + updated = store.update_for_fire( + "s1", 123.0, {"pending_user_message": "hi"} + ) + assert updated == 1 + row = store.get(active) + assert row["fire_at"] == 123.0 + assert '"pending_user_message": "hi"' in row["payload_json"] + + +class TestSchemaAndReset: + def test_clear_all(self, tmp_path): + store = make_store(tmp_path) + insert_basic(store) + insert_basic(store) + store.clear_all() + assert store.count_by_status() == {} + + def test_legacy_table_without_dedup_key_is_replaced(self, tmp_path): + db_path = str(tmp_path / "sessions.db") + with sqlite3.connect(db_path) as conn: + conn.execute("CREATE TABLE triggers (id TEXT PRIMARY KEY, blob TEXT)") + conn.execute("INSERT INTO triggers VALUES ('x', 'y')") + conn.commit() + + store = TriggerStore(db_path=db_path) + row_id, created = insert_basic(store) + assert created + assert store.get(row_id)["status"] == "PENDING" + + def test_reopen_preserves_rows(self, tmp_path): + db_path = str(tmp_path / "sessions.db") + store = TriggerStore(db_path=db_path) + row_id, _ = insert_basic(store) + + store2 = TriggerStore(db_path=db_path) # simulated restart + assert store2.get(row_id)["status"] == "PENDING" From b7068523a5d45d6ceee90df9d22a3d4d6004b564 Mon Sep 17 00:00:00 2001 From: ahmad-ajmal Date: Fri, 12 Jun 2026 01:54:06 +0100 Subject: [PATCH 2/5] all trigger producers migrated to emit with defined types --- app/agent_base.py | 186 ++++++++++++----------- app/data/action/schedule_task.py | 48 +----- app/living_ui/manager.py | 85 ++++++++--- app/triggers/sources.py | 29 +++- app/ui_layer/adapters/browser_adapter.py | 76 ++++----- tests/test_trigger_sources.py | 110 ++++++++++++++ 6 files changed, 341 insertions(+), 193 deletions(-) create mode 100644 tests/test_trigger_sources.py diff --git a/app/agent_base.py b/app/agent_base.py index 76295c84..40ec74b2 100644 --- a/app/agent_base.py +++ b/app/agent_base.py @@ -87,6 +87,13 @@ from app.state.state_manager import StateManager from app.state.agent_state import STATE from app.trigger import Trigger, TriggerQueue +from app.triggers import ( + TriggerService, + TriggerSource, + TriggerSpec, + TriggerStore, + resume_dedup_key, +) from app.prompt import ROUTE_TO_SESSION_PROMPT from app.state.types import ReasoningResult from agent_core.core.task import Task @@ -261,13 +268,7 @@ def __init__( llm=self.llm, route_to_session_prompt=ROUTE_TO_SESSION_PROMPT, ) - - # Durable trigger layer (issue #321): triggers are written to - # sessions.db before they can run; the queue stays as the in-memory - # ordering primitive. The service registers itself as the queue's - # lifecycle listener so dropped triggers settle their rows. - from app.triggers import TriggerService, TriggerStore - + self.trigger_store = TriggerStore() self.trigger_service = TriggerService(self.trigger_store, self.triggers) @@ -612,8 +613,6 @@ async def _process_memory_at_startup(self) -> None: processing trigger if needed. The trigger goes through normal processing flow which creates the task and executes it. """ - import time - # Check if memory is enabled if not is_memory_enabled(): logger.info("[MEMORY] Memory is disabled, skipping startup processing") @@ -644,17 +643,18 @@ async def _process_memory_at_startup(self) -> None: ) # Fire a memory_processing trigger (not scheduled, so won't reschedule) - trigger = Trigger( - fire_at=time.time(), - priority=50, - next_action_description="Process unprocessed events into long-term memory (startup)", - payload={ - "type": "memory_processing", - "scheduled": False, # Don't reschedule after this - }, - session_id="memory_processing_startup", + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.MEMORY, + description="Process unprocessed events into long-term memory (startup)", + priority=50, + payload={ + "type": "memory_processing", + "scheduled": False, # Don't reschedule after this + }, + session_id="memory_processing_startup", + ) ) - await self.triggers.put(trigger) except Exception as e: logger.warning(f"[MEMORY] Failed to process memory at startup: {e}") @@ -748,14 +748,17 @@ async def _handle_memory_processing_trigger(self) -> bool: # Queue trigger to start the task. Lock is now owned by the task and # will be released by TaskManager when the task ends. - trigger = Trigger( - fire_at=time.time(), - priority=60, - next_action_description="Process unprocessed events into long-term memory", - session_id=task_id, - payload={}, + # Source is TASK_CONTINUATION (not MEMORY): this trigger starts the + # already-created task via the session workflows — a MEMORY source + # would re-enter the memory-request branch in react(). + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.TASK_CONTINUATION, + description="Process unprocessed events into long-term memory", + priority=60, + session_id=task_id, + ) ) - await self.triggers.put(trigger) logger.info( f"[MEMORY] Queued trigger for memory processing task: {task_id}" ) @@ -822,18 +825,34 @@ async def _initialize_session(self, gui_mode: bool | None, session_id: str) -> N # ----- Mode Checks ----- + # Classification is source-first (typed, set once at emit time), with a + # payload["type"] fallback for triggers from legacy put() producers and + # scheduler-config entries that inject a type via their custom payload. + # The fallback is removed in Phase 5 once nothing produces bare types. + def _is_memory_trigger(self, trigger: Trigger) -> bool: - """Check if trigger is for memory processing.""" - return trigger.payload.get("type") == "memory_processing" + """Check if trigger is a memory-processing request.""" + return ( + trigger.source == TriggerSource.MEMORY + or trigger.payload.get("type") == "memory_processing" + ) def _is_proactive_trigger(self, trigger: Trigger) -> bool: - """Check if trigger is for proactive processing (heartbeat or planner).""" + """Check if trigger is a proactive-processing request (heartbeat or planner).""" + if trigger.source in ( + TriggerSource.PROACTIVE_HEARTBEAT, + TriggerSource.PROACTIVE_PLANNER, + ): + return True trigger_type = trigger.payload.get("type", "") return trigger_type in ("proactive_heartbeat", "proactive_planner") def _is_restart_notice_trigger(self, trigger: Trigger) -> bool: """Check if trigger is the consolidated post-restart notice (issue #280).""" - return trigger.payload.get("type") == "restart_notice" + return ( + trigger.source == TriggerSource.RESTART_NOTICE + or trigger.payload.get("type") == "restart_notice" + ) def _is_gui_task_mode(self, session_id: str | None = None) -> bool: """Check if in GUI task execution mode.""" @@ -920,8 +939,6 @@ async def _handle_proactive_heartbeat(self, frequency: str) -> bool: frequency: Ignored (kept for backward-compat with old configs that still pass a single frequency). """ - import time - # Collect due tasks across ALL frequencies all_due_tasks = self.proactive_manager.get_all_due_tasks() if not all_due_tasks: @@ -952,22 +969,20 @@ async def _handle_proactive_heartbeat(self, frequency: str) -> bool: f"[PROACTIVE] Created unified heartbeat task: {task_id} ({summary})" ) - trigger = Trigger( - fire_at=time.time(), - priority=50, - next_action_description=f"Execute due proactive tasks ({summary})", - session_id=task_id, - payload={}, + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.TASK_CONTINUATION, + description=f"Execute due proactive tasks ({summary})", + priority=50, + session_id=task_id, + ) ) - await self.triggers.put(trigger) logger.info(f"[PROACTIVE] Queued trigger for heartbeat task: {task_id}") return True async def _handle_proactive_planner(self, scope: str) -> bool: """Create planner task for the given scope (day, week, month).""" - import time - skill_name = f"{scope}-planner" task_id = self.task_manager.create_task( @@ -981,14 +996,14 @@ async def _handle_proactive_planner(self, scope: str) -> bool: logger.info(f"[PROACTIVE] Created planner task: {task_id} for {scope}") # Queue trigger to start the task - trigger = Trigger( - fire_at=time.time(), - priority=50, - next_action_description=f"Execute {scope} planner task", - session_id=task_id, - payload={}, - ) - await self.triggers.put(trigger) + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.TASK_CONTINUATION, + description=f"Execute {scope} planner task", + priority=50, + session_id=task_id, + ) + ) logger.info(f"[PROACTIVE] Queued trigger for planner task: {task_id}") return True @@ -1740,16 +1755,17 @@ async def _pause_task_for_limit_choice(self, session_id: str) -> None: # Create a long-delay trigger so the task stays alive try: - await self.triggers.put( - Trigger( + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.LIMIT_REACHED, + description="Waiting for user decision on limit reached", fire_at=time.time() + 10800, priority=5, - next_action_description="Waiting for user decision on limit reached", session_id=session_id, payload={"gui_mode": STATE.gui_mode}, waiting_for_reply=True, - ), - skip_merge=True, + skip_merge=True, + ) ) except Exception as e: logger.error( @@ -1805,8 +1821,8 @@ async def handle_limit_continue(self, session_id: str) -> None: ) ) - # Fire the trigger to resume execution - await self.triggers.fire(session_id) + # Fire the trigger to resume execution (durably mirrored to the store) + await self.trigger_service.fire(session_id) async def handle_limit_abort(self, session_id: str) -> None: """User chose to abort after reaching limit.""" @@ -1934,18 +1950,20 @@ async def _create_new_trigger(self, new_session_id, action_output, STATE): # simple task = 5, complex task = 7 task_priority = 5 if self.task_manager.is_simple_task() else 7 - # Build and enqueue trigger safely + # Build and enqueue trigger safely. No dedup key: a newer + # continuation supersedes the queued one via session replacement. try: - await self.triggers.put( - Trigger( + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.TASK_CONTINUATION, + description=next_action_desc, fire_at=fire_at, priority=task_priority, - next_action_description=next_action_desc, session_id=new_session_id, payload=trigger_payload, waiting_for_reply=wait_for_user_reply, - ), - skip_merge=True, # Session is already explicitly set, no LLM merge check needed + skip_merge=True, # Session is already explicitly set, no LLM merge check needed + ) ) except Exception as e: logger.error( @@ -2284,7 +2302,10 @@ async def _fire_session( Returns True if the trigger was found and fired, False otherwise. """ - fired = await self.triggers.fire( + # Routed through the service so the attached user message is durably + # persisted before the in-memory retarget — a crash mid-react can no + # longer lose it. + fired = await self.trigger_service.fire( session_id, message=chat_content, platform=platform, @@ -2407,18 +2428,18 @@ async def _create_new_session_trigger( if platform and platform.lower() != "craftbot interface": platform_hint = f" from {platform} (reply on {platform}, NOT send_message)" - await self.triggers.put( - Trigger( - fire_at=time.time(), - priority=3, - next_action_description=( + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.USER_MESSAGE, + description=( "Please perform action that best suit this user chat " f"you just received{platform_hint}: {chat_content}" ), + priority=3, session_id=await self._generate_unique_session_id(), payload=trigger_payload, - ), - skip_merge=True, + skip_merge=True, + ) ) # ───────────────────────────────────────────────────────────────────── @@ -2937,8 +2958,6 @@ async def trigger_soft_onboarding(self, reset: bool = False) -> Optional[str]: """ from app.onboarding import onboarding_manager from app.onboarding.soft.task_creator import create_soft_onboarding_task - from app.trigger import Trigger - import time # Prevent double-triggering (multiple adapters/paths may call this) if not reset and self._soft_onboarding_triggered: @@ -2953,14 +2972,15 @@ async def trigger_soft_onboarding(self, reset: bool = False) -> Optional[str]: task_id = create_soft_onboarding_task(self.task_manager) # Fire trigger to start the task - trigger = Trigger( - fire_at=time.time(), - priority=1, - next_action_description="Begin user profile interview", - session_id=task_id, - payload={"onboarding": True}, + await self.trigger_service.emit( + TriggerSpec( + source=TriggerSource.ONBOARDING, + description="Begin user profile interview", + priority=1, + session_id=task_id, + payload={"onboarding": True}, + ) ) - await self.triggers.put(trigger) logger.info(f"[ONBOARDING] Triggered soft onboarding task: {task_id}") return task_id @@ -3480,8 +3500,6 @@ async def _schedule_restored_task_triggers(self) -> None: # running agent loop, after the watcher is live, so it surfaces in # the interface just like the resumed tasks' own messages. try: - from app.triggers import TriggerSource, TriggerSpec - # No dedup key: each boot composes a fresh notice. A stale # rehydrated notice row from a crashed boot is superseded by # this emit via the queue's same-session replacement. @@ -3517,12 +3535,6 @@ async def _schedule_restored_task_triggers(self) -> None: is_simple = getattr(task, "mode", "complex") == "simple" restore_priority = 5 if is_simple else 7 - from app.triggers import ( - TriggerSource, - TriggerSpec, - resume_dedup_key, - ) - if task.waiting_for_user_reply: result = await self.trigger_service.emit( TriggerSpec( diff --git a/app/data/action/schedule_task.py b/app/data/action/schedule_task.py index a8290580..7fe7f441 100644 --- a/app/data/action/schedule_task.py +++ b/app/data/action/schedule_task.py @@ -108,10 +108,6 @@ async def schedule_task(input_data: dict) -> dict: """Add a new scheduled task or queue an immediate trigger.""" import app.internal_action_interface as iai from datetime import datetime - import asyncio - import time - import uuid - from agent_core import Trigger scheduler = iai.InternalActionInterface.scheduler if scheduler is None: @@ -152,7 +148,8 @@ async def schedule_task(input_data: dict) -> dict: ), } - # Handle immediate execution + # Handle immediate execution — queue_immediate_trigger emits durably + # via TriggerService when the scheduler has one wired (issue #321). if schedule_expr.lower() == "immediate": return await scheduler.queue_immediate_trigger( name=name, @@ -164,47 +161,6 @@ async def schedule_task(input_data: dict) -> dict: payload=payload, ) - session_id = f"immediate_{uuid.uuid4().hex[:8]}_{int(time.time())}" - - trigger_payload = { - "type": "scheduled", - "schedule_id": f"immediate_{uuid.uuid4().hex[:8]}", - "schedule_name": name, - "instruction": instruction, - "mode": mode, - "action_sets": action_sets, - "skills": skills, - **payload, - } - - # TODO: Should not have to create additional trigger (create using queue_immediate_trigger) - # Workaround for now - trigger = Trigger( - fire_at=time.time(), - priority=priority, - next_action_description=f"[Immediate] {name}: {instruction}", - payload=trigger_payload, - session_id=session_id, - ) - - trigger_queue = scheduler._trigger_queue - if trigger_queue is None: - return {"status": "error", "error": "Trigger queue not initialized"} - - try: - asyncio.get_running_loop() - asyncio.create_task(trigger_queue.put(trigger)) - except RuntimeError: - asyncio.run(trigger_queue.put(trigger)) - - return { - "status": "ok", - "schedule_id": session_id, - "name": name, - "scheduled_for": "immediate", - "message": f"Task '{name}' queued for immediate execution (session: {session_id})", - } - # Parse schedule to determine if it's recurring or one-time from app.scheduler.parser import ScheduleParser diff --git a/app/living_ui/manager.py b/app/living_ui/manager.py index 40f34774..2990b329 100644 --- a/app/living_ui/manager.py +++ b/app/living_ui/manager.py @@ -116,6 +116,7 @@ def __init__(self, workspace_root: Path, template_path: Path): # Task and trigger management (set via bind_task_manager) self._task_manager: Optional["TaskManager"] = None self._trigger_queue: Optional["TriggerQueue"] = None + self._trigger_service = None # Optional[TriggerService] — durable emit path # Watchdog state self._watchdog_task: Optional[asyncio.Task] = None @@ -129,7 +130,10 @@ def __init__(self, workspace_root: Path, template_path: Path): self._load_projects() def bind_task_manager( - self, task_manager: "TaskManager", trigger_queue: "TriggerQueue" + self, + task_manager: "TaskManager", + trigger_queue: "TriggerQueue", + trigger_service=None, ) -> None: """ Bind the task manager and trigger queue for creating development tasks. @@ -137,9 +141,12 @@ def bind_task_manager( Args: task_manager: TaskManager instance for creating tasks trigger_queue: TriggerQueue instance for firing triggers + trigger_service: Optional TriggerService for durable emits + (issue #321); falls back to direct queue puts when None. """ self._task_manager = task_manager self._trigger_queue = trigger_queue + self._trigger_service = trigger_service logger.info("[LIVING_UI] Task manager and trigger queue bound") # ======================================================================== @@ -475,17 +482,33 @@ async def _escalate_crash(self, project_id: str, crash_targets: List[str]) -> No selected_skills=["living-ui-creator"], ) - trigger = Trigger( - fire_at=time.time(), - priority=30, # Higher priority than normal creation tasks - next_action_description=f"[Living UI] Fix crash: {project.name}", - session_id=task_id, - payload={ - "type": "living_ui_crash_fix", - "project_id": project_id, - }, - ) - await self._trigger_queue.put(trigger) + if self._trigger_service is not None: + from app.triggers import TriggerSource, TriggerSpec + + await self._trigger_service.emit( + TriggerSpec( + source=TriggerSource.LIVING_UI_CRASH_FIX, + description=f"[Living UI] Fix crash: {project.name}", + priority=30, # Higher priority than normal creation tasks + session_id=task_id, + payload={ + "type": "living_ui_crash_fix", + "project_id": project_id, + }, + ) + ) + else: + trigger = Trigger( + fire_at=time.time(), + priority=30, # Higher priority than normal creation tasks + next_action_description=f"[Living UI] Fix crash: {project.name}", + session_id=task_id, + payload={ + "type": "living_ui_crash_fix", + "project_id": project_id, + }, + ) + await self._trigger_queue.put(trigger) project.task_id = task_id self._save_projects() @@ -2565,17 +2588,33 @@ async def create_development_task(self, project_id: str) -> Optional[str]: self.update_project_status(project_id, "creating") # Create and fire the trigger to start execution - trigger = Trigger( - fire_at=time.time(), - priority=50, - next_action_description=f"[Living UI] Create: {project.name}", - session_id=task_id, - payload={ - "type": "living_ui_development", - "project_id": project_id, - }, - ) - await self._trigger_queue.put(trigger) + if self._trigger_service is not None: + from app.triggers import TriggerSource, TriggerSpec + + await self._trigger_service.emit( + TriggerSpec( + source=TriggerSource.LIVING_UI_DEV, + description=f"[Living UI] Create: {project.name}", + priority=50, + session_id=task_id, + payload={ + "type": "living_ui_development", + "project_id": project_id, + }, + ) + ) + else: + trigger = Trigger( + fire_at=time.time(), + priority=50, + next_action_description=f"[Living UI] Create: {project.name}", + session_id=task_id, + payload={ + "type": "living_ui_development", + "project_id": project_id, + }, + ) + await self._trigger_queue.put(trigger) logger.info( f"[LIVING_UI] Created task {task_id} and fired trigger for project {project_id}" diff --git a/app/triggers/sources.py b/app/triggers/sources.py index 2554fac8..63a726f7 100644 --- a/app/triggers/sources.py +++ b/app/triggers/sources.py @@ -15,17 +15,44 @@ class TriggerSource(str, Enum): - """Typed origin of a trigger. Stored in the ``triggers.source`` column.""" + """Typed origin of a trigger. Stored in the ``triggers.source`` column. + Replaces the stringly-typed ``payload["type"]`` convention: every + producer states its origin once, at emit time, instead of handlers + string-matching payload fields across files. + """ + + # User input + USER_MESSAGE = "user_message" + # Scheduler SCHEDULED = "scheduled" SCHEDULED_ONCE = "scheduled_once" SCHEDULED_IMMEDIATE = "scheduled_immediate" + # Task lifecycle + TASK_CONTINUATION = "task_continuation" RESUME = "resume" RESTART_NOTICE = "restart_notice" + LIMIT_REACHED = "limit_reached" + # Background workflows + MEMORY = "memory" + PROACTIVE_HEARTBEAT = "proactive_heartbeat" + PROACTIVE_PLANNER = "proactive_planner" + ONBOARDING = "onboarding" + SKILL_WORKFLOW = "skill_workflow" + # Living UI + LIVING_UI_DEV = "living_ui_dev" + LIVING_UI_CRASH_FIX = "living_ui_crash_fix" + LIVING_UI_IMPORT = "living_ui_import" # Catch-all for producers not yet migrated to TriggerService. LEGACY = "legacy" +# Sources whose triggers start a freshly-created task get no dedup key: the +# task id itself is new each time, so the trigger's identity IS the task. +# Dedup keys exist for work whose identity predates the trigger (a schedule +# occurrence, a task resume) where a crash retry could mint a duplicate. + + def scheduled_dedup_key(schedule_id: str, fire_target: float) -> str: """Dedup key for one fire of a recurring schedule. diff --git a/app/ui_layer/adapters/browser_adapter.py b/app/ui_layer/adapters/browser_adapter.py index 57f8a42d..85f6c50d 100644 --- a/app/ui_layer/adapters/browser_adapter.py +++ b/app/ui_layer/adapters/browser_adapter.py @@ -969,7 +969,9 @@ def __init__( ) # Bind task_manager and trigger_queue for task creation agent = self._controller.agent - self._living_ui_manager.bind_task_manager(agent.task_manager, agent.triggers) + self._living_ui_manager.bind_task_manager( + agent.task_manager, agent.triggers, trigger_service=agent.trigger_service + ) # Clean up orphan processes and folders from previous sessions self._living_ui_manager.cleanup_on_startup() @@ -3414,20 +3416,23 @@ async def _handle_task_resume(self, task_id: str, message: str) -> None: # _create_new_trigger does post-action. is_simple = getattr(task, "mode", "complex") == "simple" resume_priority = 5 if is_simple else 7 - await agent.triggers.put( - Trigger( - fire_at=_time.time(), - priority=resume_priority, - next_action_description=( + from app.triggers import TriggerSource, TriggerSpec, resume_dedup_key + + await agent.trigger_service.emit( + TriggerSpec( + source=TriggerSource.RESUME, + description=( "Task was resumed by the user. Review the event stream " "history. Do NOT call task_end immediately. If the task " "was previously completed, you MUST ask the user for " "their intent FIRST before taking any action." ), + priority=resume_priority, session_id=task_id, payload={"gui_mode": STATE.gui_mode}, - ), - skip_merge=True, + dedup_key=resume_dedup_key(task_id), + skip_merge=True, + ) ) await self._broadcast( @@ -4068,16 +4073,16 @@ async def _err(msg: str) -> None: ) # ---- Queue trigger so execution actually starts --------- - from app.trigger import Trigger - - trigger = Trigger( - fire_at=time.time(), - priority=60, - next_action_description=f"{verb} skill '{target}' from completed task", - session_id=new_task_id, - payload={}, + from app.triggers import TriggerSource, TriggerSpec + + await agent.trigger_service.emit( + TriggerSpec( + source=TriggerSource.SKILL_WORKFLOW, + description=f"{verb} skill '{target}' from completed task", + priority=60, + session_id=new_task_id, + ) ) - await agent.triggers.put(trigger) # Acknowledge in the chat immediately so the user sees the work # being picked up. The agent will follow up with a presentation @@ -4914,17 +4919,16 @@ async def _handle_memory_process_trigger(self) -> None: if task_id: # Queue trigger to start the task (same as _handle_memory_processing_trigger) - import time - from app.trigger import Trigger - - trigger = Trigger( - fire_at=time.time(), - priority=60, - next_action_description="Process unprocessed events into long-term memory", - session_id=task_id, - payload={}, + from app.triggers import TriggerSource, TriggerSpec + + await agent.trigger_service.emit( + TriggerSpec( + source=TriggerSource.TASK_CONTINUATION, + description="Process unprocessed events into long-term memory", + priority=60, + session_id=task_id, + ) ) - await agent.triggers.put(trigger) await self._broadcast( { @@ -6503,21 +6507,21 @@ async def _handle_living_ui_import(self, source: str, name: str) -> None: ) if task_id: - from app.trigger import Trigger - import time + from app.triggers import TriggerSource, TriggerSpec # Link the task to the placeholder so question-mirroring and todo # broadcasts (keyed by task id) target this tab. self._living_ui_manager.set_project_task(project_id, task_id) - trigger = Trigger( - fire_at=time.time(), - priority=50, - next_action_description=f"[Living UI] Import: {name}", - session_id=task_id, - payload={"type": "living_ui_import", "source": source}, + await self._controller.agent.trigger_service.emit( + TriggerSpec( + source=TriggerSource.LIVING_UI_IMPORT, + description=f"[Living UI] Import: {name}", + priority=50, + session_id=task_id, + payload={"type": "living_ui_import", "source": source}, + ) ) - await self._controller.agent.triggers.put(trigger) else: # Couldn't create the task — don't leave a stuck "creating" tab. await self._broadcast( diff --git a/tests/test_trigger_sources.py b/tests/test_trigger_sources.py new file mode 100644 index 00000000..dd7014a1 --- /dev/null +++ b/tests/test_trigger_sources.py @@ -0,0 +1,110 @@ +# -*- coding: utf-8 -*- +"""Phase 2 tests: TriggerSource taxonomy, dedup-key builders, and react() +classification equivalence (source-first with payload["type"] fallback).""" + +import time + +import pytest + +from agent_core.core.trigger import Trigger + +from app.agent_base import AgentBase +from app.triggers import ( + TriggerSource, + resume_dedup_key, + scheduled_dedup_key, + scheduled_once_dedup_key, +) + + +def trig(source="", payload=None): + return Trigger( + fire_at=time.time(), + priority=50, + next_action_description="x", + payload=payload or {}, + session_id="s1", + source=source, + ) + + +class TestDedupKeyBuilders: + @pytest.mark.parametrize( + "builder, args, expected", + [ + (scheduled_once_dedup_key, ("abc123",), "scheduled-once:abc123"), + (resume_dedup_key, ("task42",), "resume:task42"), + # 120 seconds apart within the same minute bucket → same key + (scheduled_dedup_key, ("s1", 600.0), "scheduled:s1:10"), + (scheduled_dedup_key, ("s1", 659.9), "scheduled:s1:10"), + (scheduled_dedup_key, ("s1", 660.0), "scheduled:s1:11"), + ], + ) + def test_builders(self, builder, args, expected): + assert builder(*args) == expected + + def test_same_fire_retried_dedups_next_occurrence_does_not(self): + fire = 1_700_000_000.0 + assert scheduled_dedup_key("a", fire) == scheduled_dedup_key("a", fire + 30) + assert scheduled_dedup_key("a", fire) != scheduled_dedup_key("a", fire + 3600) + assert scheduled_dedup_key("a", fire) != scheduled_dedup_key("b", fire) + + +class TestReactClassification: + """Source-based classification must match the legacy payload["type"] + behavior exactly — for migrated producers AND for legacy/scheduler-config + triggers that still carry only a payload type.""" + + # Unbound calls (staticmethod so the test class doesn't rebind self): + # the classifiers don't touch self. + is_memory = staticmethod(AgentBase._is_memory_trigger) + is_proactive = staticmethod(AgentBase._is_proactive_trigger) + is_restart = staticmethod(AgentBase._is_restart_notice_trigger) + + def test_source_based(self): + assert self.is_memory(None, trig(source=TriggerSource.MEMORY)) + assert self.is_proactive( + None, trig(source=TriggerSource.PROACTIVE_HEARTBEAT) + ) + assert self.is_proactive(None, trig(source=TriggerSource.PROACTIVE_PLANNER)) + assert self.is_restart(None, trig(source=TriggerSource.RESTART_NOTICE)) + + def test_legacy_payload_fallback(self): + assert self.is_memory(None, trig(payload={"type": "memory_processing"})) + assert self.is_proactive(None, trig(payload={"type": "proactive_heartbeat"})) + assert self.is_proactive(None, trig(payload={"type": "proactive_planner"})) + assert self.is_restart(None, trig(payload={"type": "restart_notice"})) + + def test_scheduler_config_payload_overrides_scheduled_source(self): + # scheduler_config.json entries inject their own payload["type"] + # (e.g. proactive_heartbeat) on top of a SCHEDULED-source trigger — + # they must still classify as proactive/memory. + heartbeat = trig( + source=TriggerSource.SCHEDULED, + payload={"type": "proactive_heartbeat"}, + ) + memory = trig( + source=TriggerSource.SCHEDULED, + payload={"type": "memory_processing", "scheduled": True}, + ) + assert self.is_proactive(None, heartbeat) + assert self.is_memory(None, memory) + + def test_task_continuation_never_classified_as_request(self): + # Triggers that START an already-created task (memory task, heartbeat + # task, planner task) are TASK_CONTINUATION — routing them into the + # request branches would create duplicate tasks. + t = trig(source=TriggerSource.TASK_CONTINUATION) + assert not self.is_memory(None, t) + assert not self.is_proactive(None, t) + assert not self.is_restart(None, t) + + def test_plain_user_trigger_classified_nowhere(self): + t = trig(source=TriggerSource.USER_MESSAGE) + assert not self.is_memory(None, t) + assert not self.is_proactive(None, t) + assert not self.is_restart(None, t) + legacy = trig() # no source, no type — pre-migration shape + assert not self.is_memory(None, legacy) + assert not self.is_proactive(None, legacy) + assert not self.is_restart(None, legacy) From 8c697a9ef40cc9a3d754dbe18f13666f410bff1c Mon Sep 17 00:00:00 2001 From: ahmad-ajmal Date: Fri, 12 Jun 2026 02:15:30 +0100 Subject: [PATCH 3/5] Session Router, queue routing --- agent_core/core/impl/trigger/queue.py | 385 +++-------------------- agent_core/core/protocols/trigger.py | 8 - agent_core/core/trigger.py | 12 +- app/agent_base.py | 312 +++++------------- app/triggers/__init__.py | 2 + app/triggers/router.py | 284 +++++++++++++++++ app/triggers/service.py | 90 +++++- app/triggers/store.py | 9 + tests/test_trigger_router_and_parking.py | 163 ++++++++++ tests/test_trigger_service.py | 34 +- 10 files changed, 675 insertions(+), 624 deletions(-) create mode 100644 app/triggers/router.py create mode 100644 tests/test_trigger_router_and_parking.py diff --git a/agent_core/core/impl/trigger/queue.py b/agent_core/core/impl/trigger/queue.py index ef74aaae..3adb1f75 100644 --- a/agent_core/core/impl/trigger/queue.py +++ b/agent_core/core/impl/trigger/queue.py @@ -2,31 +2,36 @@ """ core.impl.trigger.queue -TriggerQueue implementation - manages agent trigger events with priority ordering. +TriggerQueue implementation - in-memory ordering primitive for triggers. + +The queue holds due-time-ordered triggers and hands them to the single +consumer loop. It is deliberately dumb (issue #321): + +- Durability lives in the app-layer TriggerStore; the queue reports any + trigger it discards unconsumed through a TriggerLifecycleListener so the + store can settle the corresponding rows. +- Session routing lives at the producer layer (SessionRouter); triggers + arrive here with their session already decided. The pre-#321 in-queue LLM + routing was removed — every producer sets a session_id, so it was dead + code in practice. +- Same-session ordering: a new trigger for a session replaces any queued + one ("prefer newest"), so at most one trigger per session is ever queued. """ from __future__ import annotations import asyncio import heapq -import json import logging import time -from collections import defaultdict, OrderedDict -from typing import Dict, List, Optional, Any, TYPE_CHECKING +from typing import Any, Dict, List, Optional, TYPE_CHECKING from agent_core.decorators import profile, OperationCategory from agent_core.core.trigger import Trigger -from agent_core.core.state import get_state_or_none if TYPE_CHECKING: - from agent_core.core.protocols import LLMInterfaceProtocol, TaskManagerProtocol - from agent_core.core.task import Task from agent_core.core.impl.trigger.listener import TriggerLifecycleListener - # TaskManager type alias for backwards compatibility - TaskManager = TaskManagerProtocol - # Logging setup try: from agent_core.utils.logger import logger @@ -42,11 +47,11 @@ class TriggerQueue: def __init__( self, - llm: "LLMInterfaceProtocol", + llm: Any = None, *, route_to_session_prompt: str = "", - task_manager: Optional["TaskManager"] = None, - event_stream_manager: Optional[Any] = None, + task_manager: Any = None, + event_stream_manager: Any = None, ) -> None: """ Initialize a concurrency-safe trigger queue. @@ -57,23 +62,22 @@ def __init__( loops can await triggers without busy waiting. Args: - llm: Interface used to resolve conflicts between competing triggers - for the same session. - route_to_session_prompt: Prompt template for routing triggers to sessions. - Should contain {item_type}, {item_content}, {existing_sessions}, - {source_platform}, and {conversation_id} placeholders. - task_manager: Optional task manager for accessing task details during routing. - event_stream_manager: Optional event stream manager for accessing recent events. + llm: Deprecated, ignored. In-queue LLM routing was removed + (issue #321 Phase 3); routing happens at the producer layer. + route_to_session_prompt: Deprecated, ignored. + task_manager: Deprecated, ignored. + event_stream_manager: Deprecated, ignored. """ + if llm is not None or route_to_session_prompt: + logger.debug( + "[TRIGGER QUEUE] llm/route_to_session_prompt are deprecated " + "and ignored — routing moved to the producer layer" + ) self._heap: List[Trigger] = [] self._active: Dict[ str, Trigger ] = {} # Triggers being processed (session_id -> trigger) self._cv = asyncio.Condition() - self.llm = llm - self._route_to_session_prompt = route_to_session_prompt - self._task_manager = task_manager - self._event_stream_manager = event_stream_manager self._lifecycle_listener: Optional["TriggerLifecycleListener"] = None def set_lifecycle_listener( @@ -101,26 +105,6 @@ def _notify_evicted( except Exception as e: logger.warning(f"[TRIGGER QUEUE] Lifecycle listener failed: {e}") - def set_task_manager(self, task_manager: Optional["TaskManager"]) -> None: - """Set the task manager for accessing task details during routing. - - This allows late binding of task_manager when it's created after TriggerQueue. - - Args: - task_manager: The task manager instance to use for routing context. - """ - self._task_manager = task_manager - - def set_event_stream_manager(self, event_stream_manager: Optional[Any]) -> None: - """Set the event stream manager for accessing recent events during routing. - - This allows late binding of event_stream_manager when it's created after TriggerQueue. - - Args: - event_stream_manager: The event stream manager instance. - """ - self._event_stream_manager = event_stream_manager - # ================================================================= # Pretty Printer for Debugging # ================================================================= @@ -146,47 +130,6 @@ def _print_queue(self, label: str) -> None: ) logger.debug("=" * 70 + "\n") - def create_event_stream_state(self) -> str: - """Return formatted event stream content for trigger comparison.""" - state = get_state_or_none() - event_stream = state.event_stream if state else None - if event_stream: - return ( - "Use the event stream to understand the current situation, past agent actions to craft the input parameters:\nEvent stream (oldest to newest):" - f"\n{event_stream}" - ) - return "" - - def create_task_state(self) -> str: - """Return formatted task/plan context for trigger comparison.""" - state = get_state_or_none() - current_task: Optional["Task"] = state.current_task if state else None - if current_task: - # Format task in LLM-friendly way (matching context_engine format) - lines = [ - "", - f"Task: {current_task.name}", - f"Instruction: {current_task.instruction}", - "", - "Todos:", - ] - - if current_task.todos: - for todo in current_task.todos: - if todo.status == "completed": - checkbox = "[x]" - elif todo.status == "in_progress": - checkbox = "[>]" - else: - checkbox = "[ ]" - lines.append(f"{checkbox} {todo.content}") - else: - lines.append("(no todos yet)") - - lines.append("") - return "\n".join(lines) - return "" - async def clear(self) -> None: """ Remove all pending and active triggers from the queue. @@ -206,193 +149,24 @@ async def clear(self) -> None: # PUT # ================================================================= - def _format_sessions_for_routing( - self, - running_tasks: List["Task"], - event_stream_manager: Optional[Any] = None, - ) -> str: - """Format running tasks with rich context for routing prompt. - - Args: - running_tasks: List of currently running tasks from TaskManager - event_stream_manager: Optional event stream manager to retrieve recent events - - Returns: - Formatted string with session context for routing decision - """ - if not running_tasks: - return "No existing sessions." - - sections = [] - for i, task in enumerate(running_tasks, 1): - # Check waiting_for_user_reply state on task - is_waiting = getattr(task, "waiting_for_user_reply", False) - status = "WAITING FOR REPLY" if is_waiting else "ACTIVE" - - lines = [ - f"--- Session {i} ---", - f"Session ID: {task.id}", - f"Status: {status}", - f'Task Name: "{task.name}"', - f'Original Request: "{task.instruction}"', - f"Mode: {task.mode}", - f"Created: {task.created_at}", - ] - - # Todo progress - if task.todos: - completed = sum(1 for t in task.todos if t.status == "completed") - in_progress_todo = next( - (t for t in task.todos if t.status == "in_progress"), None - ) - lines.append(f"Progress: {completed}/{len(task.todos)} todos completed") - if in_progress_todo: - lines.append(f'Currently working on: "{in_progress_todo.content}"') - - # Get recent events from event stream for this task - if event_stream_manager and task.id: - try: - stream = event_stream_manager.get_stream_by_id(task.id) - if stream and stream.tail_events: - # Get last 10 events for better routing context - recent_events = stream.tail_events[-10:] - lines.append("Recent Activity:") - for rec in recent_events: - lines.append(f" - {rec.compact_line()}") - except Exception: - pass # Gracefully handle if event stream not available - - # Add platform/conversation info if available - platform = getattr(task, "platform", "default") - conversation_id = getattr(task, "conversation_id", "N/A") - lines.append(f"Platform: {platform}") - lines.append(f"Conversation ID: {conversation_id}") - - sections.append("\n".join(lines)) - - return "\n\n".join(sections) - @profile("trigger_queue_put", OperationCategory.TRIGGER) async def put(self, trig: Trigger, skip_merge: bool = False) -> None: """ - Insert a trigger into the queue, optionally merging with existing session triggers. + Insert a trigger into the queue, replacing queued same-session triggers. - When a trigger arrives for a session that already has queued work, the - method consults the LLM to generate a new session identifier that - represents the preferred trigger. Existing triggers for that session - are removed so the freshest trigger wins. + When a trigger arrives for a session that already has queued work, + the existing triggers are replaced ("prefer newest") and reported to + the lifecycle listener as superseded. Args: trig: Trigger instance describing when and why the agent should act. - skip_merge: If True, skip LLM-based trigger merging. Use for system - triggers that should not be merged with user triggers. + skip_merge: Deprecated, ignored — kept for call-site compatibility. + (It previously skipped the in-queue LLM routing, which was + removed; same-session replacement was always unconditional.) """ - logger.debug( - f"\n[PUT] Incoming trigger for session={trig.session_id} (skip_merge={skip_merge})" - ) + logger.debug(f"\n[PUT] Incoming trigger for session={trig.session_id}") self._print_queue("BEFORE PUT") - # Get running tasks from TaskManager (the source of truth for active sessions) - # This includes tasks being processed (trigger consumed) AND tasks with queued triggers - running_tasks: List["Task"] = [] - if self._task_manager: - running_tasks = [ - t for t in self._task_manager.tasks.values() if t.status == "running" - ] - - # Skip LLM routing if: - # 1. Trigger already has a session_id assigned (proceed with that session) - # 2. skip_merge is True (already routed at message handler level) - # 3. System triggers (memory_processing, task_execution, scheduled) - trigger_type = trig.payload.get("type", "") - is_system_trigger = trigger_type in ( - "memory_processing", - "task_execution", - "scheduled", - ) - has_session_id = trig.session_id is not None and trig.session_id != "" - - if has_session_id: - logger.debug( - f"[PUT] Trigger already has session_id={trig.session_id}, skipping LLM routing" - ) - elif ( - len(running_tasks) > 0 - and not skip_merge - and not is_system_trigger - and self._route_to_session_prompt - ): - # Use unified routing prompt with rich task context from running tasks - existing_sessions = self._format_sessions_for_routing( - running_tasks, - event_stream_manager=self._event_stream_manager, - ) - - # Build recent conversation context for routing - recent_conversation = "No recent conversation history." - if self._event_stream_manager: - recent_msgs = ( - self._event_stream_manager.get_recent_conversation_messages( - limit=10 - ) - ) - if recent_msgs: - conv_lines = [] - for evt in recent_msgs: - ts = ( - evt.ts.strftime("%Y-%m-%d %H:%M:%S") - if evt.ts - else "unknown" - ) - conv_line = f"[{ts}] [{evt.kind}]: {evt.message}" - if len(conv_line) > 300: - conv_line = conv_line[:297] + "..." - conv_lines.append(conv_line) - recent_conversation = "\n".join(conv_lines) - - # Format prompt with available placeholders - usr_msg = self._route_to_session_prompt.format( - item_type="trigger", - item_content=trig.next_action_description, - source_platform=trig.payload.get("platform", "default"), - conversation_id=trig.payload.get("conversation_id", "N/A"), - existing_sessions=existing_sessions, - recent_conversation=recent_conversation, - current_living_ui_id=trig.payload.get("living_ui_id") - or "(not on a Living UI page)", - ) - - logger.debug(f"[UNIFIED ROUTING PROMPT]:\n{usr_msg}") - response = await self.llm.generate_response_async( - system_prompt="You are a session routing system.", - user_prompt=usr_msg, - ) - logger.debug(f"[UNIFIED ROUTING RESPONSE]: {response}") - - # Parse routing response - try: - routing_result = json.loads(response) - action = routing_result.get("action", "route") - - if action == "route": - matched_session_id = routing_result.get("session_id", "new") - else: # action == "new" or unknown - matched_session_id = "new" - except (json.JSONDecodeError, TypeError): - logger.error("[PUT] Failed to parse routing response JSON") - matched_session_id = "new" - - # Update the incoming trigger's session ID based on routing result - if matched_session_id != "new": - trig.session_id = matched_session_id - logger.debug(f"[PUT] Routed to existing session: {matched_session_id}") - else: - logger.debug("[PUT] Creating new session (no match found)") - else: - logger.debug( - f"[PUT] Skipping LLM routing (no_running_tasks={len(running_tasks) == 0}, skip_merge={skip_merge}, is_system={is_system_trigger})" - ) - async with self._cv: # find all triggers in heap with same session_id same = [t for t in self._heap if t.session_id == trig.session_id] @@ -431,14 +205,16 @@ async def get(self) -> Trigger: """ Retrieve the next trigger to execute, waiting until one is ready. - The method drains all triggers that are ready to fire, merges triggers - belonging to the same session, and returns the highest-priority - combined trigger. If no trigger is ready, it waits until either the - earliest trigger's ``fire_at`` time arrives or a producer notifies the - condition. + Pops the highest-priority due trigger. If no trigger is ready, waits + until either the earliest trigger's ``fire_at`` time arrives or a + producer notifies the condition. + + Same-session replacement in put() guarantees at most one queued + trigger per session, so no cross-trigger merging is needed here + (the pre-#321 merge machinery was removed with that invariant). Returns: - The next merged :class:`Trigger` ready for execution. + The next :class:`Trigger` ready for execution. """ logger.debug("\n[GET] CALLED") self._print_queue("QUEUE BEFORE GET") @@ -454,25 +230,22 @@ async def get(self) -> Trigger: if ready: logger.debug(f"[GET] {len(ready)} trigger(s) are ready") - self._print_queue("READY BEFORE MERGE (GET)") - - merged_ready = self._merge_ready_triggers(ready) - merged_ready.sort(key=lambda t: (t.priority, t.fire_at)) - trig = merged_ready.pop(0) + ready.sort(key=lambda t: (t.priority, t.fire_at)) + trig = ready.pop(0) logger.info( f"[TRIGGER FIRED] session={trig.session_id} | desc={trig.next_action_description}" ) # requeue leftover - for t in merged_ready: + for t in ready: heapq.heappush(self._heap, t) # Track as active so fire() can find it while processing if trig.session_id: self._active[trig.session_id] = trig - self._print_queue("QUEUE AFTER GET (POST-MERGE)") + self._print_queue("QUEUE AFTER GET") return trig # wait for next trigger @@ -647,65 +420,3 @@ def pop_pending_user_message( ) return message, platform - - # ================================================================= - # MERGE HELPERS - # ================================================================= - def _merge_ready_triggers(self, ready: List[Trigger]) -> List[Trigger]: - grouped = defaultdict(list) - for trig in ready: - grouped[trig.session_id].append(trig) - - result = [] - for session_id, triggers in grouped.items(): - logger.debug( - f"[MERGE READY] Merging {len(triggers)} triggers for session={session_id}" - ) - result.append(self._merge_trigger_group(session_id, triggers)) - - return result - - def _merge_trigger_group( - self, session_id: Optional[str], triggers: List[Trigger] - ) -> Trigger: - logger.debug(f"[MERGE GROUP] session={session_id}, count={len(triggers)}") - triggers.sort(key=lambda t: (t.priority, t.fire_at)) - - combined_payload: Dict[str, Any] = {} - combined_desc: OrderedDict[str, None] = OrderedDict() - combined_store_ids: List[int] = [] - priority = triggers[0].priority - fire_at = triggers[0].fire_at - - for trig in triggers: - priority = min(priority, trig.priority) - fire_at = min(fire_at, trig.fire_at) - - desc = (trig.next_action_description or "").strip() - if desc and desc not in combined_desc: - combined_desc[desc] = None - - combined_payload.update(trig.payload) - # Union store rows so acking the merged trigger settles every - # constituent — a missed id would strand its row in CLAIMED. - combined_store_ids.extend(trig.store_ids) - - merged_desc = ( - "\n\n".join(combined_desc.keys()) or triggers[0].next_action_description - ) - - merged = Trigger( - fire_at=fire_at, - priority=priority, - next_action_description=merged_desc, - payload=combined_payload, - session_id=session_id, - id=triggers[0].id, - source=triggers[0].source, - store_ids=combined_store_ids, - ) - - logger.debug( - f"[MERGE RESULT] session={session_id}, fire_at={fire_at}, priority={priority}" - ) - return merged diff --git a/agent_core/core/protocols/trigger.py b/agent_core/core/protocols/trigger.py index 4ae417fd..aaf6f3f6 100644 --- a/agent_core/core/protocols/trigger.py +++ b/agent_core/core/protocols/trigger.py @@ -41,11 +41,3 @@ async def remove_sessions(self, session_ids: List[str]) -> None: async def clear(self) -> None: """Remove all pending triggers from the queue.""" ... - - def create_event_stream_state(self) -> str: - """Return formatted event stream content.""" - ... - - def create_task_state(self) -> str: - """Return formatted task/plan context.""" - ... diff --git a/agent_core/core/trigger.py b/agent_core/core/trigger.py index ea8b07a8..09fabb8a 100644 --- a/agent_core/core/trigger.py +++ b/agent_core/core/trigger.py @@ -8,7 +8,7 @@ from __future__ import annotations from dataclasses import dataclass, field -from typing import Dict, Any, List, Optional +from typing import Dict, Any, Optional @dataclass(order=True) @@ -28,12 +28,11 @@ class Trigger: waiting_for_reply: Whether this trigger is waiting for a user response (used by CraftBot for multi-user chat scenarios). id: Durable-store row id when this trigger is backed by a TriggerStore - row; None for legacy in-memory-only triggers. + row; None for in-memory-only triggers. Claim/ack/nack operate on + this id (the queue holds at most one trigger per session, so one + trigger maps to exactly one row). source: Typed origin of the trigger (TriggerSource value); empty for - legacy producers that haven't been migrated to TriggerService. - store_ids: All durable-store row ids this trigger represents. A merged - trigger carries the ids of every constituent so acking it settles - all of them. + producers that haven't been migrated to TriggerService. """ fire_at: float @@ -44,4 +43,3 @@ class Trigger: waiting_for_reply: bool = field(default=False, compare=False) id: Optional[int] = field(default=None, compare=False) source: str = field(default="", compare=False) - store_ids: List[int] = field(default_factory=list, compare=False) diff --git a/app/agent_base.py b/app/agent_base.py index 40ec74b2..536ff889 100644 --- a/app/agent_base.py +++ b/app/agent_base.py @@ -30,7 +30,7 @@ import uuid import json from dataclasses import dataclass -from typing import Any, Awaitable, Callable, Dict, Iterable, List, Optional +from typing import Awaitable, Callable, Dict, Iterable, Optional from agent_core import ActionLibrary, ActionManager, ActionRouter from agent_core import settings_manager, config_watcher @@ -88,6 +88,7 @@ from app.state.agent_state import STATE from app.trigger import Trigger, TriggerQueue from app.triggers import ( + SessionRouter, TriggerService, TriggerSource, TriggerSpec, @@ -264,13 +265,17 @@ def __init__( # action & task layers self.action_library = ActionLibrary(self.llm, db_interface=self.db_interface) - self.triggers = TriggerQueue( + self.triggers = TriggerQueue() + + self.trigger_store = TriggerStore() + self.trigger_service = TriggerService(self.trigger_store, self.triggers) + + # The single session-routing implementation (Phase 3): consulted by + # the chat handler only, after the message is durably parked. + self.session_router = SessionRouter( llm=self.llm, route_to_session_prompt=ROUTE_TO_SESSION_PROMPT, ) - - self.trigger_store = TriggerStore() - self.trigger_service = TriggerService(self.trigger_store, self.triggers) # global state self.state_manager = StateManager(self.event_stream_manager) @@ -306,9 +311,12 @@ def __init__( # Bind task_manager so state_manager can look up tasks by session_id self.state_manager.bind_task_manager(self.task_manager) - # Bind task_manager and event_stream_manager to trigger queue for rich routing context - self.triggers.set_task_manager(self.task_manager) - self.triggers.set_event_stream_manager(self.event_stream_manager) + # Bind task_manager and event_stream_manager to the router for rich + # routing context (the queue no longer routes — Phase 3). + self.session_router.bind( + task_manager=self.task_manager, + event_stream_manager=self.event_stream_manager, + ) # Set _interface_mode early so context_engine.make_prompt() works during restore # (will be updated again in run() based on selected interface) @@ -1977,168 +1985,8 @@ async def _create_new_trigger(self, new_session_id, action_output, STATE): ) # ----- Chat Handling ----- - - def _format_sessions_for_routing( - self, active_task_ids: List[str], triggers: Optional[List[Trigger]] = None - ) -> str: - """Format active sessions with rich context for routing prompt. - - Uses active task IDs from state_manager (not just triggers in queue) to ensure - all running tasks are visible for routing decisions. - - Args: - active_task_ids: List of task IDs from state_manager.main_state.active_task_ids - triggers: Optional list of triggers (used to check waiting_for_reply status) - - Returns: - Formatted string with session context for routing decisions. - """ - if not active_task_ids: - return "No existing sessions." - - # Build a lookup of triggers by session_id for waiting_for_reply status - trigger_map = {} - if triggers: - for tr in triggers: - if tr.session_id: - trigger_map[tr.session_id] = tr - - sections = [] - for i, task_id in enumerate(active_task_ids, 1): - task = self.task_manager.tasks.get(task_id) if self.task_manager else None - trigger = trigger_map.get(task_id) - - # Check waiting_for_reply from trigger OR from task state - is_waiting = False - if trigger and trigger.waiting_for_reply: - is_waiting = True - if ( - task - and hasattr(task, "waiting_for_user_reply") - and task.waiting_for_user_reply - ): - is_waiting = True - - status = "WAITING FOR REPLY" if is_waiting else "ACTIVE" - platform = ( - trigger.payload.get("platform", "default") if trigger else "default" - ) - - lines = [ - f"--- Session {i} ---", - f"Session ID: {task_id}", - f"Status: {status}", - ] - - if task: - lines.extend( - [ - f'Task Name: "{task.name}"', - f'Original Request: "{task.instruction}"', - f"Mode: {task.mode}", - f"Created: {task.created_at}", - ] - ) - - # Todo progress - if task.todos: - completed = sum(1 for t in task.todos if t.status == "completed") - in_progress_todo = next( - (t for t in task.todos if t.status == "in_progress"), None - ) - lines.append( - f"Progress: {completed}/{len(task.todos)} todos completed" - ) - if in_progress_todo: - lines.append( - f'Currently working on: "{in_progress_todo.content}"' - ) - - # Get recent events from event stream for this task - if self.event_stream_manager and task_id: - stream = self.event_stream_manager.get_stream_by_id(task_id) - if stream and stream.tail_events: - # Get last 10 events for better routing context - # (5 was insufficient - file creation events were missed) - recent_events = stream.tail_events[-10:] - lines.append("Recent Activity:") - for rec in recent_events: - # Only truncate very long event messages (500+ chars) - # Short truncation caused loss of important context like file paths - event_line = rec.compact_line() - if len(event_line) > 500: - event_line = event_line[:497] + "..." - lines.append(f" - {event_line}") - else: - # Fallback to trigger description if no task found - desc = trigger.next_action_description if trigger else "Unknown task" - lines.append(f'Description: "{desc}"') - - lines.append(f"Platform: {platform}") - - # Add Living UI context if the user is on a Living UI page - living_ui_id = trigger.payload.get("living_ui_id") if trigger else None - if living_ui_id: - lines.append(f"Living UI ID: {living_ui_id}") - try: - from app.living_ui import get_living_ui_manager - - mgr = get_living_ui_manager() - if mgr: - proj = mgr.get_project(living_ui_id) - if proj: - lines.append(f"Living UI Name: {proj.name}") - lines.append(f"Living UI Path: {proj.path}") - lines.append( - f" Read {proj.path}/LIVING_UI.md for app context" - ) - lines.append( - " If debugging issues, FIRST read these logs:" - ) - lines.append( - f" - {proj.path}/backend/logs/subprocess_output.log (crashes, stack traces)" - ) - lines.append( - f" - {proj.path}/backend/logs/frontend_console.log (frontend errors, network failures)" - ) - except Exception: - pass - - sections.append("\n".join(lines)) - - return "\n\n".join(sections) - - def _format_recent_conversation(self, limit: int = 10) -> str: - """Format recent conversation messages for routing context. - - Provides the routing LLM with recent conversation history so it can - recognize messages related to completed tasks that are no longer in - the active sessions list. - - Args: - limit: Maximum number of recent messages to include. - - Returns: - Formatted string of recent conversation messages. - """ - if not self.event_stream_manager: - return "No recent conversation history." - - recent_msgs = self.event_stream_manager.get_recent_conversation_messages( - limit=limit - ) - if not recent_msgs: - return "No recent conversation history." - - lines = [] - for evt in recent_msgs: - ts = evt.ts.strftime("%Y-%m-%d %H:%M:%S") if evt.ts else "unknown" - line = f"[{ts}] [{evt.kind}]: {evt.message}" - if len(line) > 300: - line = line[:297] + "..." - lines.append(line) - - return "\n".join(lines) + # Session routing (LLM decision + context formatting) lives in + # app/triggers/router.py (SessionRouter) as of Phase 3. async def _generate_unique_session_id(self) -> str: """Generate a unique 6-character session ID. @@ -2177,68 +2025,6 @@ async def _generate_unique_session_id(self) -> str: ) return uuid.uuid4().hex - async def _route_to_session( - self, - item_type: str, - item_content: str, - existing_sessions: str, - source_platform: str = "default", - current_living_ui_id: Optional[str] = None, - recent_conversation: str = "(no recent conversation)", - ) -> Dict[str, Any]: - """Route incoming item to appropriate session using unified prompt. - - Args: - item_type: Type of incoming item ("message" or "trigger") - item_content: The content of the message or trigger description - existing_sessions: Formatted string of existing sessions - source_platform: The platform the message came from (e.g., "cli", "gui") - current_living_ui_id: The Living UI page the user is currently viewing, - if any. Used by the prompt to default context-dependent messages - ("fix this", "it's broken") to that Living UI's task while still - allowing explicit cross-Living-UI references to override. - recent_conversation: Formatted recent messages across sessions for - cross-session context (helps disambiguate "and Spanish" style - continuations and references to completed tasks). - - Returns: - Dict with routing decision containing: - - action: "route" | "new" - - session_id: The session to route to (or "new") - - reason: Explanation of the routing decision - """ - prompt = ROUTE_TO_SESSION_PROMPT.format( - item_type=item_type, - item_content=item_content, - source_platform=source_platform, - existing_sessions=existing_sessions, - current_living_ui_id=current_living_ui_id or "(not on a Living UI page)", - recent_conversation=recent_conversation, - ) - - logger.debug(f"[UNIFIED ROUTING PROMPT]:\n{prompt}") - response = await self.llm.generate_response_async( - system_prompt="You are a session routing system.", - user_prompt=prompt, - ) - logger.debug(f"[UNIFIED ROUTING RESPONSE]: {response}") - - try: - result = json.loads(response) - # Ensure action field exists for backward compatibility - if "action" not in result: - result["action"] = ( - "route" if result.get("session_id", "new") != "new" else "new" - ) - return result - except json.JSONDecodeError: - logger.error("[ROUTING] Failed to parse routing response JSON") - return { - "action": "new", - "session_id": "new", - "reason": "Failed to parse routing response", - } - # ───────────────────────────────────────────────────────────────────── # Chat routing helpers # ───────────────────────────────────────────────────────────────────── @@ -2378,8 +2164,15 @@ async def _create_new_session_trigger( payload: Dict, platform: str, gui_mode: Optional[bool], + parked_row_id: Optional[int] = None, ) -> None: - """Start a new session and queue a trigger to handle this message.""" + """Start a new session and queue a trigger to handle this message. + + Args: + parked_row_id: The durably-parked copy of this message (written + before routing); settled here once the new session's own + trigger row exists. + """ await self.state_manager.start_session(gui_mode) # Prepend Living UI context to the message if the user is on a Living UI page. @@ -2428,7 +2221,7 @@ async def _create_new_session_trigger( if platform and platform.lower() != "craftbot interface": platform_hint = f" from {platform} (reply on {platform}, NOT send_message)" - await self.trigger_service.emit( + result = await self.trigger_service.emit( TriggerSpec( source=TriggerSource.USER_MESSAGE, description=( @@ -2438,9 +2231,13 @@ async def _create_new_session_trigger( priority=3, session_id=await self._generate_unique_session_id(), payload=trigger_payload, - skip_merge=True, ) ) + # The message now lives in the new session's own trigger row — the + # parked pre-routing copy is settled (superseded by that row). + self.trigger_service.settle_parked( + parked_row_id, delivered_as=result.trigger_id + ) # ───────────────────────────────────────────────────────────────────── # Chat message entry point @@ -2513,6 +2310,35 @@ async def _handle_chat_message(self, payload: Dict): self._post_third_party_notification(payload, platform) return + # ── Durable parking (issue #321): record the message in the + # trigger store BEFORE any routing work. Routing below may take + # an LLM call (seconds) — with the row parked, a crash anywhere + # in this method no longer loses the message; the next boot's + # rehydration re-delivers it as a fresh session. Every delivery + # path below settles the row once the message lands. + parked_id = None + try: + parked_payload = { + "gui_mode": gui_mode, + "platform": platform, + "user_message": chat_content, + } + if living_ui_id: + parked_payload["living_ui_id"] = living_ui_id + parked_id = self.trigger_service.park( + TriggerSpec( + source=TriggerSource.USER_MESSAGE, + description=( + "Please perform action that best suit this user chat " + f"you just received: {chat_content}" + ), + priority=3, + payload=parked_payload, + ) + ) + except Exception as e: + logger.warning(f"[CHAT] Failed to park message durably: {e}") + active_task_ids = self.state_manager.get_main_state().active_task_ids # ── Rule 2: Explicit UI reply with valid target_session_id. @@ -2521,6 +2347,9 @@ async def _handle_chat_message(self, payload: Dict): if await self._fire_session( target_session_id, chat_content, platform, living_ui_id ): + # Message durably attached to the session's trigger row + # by trigger_service.fire() — the parked copy is settled. + self.trigger_service.settle_parked(parked_id) return logger.warning( f"[CHAT] target_session_id {target_session_id} not found — falling through to next rule" @@ -2534,7 +2363,7 @@ async def _handle_chat_message(self, payload: Dict): "[CHAT] UI reply marker without valid target — creating new session" ) await self._create_new_session_trigger( - chat_content, payload, platform, gui_mode + chat_content, payload, platform, gui_mode, parked_row_id=parked_id ) return @@ -2547,11 +2376,13 @@ async def _handle_chat_message(self, payload: Dict): # deserves its own session. if active_task_ids: active_triggers = await self.triggers.list_triggers() - existing_sessions = self._format_sessions_for_routing( + existing_sessions = self.session_router.format_sessions_for_routing( active_task_ids, active_triggers ) - recent_conversation = self._format_recent_conversation(limit=10) - routing_result = await self._route_to_session( + recent_conversation = self.session_router.format_recent_conversation( + limit=10 + ) + routing_result = await self.session_router.route( item_type="message", item_content=chat_content, existing_sessions=existing_sessions, @@ -2568,6 +2399,7 @@ async def _handle_chat_message(self, payload: Dict): if await self._fire_session( matched, chat_content, platform, living_ui_id ): + self.trigger_service.settle_parked(parked_id) return logger.warning( f"[CHAT] LLM routed to {matched} but trigger not found — creating new session" @@ -2575,7 +2407,7 @@ async def _handle_chat_message(self, payload: Dict): # ── Rule 5: Default — create a new session. await self._create_new_session_trigger( - chat_content, payload, platform, gui_mode + chat_content, payload, platform, gui_mode, parked_row_id=parked_id ) except Exception as e: diff --git a/app/triggers/__init__.py b/app/triggers/__init__.py index bdacac1f..0032f662 100644 --- a/app/triggers/__init__.py +++ b/app/triggers/__init__.py @@ -14,6 +14,7 @@ ) from app.triggers.store import TriggerStore, get_trigger_store from app.triggers.service import EmitResult, TriggerService, TriggerSpec +from app.triggers.router import SessionRouter __all__ = [ "TriggerSource", @@ -21,6 +22,7 @@ "TriggerService", "TriggerSpec", "EmitResult", + "SessionRouter", "get_trigger_store", "resume_dedup_key", "scheduled_dedup_key", diff --git a/app/triggers/router.py b/app/triggers/router.py new file mode 100644 index 00000000..fc68f7c7 --- /dev/null +++ b/app/triggers/router.py @@ -0,0 +1,284 @@ +# -*- coding: utf-8 -*- +""" +app.triggers.router + +SessionRouter — decides which session an incoming item belongs to +(issue #321, Phase 3). + +This is the ONE routing implementation. It was extracted from AgentBase +(`_route_to_session` + context formatters); the second, near-duplicate +routing path that lived inside TriggerQueue.put() was deleted outright — +every producer sets a session_id, so it was unreachable in practice. + +Routing is consulted only by the chat-message handler, only when active +tasks exist, and only AFTER the message has been durably parked — so the +LLM call here is off the persistence-critical path: a crash mid-route +loses nothing. +""" + +from __future__ import annotations + +import json +import logging +from typing import Any, Dict, List, Optional + +from agent_core.core.trigger import Trigger + +try: + from app.logger import logger +except Exception: + logger = logging.getLogger(__name__) + logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") + + +class SessionRouter: + """Routes incoming messages/triggers to existing sessions via the LLM.""" + + def __init__( + self, + llm: Any, + route_to_session_prompt: str, + task_manager: Any = None, + event_stream_manager: Any = None, + ) -> None: + self._llm = llm + self._prompt = route_to_session_prompt + self._task_manager = task_manager + self._event_stream_manager = event_stream_manager + + def bind(self, *, task_manager: Any = None, event_stream_manager: Any = None): + """Late-bind managers created after the router.""" + if task_manager is not None: + self._task_manager = task_manager + if event_stream_manager is not None: + self._event_stream_manager = event_stream_manager + + # ─────────────────────── Routing decision ─────────────────────────────── + + async def route( + self, + item_type: str, + item_content: str, + existing_sessions: str, + source_platform: str = "default", + current_living_ui_id: Optional[str] = None, + recent_conversation: str = "(no recent conversation)", + ) -> Dict[str, Any]: + """Route incoming item to appropriate session using unified prompt. + + Args: + item_type: Type of incoming item ("message" or "trigger") + item_content: The content of the message or trigger description + existing_sessions: Formatted string of existing sessions + source_platform: The platform the message came from (e.g., "cli", "gui") + current_living_ui_id: The Living UI page the user is currently viewing, + if any. Used by the prompt to default context-dependent messages + ("fix this", "it's broken") to that Living UI's task while still + allowing explicit cross-Living-UI references to override. + recent_conversation: Formatted recent messages across sessions for + cross-session context (helps disambiguate "and Spanish" style + continuations and references to completed tasks). + + Returns: + Dict with routing decision containing: + - action: "route" | "new" + - session_id: The session to route to (or "new") + - reason: Explanation of the routing decision + """ + prompt = self._prompt.format( + item_type=item_type, + item_content=item_content, + source_platform=source_platform, + existing_sessions=existing_sessions, + current_living_ui_id=current_living_ui_id or "(not on a Living UI page)", + recent_conversation=recent_conversation, + ) + + logger.debug(f"[UNIFIED ROUTING PROMPT]:\n{prompt}") + response = await self._llm.generate_response_async( + system_prompt="You are a session routing system.", + user_prompt=prompt, + ) + logger.debug(f"[UNIFIED ROUTING RESPONSE]: {response}") + + try: + result = json.loads(response) + # Ensure action field exists for backward compatibility + if "action" not in result: + result["action"] = ( + "route" if result.get("session_id", "new") != "new" else "new" + ) + return result + except json.JSONDecodeError: + logger.error("[ROUTING] Failed to parse routing response JSON") + return { + "action": "new", + "session_id": "new", + "reason": "Failed to parse routing response", + } + + # ─────────────────────── Context formatting ───────────────────────────── + + def format_sessions_for_routing( + self, active_task_ids: List[str], triggers: Optional[List[Trigger]] = None + ) -> str: + """Format active sessions with rich context for routing prompt. + + Uses active task IDs from state_manager (not just triggers in queue) to ensure + all running tasks are visible for routing decisions. + + Args: + active_task_ids: List of task IDs from state_manager.main_state.active_task_ids + triggers: Optional list of triggers (used to check waiting_for_reply status) + + Returns: + Formatted string with session context for routing decisions. + """ + if not active_task_ids: + return "No existing sessions." + + # Build a lookup of triggers by session_id for waiting_for_reply status + trigger_map = {} + if triggers: + for tr in triggers: + if tr.session_id: + trigger_map[tr.session_id] = tr + + sections = [] + for i, task_id in enumerate(active_task_ids, 1): + task = ( + self._task_manager.tasks.get(task_id) if self._task_manager else None + ) + trigger = trigger_map.get(task_id) + + # Check waiting_for_reply from trigger OR from task state + is_waiting = False + if trigger and trigger.waiting_for_reply: + is_waiting = True + if ( + task + and hasattr(task, "waiting_for_user_reply") + and task.waiting_for_user_reply + ): + is_waiting = True + + status = "WAITING FOR REPLY" if is_waiting else "ACTIVE" + platform = ( + trigger.payload.get("platform", "default") if trigger else "default" + ) + + lines = [ + f"--- Session {i} ---", + f"Session ID: {task_id}", + f"Status: {status}", + ] + + if task: + lines.extend( + [ + f'Task Name: "{task.name}"', + f'Original Request: "{task.instruction}"', + f"Mode: {task.mode}", + f"Created: {task.created_at}", + ] + ) + + # Todo progress + if task.todos: + completed = sum(1 for t in task.todos if t.status == "completed") + in_progress_todo = next( + (t for t in task.todos if t.status == "in_progress"), None + ) + lines.append( + f"Progress: {completed}/{len(task.todos)} todos completed" + ) + if in_progress_todo: + lines.append( + f'Currently working on: "{in_progress_todo.content}"' + ) + + # Get recent events from event stream for this task + if self._event_stream_manager and task_id: + stream = self._event_stream_manager.get_stream_by_id(task_id) + if stream and stream.tail_events: + # Get last 10 events for better routing context + # (5 was insufficient - file creation events were missed) + recent_events = stream.tail_events[-10:] + lines.append("Recent Activity:") + for rec in recent_events: + # Only truncate very long event messages (500+ chars) + # Short truncation caused loss of important context like file paths + event_line = rec.compact_line() + if len(event_line) > 500: + event_line = event_line[:497] + "..." + lines.append(f" - {event_line}") + else: + # Fallback to trigger description if no task found + desc = trigger.next_action_description if trigger else "Unknown task" + lines.append(f'Description: "{desc}"') + + lines.append(f"Platform: {platform}") + + # Add Living UI context if the user is on a Living UI page + living_ui_id = trigger.payload.get("living_ui_id") if trigger else None + if living_ui_id: + lines.append(f"Living UI ID: {living_ui_id}") + try: + from app.living_ui import get_living_ui_manager + + mgr = get_living_ui_manager() + if mgr: + proj = mgr.get_project(living_ui_id) + if proj: + lines.append(f"Living UI Name: {proj.name}") + lines.append(f"Living UI Path: {proj.path}") + lines.append( + f" Read {proj.path}/LIVING_UI.md for app context" + ) + lines.append( + " If debugging issues, FIRST read these logs:" + ) + lines.append( + f" - {proj.path}/backend/logs/subprocess_output.log (crashes, stack traces)" + ) + lines.append( + f" - {proj.path}/backend/logs/frontend_console.log (frontend errors, network failures)" + ) + except Exception: + pass + + sections.append("\n".join(lines)) + + return "\n\n".join(sections) + + def format_recent_conversation(self, limit: int = 10) -> str: + """Format recent conversation messages for routing context. + + Provides the routing LLM with recent conversation history so it can + recognize messages related to completed tasks that are no longer in + the active sessions list. + + Args: + limit: Maximum number of recent messages to include. + + Returns: + Formatted string of recent conversation messages. + """ + if not self._event_stream_manager: + return "No recent conversation history." + + recent_msgs = self._event_stream_manager.get_recent_conversation_messages( + limit=limit + ) + if not recent_msgs: + return "No recent conversation history." + + lines = [] + for evt in recent_msgs: + ts = evt.ts.strftime("%Y-%m-%d %H:%M:%S") if evt.ts else "unknown" + line = f"[{ts}] [{evt.kind}]: {evt.message}" + if len(line) > 300: + line = line[:297] + "..." + lines.append(line) + + return "\n".join(lines) diff --git a/app/triggers/service.py b/app/triggers/service.py index 66fe2dc6..67d5e285 100644 --- a/app/triggers/service.py +++ b/app/triggers/service.py @@ -13,8 +13,13 @@ triggers the queue discards (same-session replacement, session removal, clear) settle their rows instead of resurrecting on the next boot. -Legacy producers that still call ``queue.put()`` directly keep working: -their triggers carry no ``store_ids``, so claim/ack are no-ops for them. +Producers that still call ``queue.put()`` directly keep working: their +triggers carry no store ``id``, so claim/ack are no-ops for them. + +For user messages there is additionally ``park()``: the message is durably +recorded BEFORE the session-routing LLM call, so a crash mid-routing no +longer loses it — the parked row is re-delivered (as a fresh session) by +the next boot's rehydration. """ from __future__ import annotations @@ -22,6 +27,7 @@ import json import logging import time +import uuid from dataclasses import dataclass, field from typing import Any, Dict, List, Optional, Union @@ -55,6 +61,8 @@ class TriggerSpec: session_id: Optional[str] = None payload: Dict[str, Any] = field(default_factory=dict) dedup_key: Optional[str] = None + # Deprecated, ignored: in-queue routing was removed (Phase 3); the queue + # treats every put identically. Kept so existing call sites don't break. skip_merge: bool = False waiting_for_reply: bool = False @@ -128,29 +136,70 @@ async def emit(self, spec: TriggerSpec) -> EmitResult: waiting_for_reply=spec.waiting_for_reply, id=row_id, source=source, - store_ids=[row_id] if row_id is not None else [], ) - await self._queue.put(trig, skip_merge=spec.skip_merge) + await self._queue.put(trig) return EmitResult(row_id, False) + def park(self, spec: TriggerSpec) -> Optional[int]: + """Durably record a trigger WITHOUT enqueueing it. + + Used for incoming user messages before session routing: the routing + LLM call takes seconds and a crash during it would otherwise lose + the message entirely. The caller settles the parked row once the + message reaches its destination (``settle_parked``); if it never + does, rehydration re-delivers the row as a fresh session. + """ + fire_at = spec.fire_at if spec.fire_at is not None else time.time() + source = ( + spec.source.value + if isinstance(spec.source, TriggerSource) + else str(spec.source) + ) + row_id, _ = self._store.insert( + source=source, + description=spec.description, + fire_at=fire_at, + priority=spec.priority, + session_id=spec.session_id, + payload=spec.payload, + dedup_key=spec.dedup_key, + waiting_for_reply=spec.waiting_for_reply, + ) + return row_id + + def settle_parked( + self, row_id: Optional[int], delivered_as: Optional[int] = None + ) -> None: + """Mark a parked row as delivered to its destination. + + Args: + row_id: The parked row (None is a no-op for convenience). + delivered_as: The store row that now carries the work — the new + session's trigger row, or None when the message was attached + to an existing session's trigger via fire(). + """ + if row_id is None: + return + self._store.supersede([row_id], by_id=delivered_as) + # ─────────────────────── Consumer API ─────────────────────────────────── async def next(self) -> Trigger: - """Wait for the next due trigger and claim its store rows.""" + """Wait for the next due trigger and claim its store row.""" trig = await self._queue.get() - if trig.store_ids: - self._store.claim(trig.store_ids) + if trig.id is not None: + self._store.claim([trig.id]) return trig async def ack(self, trig: Trigger) -> None: """The react cycle for this trigger completed.""" - if trig.store_ids: - self._store.ack(trig.store_ids) + if trig.id is not None: + self._store.ack([trig.id]) async def nack(self, trig: Trigger, error: str) -> None: """The react cycle raised before completing.""" - if trig.store_ids: - self._store.fail(trig.store_ids, error=error) + if trig.id is not None: + self._store.fail([trig.id], error=error) # ─────────────────────── fire() pass-through ──────────────────────────── @@ -237,18 +286,29 @@ async def rehydrate(self) -> int: payload["overdue_seconds"] = overdue description = f"{description}\n\n{note}" + # A row with no session is a parked user message whose routing + # never completed (crash mid-route). Re-deliver it as a fresh + # session — the agent handles it like a newly arrived message. + session_id = row["session_id"] + if not session_id: + session_id = uuid.uuid4().hex[:6] + self._store.update_session(row["id"], session_id) + logger.info( + f"[TriggerService] Recovered unrouted trigger {row['id']} " + f"as new session {session_id}" + ) + trig = Trigger( fire_at=row["fire_at"], priority=row["priority"], next_action_description=description, payload=payload, - session_id=row["session_id"], + session_id=session_id, waiting_for_reply=bool(row["waiting_for_reply"]), id=row["id"], source=row["source"] or "", - store_ids=[row["id"]], ) - await self._queue.put(trig, skip_merge=True) + await self._queue.put(trig) requeued += 1 if stale_ids: @@ -277,7 +337,7 @@ def on_evicted( self, evicted: List[Trigger], replacement: Optional[Trigger] ) -> None: """Queue discarded triggers unconsumed — settle their rows.""" - ids = [row_id for t in evicted for row_id in (t.store_ids or [])] + ids = [t.id for t in evicted if t.id is not None] if not ids: return if replacement is not None: diff --git a/app/triggers/store.py b/app/triggers/store.py index 8ac0506a..ec786a39 100644 --- a/app/triggers/store.py +++ b/app/triggers/store.py @@ -403,6 +403,15 @@ def update_for_fire( conn.commit() return updated + def update_session(self, row_id: int, session_id: str) -> None: + """Assign a session to a row (recovery of unrouted parked messages).""" + with self._connect() as conn: + conn.execute( + "UPDATE triggers SET session_id = ?, updated_at = ? WHERE id = ?", + (session_id, _now_iso(), row_id), + ) + conn.commit() + # ─────────────────────── Utilities ────────────────────────────────────── def get(self, row_id: int) -> Optional[Dict[str, Any]]: diff --git a/tests/test_trigger_router_and_parking.py b/tests/test_trigger_router_and_parking.py new file mode 100644 index 00000000..c25e6745 --- /dev/null +++ b/tests/test_trigger_router_and_parking.py @@ -0,0 +1,163 @@ +# -*- coding: utf-8 -*- +"""Phase 3 tests: SessionRouter decisions and durable message parking +(crash-during-routing recovery).""" + +import asyncio +import json +import time + +from app.triggers import SessionRouter, TriggerSource, TriggerSpec +from app.triggers.service import TriggerService +from app.triggers.store import TriggerStore + +from agent_core.core.impl.trigger.queue import TriggerQueue + + +def run(coro): + return asyncio.run(coro) + + +class FakeLLM: + def __init__(self, response: str): + self.response = response + self.calls = 0 + + async def generate_response_async(self, system_prompt: str, user_prompt: str): + self.calls += 1 + self.last_prompt = user_prompt + return self.response + + +ROUTING_PROMPT = ( + "{item_type}|{item_content}|{source_platform}|{existing_sessions}|" + "{current_living_ui_id}|{recent_conversation}" +) + + +class TestSessionRouter: + def test_route_to_existing_session(self): + llm = FakeLLM(json.dumps({"action": "route", "session_id": "abc123"})) + router = SessionRouter(llm, ROUTING_PROMPT) + result = run(router.route("message", "continue that task", "sessions")) + assert result["session_id"] == "abc123" + assert llm.calls == 1 + + def test_new_session_decision(self): + llm = FakeLLM(json.dumps({"action": "new", "session_id": "new"})) + router = SessionRouter(llm, ROUTING_PROMPT) + result = run(router.route("message", "do something else", "sessions")) + assert result["action"] == "new" + + def test_action_field_backfilled(self): + # Old-style response without "action" — inferred from session_id. + llm = FakeLLM(json.dumps({"session_id": "abc123"})) + router = SessionRouter(llm, ROUTING_PROMPT) + result = run(router.route("message", "x", "sessions")) + assert result["action"] == "route" + + def test_garbage_response_defaults_to_new(self): + llm = FakeLLM("not json at all {{{") + router = SessionRouter(llm, ROUTING_PROMPT) + result = run(router.route("message", "x", "sessions")) + assert result == { + "action": "new", + "session_id": "new", + "reason": "Failed to parse routing response", + } + + def test_format_sessions_empty(self): + router = SessionRouter(FakeLLM(""), ROUTING_PROMPT) + assert router.format_sessions_for_routing([]) == "No existing sessions." + + def test_queue_never_calls_llm(self, tmp_path): + # Phase 3 invariant: the queue has no routing — an LLM passed to its + # deprecated ctor param is never invoked on put/get. + llm = FakeLLM(json.dumps({"action": "new"})) + queue = TriggerQueue(llm=llm) + store = TriggerStore(db_path=str(tmp_path / "s.db")) + service = TriggerService(store, queue) + + async def scenario(): + # No session_id at all — the pre-#321 queue would have routed this. + await service.emit( + TriggerSpec( + source=TriggerSource.USER_MESSAGE, + description="hello", + session_id="s1", + ) + ) + await asyncio.wait_for(service.next(), timeout=2) + + run(scenario()) + assert llm.calls == 0 + + +class TestDurableParking: + def make_stack(self, tmp_path): + store = TriggerStore(db_path=str(tmp_path / "sessions.db")) + queue = TriggerQueue() + service = TriggerService(store, queue) + return store, queue, service + + def parked_spec(self): + return TriggerSpec( + source=TriggerSource.USER_MESSAGE, + description="Please perform action that best suit this user chat " + "you just received: send the report", + priority=3, + payload={"user_message": "send the report", "platform": "Telegram"}, + ) + + def test_park_records_without_enqueue(self, tmp_path): + store, queue, service = self.make_stack(tmp_path) + + async def scenario(): + row_id = service.park(self.parked_spec()) + assert store.get(row_id)["status"] == "PENDING" + assert store.get(row_id)["session_id"] is None + assert await queue.size() == 0 # parked, not queued + return row_id + + run(scenario()) + + def test_settled_park_does_not_rehydrate(self, tmp_path): + async def scenario(): + store, queue, service = self.make_stack(tmp_path) + row_id = service.park(self.parked_spec()) + # message delivered into a new session's row + result = await service.emit( + TriggerSpec( + source=TriggerSource.USER_MESSAGE, + description="delivered", + session_id="abc123", + ) + ) + service.settle_parked(row_id, delivered_as=result.trigger_id) + row = store.get(row_id) + assert row["status"] == "DONE" + assert row["superseded_by"] == result.trigger_id + + run(scenario()) + + def test_crash_during_routing_recovers_message(self, tmp_path): + # The Phase 3 headline: park → crash before routing finishes → + # next boot re-delivers the message as a fresh session. + async def scenario(): + store, queue, service = self.make_stack(tmp_path) + row_id = service.park(self.parked_spec()) + # crash: routing LLM call never completed, nothing was settled + + store2, queue2, service2 = self.make_stack(tmp_path) # restart + requeued = await service2.rehydrate() + assert requeued == 1 + + trig = await asyncio.wait_for(service2.next(), timeout=2) + assert trig.id == row_id + assert trig.session_id # assigned a fresh session on recovery + assert "send the report" in trig.next_action_description + assert store2.get(row_id)["session_id"] == trig.session_id + + await service2.ack(trig) + assert store2.get(row_id)["status"] == "DONE" + + run(scenario()) diff --git a/tests/test_trigger_service.py b/tests/test_trigger_service.py index c252a33d..b11acc88 100644 --- a/tests/test_trigger_service.py +++ b/tests/test_trigger_service.py @@ -49,7 +49,7 @@ async def scenario(): assert store.get(result.trigger_id)["status"] == "PENDING" trig = await asyncio.wait_for(service.next(), timeout=2) - assert trig.store_ids == [result.trigger_id] + assert trig.id == result.trigger_id assert store.get(result.trigger_id)["status"] == "CLAIMED" await service.ack(trig) @@ -88,8 +88,8 @@ async def scenario(): run(scenario()) def test_legacy_put_passthrough(self, tmp_path): - # Producers not yet migrated still work; their triggers carry no - # store rows and ack is a no-op. + # Direct queue.put() still works; such triggers carry no store row + # and ack is a no-op. store, queue, service = make_stack(tmp_path) async def scenario(): @@ -99,11 +99,10 @@ async def scenario(): priority=3, next_action_description="legacy", session_id="legacy-session", - ), - skip_merge=True, + ) ) trig = await asyncio.wait_for(service.next(), timeout=2) - assert trig.store_ids == [] + assert trig.id is None await service.ack(trig) # must not raise assert store.count_by_status() == {} @@ -123,7 +122,7 @@ async def scenario(): assert await queue2.size() == 1 trig = await asyncio.wait_for(service2.next(), timeout=2) - assert trig.store_ids == [result.trigger_id] + assert trig.id == result.trigger_id await service2.ack(trig) # a second restart must not re-deliver settled work @@ -207,7 +206,7 @@ async def scenario(): requeued = await service2.rehydrate() assert requeued == 1 trig = await asyncio.wait_for(service2.next(), timeout=2) - assert trig.store_ids == [r2.trigger_id] + assert trig.id == r2.trigger_id run(scenario()) @@ -233,11 +232,11 @@ async def scenario(): run(scenario()) -class TestMergedTriggerAck: - def test_merge_unions_store_ids_and_ack_settles_all(self, tmp_path): - # put() replaces same-session triggers, so two same-session entries - # can only coexist via internal paths — inject directly to verify the - # merge/ack contract (a missed id would strand a row in CLAIMED). +class TestSequentialConsumption: + def test_directly_pushed_same_session_triggers_each_settle(self, tmp_path): + # The pre-#321 merge machinery is gone: if two same-session triggers + # ever coexist (only possible via direct heap pushes — put() replaces), + # they are consumed one at a time and each settles its own row. store, queue, service = make_stack(tmp_path) async def scenario(): @@ -257,13 +256,14 @@ async def scenario(): session_id="s1", id=row_id, source="scheduled", - store_ids=[row_id], ), ) - trig = await asyncio.wait_for(service.next(), timeout=2) - assert sorted(trig.store_ids) == sorted([id1, id2]) - await service.ack(trig) + first = await asyncio.wait_for(service.next(), timeout=2) + await service.ack(first) + second = await asyncio.wait_for(service.next(), timeout=2) + await service.ack(second) + assert {first.id, second.id} == {id1, id2} assert store.get(id1)["status"] == "DONE" assert store.get(id2)["status"] == "DONE" From 464d17045166ddd87ca361b7a163b59538ed8f7b Mon Sep 17 00:00:00 2001 From: ahmad-ajmal Date: Fri, 12 Jun 2026 03:11:00 +0100 Subject: [PATCH 4/5] Irreversible actions --- agent_core/core/action/action.py | 8 + agent_core/core/action_framework/registry.py | 12 + agent_core/core/impl/action/idempotency.py | 66 ++++ agent_core/core/impl/action/manager.py | 61 ++++ agent_core/core/impl/trigger/queue.py | 4 +- app/agent_base.py | 14 +- .../integrations/discord/discord_actions.py | 4 + .../google_workspace/gmail_actions.py | 5 + .../google_youtube_actions.py | 1 + .../integrations/hubspot/hubspot_actions.py | 2 + .../action/integrations/lark/lark_actions.py | 10 + .../action/integrations/line/line_actions.py | 12 + .../integrations/linkedin/linkedin_actions.py | 2 + .../integrations/outlook/outlook_actions.py | 5 + .../integrations/slack/slack_actions.py | 2 + .../integrations/telegram/telegram_actions.py | 20 ++ .../integrations/twitter/twitter_actions.py | 6 + .../integrations/whatsapp/whatsapp_actions.py | 5 + app/data/action/schedule_task.py | 2 +- app/data/action/send_message.py | 1 + .../action/send_message_with_attachment.py | 1 + app/living_ui/manager.py | 2 +- app/scheduler/manager.py | 2 +- app/triggers/__init__.py | 2 +- app/triggers/activity_log.py | 288 ++++++++++++++++++ app/triggers/router.py | 3 +- app/triggers/service.py | 3 +- app/triggers/sources.py | 2 +- app/triggers/store.py | 2 +- app/ui_layer/controller/ui_controller.py | 2 +- app/usage/session_storage.py | 2 +- tests/test_activity_log.py | 137 +++++++++ tests/test_trigger_service.py | 2 +- tests/test_trigger_store.py | 2 +- 34 files changed, 675 insertions(+), 17 deletions(-) create mode 100644 agent_core/core/impl/action/idempotency.py create mode 100644 app/triggers/activity_log.py create mode 100644 tests/test_activity_log.py diff --git a/agent_core/core/action/action.py b/agent_core/core/action/action.py index 70154357..c797b521 100644 --- a/agent_core/core/action/action.py +++ b/agent_core/core/action/action.py @@ -61,6 +61,7 @@ def __init__( requirements: Optional[List[str]] = None, timeout: Optional[int] = None, parallelizable: bool = True, + irreversible: bool = False, ): """ Initialize a new Action definition. @@ -101,6 +102,10 @@ def __init__( parallelizable: Whether this action can be executed in parallel with others. Defaults to True. Set to False for write operations, GUI actions, state changes, send_message, etc. + irreversible: Whether the action's side effect cannot be undone once + it reaches the outside world (send email/message, post publicly). + Irreversible actions are guarded by the activity ledger so a + completed run is never silently re-executed after a crash. """ self.name = name self.description = description @@ -125,6 +130,7 @@ def __init__( self.requirements = requirements or [] self.timeout = timeout if timeout is not None else self.DEFAULT_TIMEOUT self.parallelizable = parallelizable + self.irreversible = irreversible @property def display_name(self) -> str: @@ -168,6 +174,7 @@ def to_dict(self) -> Dict[str, Any]: "requirements": self.requirements, "timeout": self.timeout, "parallelizable": self.parallelizable, + "irreversible": self.irreversible, } @classmethod @@ -211,6 +218,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "Action": requirements=data.get("requirements", []), timeout=data.get("timeout"), parallelizable=data.get("parallelizable", True), + irreversible=data.get("irreversible", False), ) return data_to_return diff --git a/agent_core/core/action_framework/registry.py b/agent_core/core/action_framework/registry.py index b417d65e..a8e95aa6 100644 --- a/agent_core/core/action_framework/registry.py +++ b/agent_core/core/action_framework/registry.py @@ -74,6 +74,11 @@ class ActionMetadata: # Whether this action can be executed in parallel with other actions. # Set to False for: write operations, GUI actions, state changes, send_message, etc. parallelizable: bool = True + # Whether this action's side effect cannot be undone once it reaches the + # outside world (send email/message, post publicly). Irreversible actions + # are guarded by the activity ledger: intent is recorded + # before execution and a completed run is never silently re-executed. + irreversible: bool = False @property def display_name(self) -> str: @@ -264,6 +269,7 @@ def _get_action_as_json(self, platform_impls) -> Dict[str, Any]: "code": main_code_str, "platform_overrides": {}, "parallelizable": meta.parallelizable, + "irreversible": meta.irreversible, } # 3. Handle Platform Overrides @@ -405,6 +411,7 @@ def action( test_payload: Optional[Dict[str, Any]] = None, action_sets: Optional[List[str]] = None, parallelizable: bool = True, + irreversible: bool = False, ): """ Decorator used by developers to register functions as actions. @@ -425,6 +432,10 @@ def action( (e.g., ["file_operations", "core"]) parallelizable: Whether this action can run in parallel with others. Set to False for write operations, GUI actions, state changes, etc. + irreversible: Whether the action's side effect cannot be undone once + it reaches the outside world (send email/message, post + publicly). Guarded by the activity ledger: a completed + run is never silently re-executed after a crash. """ # Normalize platforms input to a list of lowercase strings if platforms is None: @@ -449,6 +460,7 @@ def decorator_factory(func: Callable): test_payload=test_payload, action_sets=action_sets or [], parallelizable=parallelizable, + irreversible=irreversible, ) # 2. Create the full registration object diff --git a/agent_core/core/impl/action/idempotency.py b/agent_core/core/impl/action/idempotency.py new file mode 100644 index 00000000..0d1739fa --- /dev/null +++ b/agent_core/core/impl/action/idempotency.py @@ -0,0 +1,66 @@ +# -*- coding: utf-8 -*- +""" +core.impl.action.idempotency + +Idempotency guard protocol for irreversible actions. + +Actions flagged ``irreversible=True`` (send email/message, post publicly) +have a real crash window today: the side effect happens first, the +``action_end`` record is persisted after. A crash in between leaves the +effect done with no durable record — and on resume the LLM, re-reading the +event stream, sees no completion and may re-execute it. + +The guard turns "did this already run?" into a hard database check: + +- ``begin()`` is called BEFORE execution. It durably records intent and + decides: proceed (fresh work), short-circuit with the stored output + (this exact run already completed), or refuse with a warning (a previous + attempt was interrupted and the side effect MAY have happened — verify + or confirm with the user instead of blindly re-sending). +- ``complete()`` is called AFTER execution with the outcome. + +The implementation (the activity ledger on sessions.db) lives in the app +layer; ActionManager only knows this protocol. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Dict, Optional, Protocol, runtime_checkable + + +@dataclass +class GuardDecision: + """begin()'s verdict for one irreversible action execution.""" + + proceed: bool + idem_key: Optional[str] = None + # Set when this exact run already completed: returned as the action's + # output instead of re-executing the side effect. + stored_output: Optional[Dict[str, Any]] = None + # Set when a previous attempt was interrupted mid-flight: a warning the + # LLM sees as the action result instead of a blind re-execution. + note: Optional[str] = None + + +@runtime_checkable +class IdempotencyGuard(Protocol): + """Pre/post execution hooks for irreversible actions.""" + + def begin( + self, + action_name: str, + input_data: Dict[str, Any], + session_id: Optional[str], + ) -> GuardDecision: + """Record intent durably and decide whether execution may proceed.""" + ... + + def complete( + self, + idem_key: str, + status: str, + outputs: Optional[Dict[str, Any]], + ) -> None: + """Record the outcome of an execution begin() allowed.""" + ... diff --git a/agent_core/core/impl/action/manager.py b/agent_core/core/impl/action/manager.py index 7f11eaf2..ec0c5db4 100644 --- a/agent_core/core/impl/action/manager.py +++ b/agent_core/core/impl/action/manager.py @@ -28,6 +28,7 @@ from agent_core.core.protocols.context import ContextEngineProtocol from agent_core.core.protocols.state import StateManagerProtocol from agent_core.core.impl.action.executor import ActionExecutor +from agent_core.core.impl.action.idempotency import IdempotencyGuard from agent_core.utils.logger import logger # ============================================================================ @@ -120,6 +121,7 @@ def __init__( on_action_start: Optional[OnActionStartHook] = None, on_action_end: Optional[OnActionEndHook] = None, get_parent_id: Optional[GetParentIdHook] = None, + idempotency_guard: Optional["IdempotencyGuard"] = None, ): """ Build an ActionManager that can execute and track actions. @@ -134,6 +136,10 @@ def __init__( on_action_start: Optional hook called when action starts. on_action_end: Optional hook called when action ends. get_parent_id: Optional hook to resolve parent_id from task context. + idempotency_guard: Optional guard consulted before/after actions + flagged ``irreversible=True``: records intent + before the side effect and prevents a completed run from + being silently re-executed after a crash. """ self.action_library = action_library self.llm_interface = llm_interface @@ -150,6 +156,7 @@ def __init__( self._on_action_start = on_action_start self._on_action_end = on_action_end self._get_parent_id = get_parent_id + self._idempotency_guard = idempotency_guard def _generate_unique_session_id(self) -> str: """Generate a unique 6-character session ID. @@ -234,6 +241,50 @@ async def execute_action( input_data["_session_id"] = session_id logger.debug(f"[INPUT DATA] {input_data}") + + # ── Idempotency guard for irreversible actions ── + # BEFORE the side effect: record intent durably, and refuse to + # re-execute work the ledger shows as already completed (or as + # interrupted mid-flight, where the effect may have happened). + idem_key = None + if getattr(action, "irreversible", False) and self._idempotency_guard: + try: + decision = self._idempotency_guard.begin( + action.name, input_data, session_id + ) + except Exception as exc: + logger.warning(f"Idempotency guard begin() failed: {exc}") + decision = None + if decision is not None: + idem_key = decision.idem_key + if not decision.proceed: + if decision.stored_output is not None: + skip_outputs = decision.stored_output + skip_message = ( + f"Action {action.name} skipped — an identical run " + f"already completed; returning its stored output." + ) + else: + skip_outputs = { + "status": "error", + "error": decision.note, + "error_code": "irreversible_uncertain", + } + skip_message = ( + f"Action {action.name} blocked — a previous " + f"attempt was interrupted and its side effect may " + f"already have happened. See the action output." + ) + self._log_event_stream( + is_gui_task=is_gui_task, + event_type="action_end", + event=skip_message, + display_message=f"{action.display_name} → skipped (idempotent)", + action_name=action.name, + session_id=session_id, + ) + return skip_outputs + run_id = str(uuid.uuid4()) started_at = datetime.utcnow().isoformat() @@ -365,6 +416,16 @@ async def execute_action( ended_at = datetime.utcnow().isoformat() + # ── Idempotency guard: record the outcome durably ── + # AFTER the side effect, BEFORE anything that could fail below — a + # crash between the side effect and this line is the §4.2 window the + # ledger exists to shrink. + if idem_key and self._idempotency_guard: + try: + self._idempotency_guard.complete(idem_key, status, outputs) + except Exception as exc: + logger.warning(f"Idempotency guard complete() failed: {exc}") + # Re-resolve parent_id after execution if hook provided if not parent_id and self._get_parent_id: parent_id = self._get_parent_id() diff --git a/agent_core/core/impl/trigger/queue.py b/agent_core/core/impl/trigger/queue.py index 3adb1f75..6fd0d0e5 100644 --- a/agent_core/core/impl/trigger/queue.py +++ b/agent_core/core/impl/trigger/queue.py @@ -5,7 +5,7 @@ TriggerQueue implementation - in-memory ordering primitive for triggers. The queue holds due-time-ordered triggers and hands them to the single -consumer loop. It is deliberately dumb (issue #321): +consumer loop. It is deliberately dumb: - Durability lives in the app-layer TriggerStore; the queue reports any trigger it discards unconsumed through a TriggerLifecycleListener so the @@ -63,7 +63,7 @@ def __init__( Args: llm: Deprecated, ignored. In-queue LLM routing was removed - (issue #321 Phase 3); routing happens at the producer layer. + ; routing happens at the producer layer. route_to_session_prompt: Deprecated, ignored. task_manager: Deprecated, ignored. event_stream_manager: Deprecated, ignored. diff --git a/app/agent_base.py b/app/agent_base.py index 536ff889..97bcb9a4 100644 --- a/app/agent_base.py +++ b/app/agent_base.py @@ -282,6 +282,13 @@ def __init__( self.context_engine = ContextEngine(state_manager=self.state_manager) self.context_engine.set_role_info_hook(self._generate_role_info_prompt) + # Idempotency guard: actions flagged + # irreversible=True record intent to the activity ledger before the + # side effect and their completed runs are never silently re-executed + # after a crash — "did this already run?" is a database check. + from app.triggers.activity_log import ActivityLogGuard, get_activity_log + + self.activity_log = get_activity_log() self.action_manager = ActionManager( self.action_library, self.llm, @@ -289,6 +296,7 @@ def __init__( self.event_stream_manager, self.context_engine, self.state_manager, + idempotency_guard=ActivityLogGuard(self.activity_log), ) self.action_router = ActionRouter( self.action_library, self.llm, self.context_engine @@ -2310,7 +2318,7 @@ async def _handle_chat_message(self, payload: Dict): self._post_third_party_notification(payload, platform) return - # ── Durable parking (issue #321): record the message in the + # ── Durable parking: record the message in the # trigger store BEFORE any routing work. Routing below may take # an LLM call (seconds) — with the row parked, a crash anywhere # in this method no longer loses the message; the next boot's @@ -2594,6 +2602,10 @@ async def reset_agent_state(self) -> str: self.trigger_store.clear_all() except Exception as e: logger.warning(f"[RESET] Failed to clear trigger store: {e}") + try: + self.activity_log.clear_all() + except Exception as e: + logger.warning(f"[RESET] Failed to clear activity log: {e}") self.task_manager.reset() self.state_manager.reset() self.event_stream_manager.clear_all() diff --git a/app/data/action/integrations/discord/discord_actions.py b/app/data/action/integrations/discord/discord_actions.py index 5edaa14e..e6f36f66 100644 --- a/app/data/action/integrations/discord/discord_actions.py +++ b/app/data/action/integrations/discord/discord_actions.py @@ -8,6 +8,7 @@ @action( name="send_discord_message", + irreversible=True, description="Send a message to a Discord channel.", action_sets=["discord_messages", "discord"], input_schema={ @@ -1890,6 +1891,7 @@ def get_discord_bot_user(input_data: dict) -> dict: @action( name="send_discord_dm", + irreversible=True, description="Send a direct message to a Discord user.", action_sets=["discord_messages", "discord"], input_schema={ @@ -1938,6 +1940,7 @@ def get_discord_user_account(input_data: dict) -> dict: @action( name="send_discord_user_message", + irreversible=True, description="Send user message (self-bot).", action_sets=["discord_user"], input_schema={ @@ -1990,6 +1993,7 @@ def get_discord_user_dm_channels(input_data: dict) -> dict: @action( name="send_discord_user_dm", + irreversible=True, description="Send user DM.", action_sets=["discord_user"], input_schema={ diff --git a/app/data/action/integrations/google_workspace/gmail_actions.py b/app/data/action/integrations/google_workspace/gmail_actions.py index e7c82b18..27391b0d 100644 --- a/app/data/action/integrations/google_workspace/gmail_actions.py +++ b/app/data/action/integrations/google_workspace/gmail_actions.py @@ -8,6 +8,7 @@ @action( name="send_gmail", + irreversible=True, description="Send an email via Gmail.", action_sets=["gmail_mail", "gmail"], input_schema={ @@ -182,6 +183,7 @@ def search_gmail(input_data: dict) -> dict: @action( name="reply_gmail", + irreversible=True, description="Reply to a Gmail message. Preserves thread + In-Reply-To/References headers. Set reply_all=true to also CC the original To/Cc.", action_sets=["gmail_mail", "gmail"], input_schema={ @@ -222,6 +224,7 @@ def reply_gmail(input_data: dict) -> dict: @action( name="forward_gmail", + irreversible=True, description="Forward a Gmail message to another address.", action_sets=["gmail_mail", "gmail"], input_schema={ @@ -728,6 +731,7 @@ def update_gmail_draft(input_data: dict) -> dict: @action( name="send_gmail_draft", + irreversible=True, description="Send a previously-created Gmail draft.", action_sets=["gmail_drafts", "gmail"], input_schema={ @@ -1013,6 +1017,7 @@ def get_gmail_profile(input_data: dict) -> dict: @action( name="send_google_workspace_email", + irreversible=True, description="Send email via Google Workspace.", action_sets=["gmail_mail"], input_schema={ diff --git a/app/data/action/integrations/google_workspace/google_youtube_actions.py b/app/data/action/integrations/google_workspace/google_youtube_actions.py index e47b1ccf..ec9fee2a 100644 --- a/app/data/action/integrations/google_workspace/google_youtube_actions.py +++ b/app/data/action/integrations/google_workspace/google_youtube_actions.py @@ -247,6 +247,7 @@ def rate_youtube_video(input_data: dict) -> dict: @action( name="post_youtube_comment", + irreversible=True, description="Post a top-level comment on a YouTube video.", action_sets=["google_youtube"], input_schema={ diff --git a/app/data/action/integrations/hubspot/hubspot_actions.py b/app/data/action/integrations/hubspot/hubspot_actions.py index 58d8ee36..823fe33a 100644 --- a/app/data/action/integrations/hubspot/hubspot_actions.py +++ b/app/data/action/integrations/hubspot/hubspot_actions.py @@ -2570,6 +2570,7 @@ async def get_hubspot_marketing_email(input_data: dict) -> dict: @action( name="send_hubspot_single_send", + irreversible=True, description="Send a one-off transactional email based on a pre-built marketing email template.", action_sets=["hubspot_marketing_email", "hubspot"], input_schema={ @@ -2816,6 +2817,7 @@ async def list_hubspot_conversation_messages(input_data: dict) -> dict: @action( name="send_hubspot_conversation_message", + irreversible=True, description="Send a message into a conversation thread. Requires the channel + channel-account IDs from the thread metadata.", action_sets=["hubspot_conversations"], input_schema={ diff --git a/app/data/action/integrations/lark/lark_actions.py b/app/data/action/integrations/lark/lark_actions.py index 68eb577c..15ee198b 100644 --- a/app/data/action/integrations/lark/lark_actions.py +++ b/app/data/action/integrations/lark/lark_actions.py @@ -8,6 +8,7 @@ @action( name="send_lark_message", + irreversible=True, description="Send a plain text message in Lark. receive_id_type: open_id | user_id | email | chat_id | union_id.", action_sets=["lark_messages", "lark"], input_schema={ @@ -40,6 +41,7 @@ async def send_lark_message(input_data: dict) -> dict: @action( name="reply_lark_message", + irreversible=True, description="Reply to a Lark message by message_id.", action_sets=["lark_messages", "lark"], input_schema={ @@ -66,6 +68,7 @@ async def reply_lark_message(input_data: dict) -> dict: @action( name="send_lark_rich_message", + irreversible=True, description="Send a generic Lark message. msg_type: text | post | image | file | audio | media | sticker | interactive | share_chat | share_user. content is the per-type dict (this action JSON-encodes it for you).", action_sets=["lark_messages", "lark"], input_schema={ @@ -110,6 +113,7 @@ async def send_lark_rich_message(input_data: dict) -> dict: @action( name="send_lark_image", + irreversible=True, description="Send an image (use upload_lark_image first to get image_key).", action_sets=["lark_messages", "lark"], input_schema={ @@ -142,6 +146,7 @@ async def send_lark_image(input_data: dict) -> dict: @action( name="send_lark_file", + irreversible=True, description="Send a file (use upload_lark_im_file first to get file_key).", action_sets=["lark_messages", "lark"], input_schema={ @@ -174,6 +179,7 @@ async def send_lark_file(input_data: dict) -> dict: @action( name="send_lark_card", + irreversible=True, description="Send an interactive card (Lark's Block Kit equivalent). card is the card schema dict.", action_sets=["lark_messages", "lark"], input_schema={ @@ -202,6 +208,7 @@ async def send_lark_card(input_data: dict) -> dict: @action( name="send_lark_post", + irreversible=True, description="Send a rich-text 'post' message (multi-line, styled). post is Lark's post schema: {zh_cn: {title, content: [[{tag,text}]]}}.", action_sets=["lark_messages"], input_schema={ @@ -230,6 +237,7 @@ async def send_lark_post(input_data: dict) -> dict: @action( name="reply_lark_rich_message", + irreversible=True, description="Reply with non-text content (image / file / card / etc.). reply_in_thread starts a thread off the parent.", action_sets=["lark_messages"], input_schema={ @@ -337,6 +345,7 @@ async def update_lark_message(input_data: dict) -> dict: @action( name="forward_lark_message", + irreversible=True, description="Forward a message to another recipient.", action_sets=["lark_messages", "lark"], input_schema={ @@ -603,6 +612,7 @@ async def list_lark_pinned_messages(input_data: dict) -> dict: @action( name="send_lark_urgent", + irreversible=True, description="Escalate a message to selected users. urgent_type: app (in-app push) | sms | phone (call). Use sparingly — sms/phone require special permission.", action_sets=["lark_messages"], input_schema={ diff --git a/app/data/action/integrations/line/line_actions.py b/app/data/action/integrations/line/line_actions.py index 433da79e..9621717f 100644 --- a/app/data/action/integrations/line/line_actions.py +++ b/app/data/action/integrations/line/line_actions.py @@ -9,6 +9,7 @@ @action( name="send_line_message", + irreversible=True, description="Push a text message to a LINE user/group/room.", action_sets=["line_messages", "line"], input_schema={ @@ -32,6 +33,7 @@ def send_line_message(input_data: dict) -> dict: @action( name="reply_line_message", + irreversible=True, description="Reply to a LINE message using the reply token (1-minute window).", action_sets=["line_messages", "line"], input_schema={ @@ -126,6 +128,7 @@ def push_line_messages(input_data: dict) -> dict: @action( name="reply_line_messages", + irreversible=True, description="Reply with up to 5 LINE message objects (rich reply).", action_sets=["line_messages", "line"], input_schema={ @@ -218,6 +221,7 @@ def broadcast_line_messages(input_data: dict) -> dict: @action( name="send_line_image", + irreversible=True, description="Push an image. Image must be publicly accessible HTTPS URL.", action_sets=["line_messages", "line"], input_schema={ @@ -250,6 +254,7 @@ def send_line_image(input_data: dict) -> dict: @action( name="send_line_video", + irreversible=True, description="Push a video (HTTPS URL + preview image).", action_sets=["line_messages", "line"], input_schema={ @@ -282,6 +287,7 @@ def send_line_video(input_data: dict) -> dict: @action( name="send_line_audio", + irreversible=True, description="Push an audio file. duration_ms is required.", action_sets=["line_messages", "line"], input_schema={ @@ -314,6 +320,7 @@ def send_line_audio(input_data: dict) -> dict: @action( name="send_line_location", + irreversible=True, description="Push a location pin.", action_sets=["line_messages", "line"], input_schema={ @@ -346,6 +353,7 @@ def send_line_location(input_data: dict) -> dict: @action( name="send_line_sticker", + irreversible=True, description="Push a LINE sticker. See https://developers.line.biz/en/docs/messaging-api/sticker-list/ for IDs.", action_sets=["line_messages", "line"], input_schema={ @@ -378,6 +386,7 @@ def send_line_sticker(input_data: dict) -> dict: @action( name="send_line_flex", + irreversible=True, description="Push a Flex Message — LINE's rich, interactive card format. contents is the Flex container JSON (bubble or carousel).", action_sets=["line_messages", "line"], input_schema={ @@ -410,6 +419,7 @@ def send_line_flex(input_data: dict) -> dict: @action( name="send_line_template", + irreversible=True, description="Push a template message: buttons / confirm / carousel / image_carousel. template is the Template object.", action_sets=["line_messages", "line"], input_schema={ @@ -438,6 +448,7 @@ def send_line_template(input_data: dict) -> dict: @action( name="send_line_imagemap", + irreversible=True, description="Push an imagemap: a clickable image overlaid with tappable regions. actions is a list of imagemap-action objects.", action_sets=["line_messages"], input_schema={ @@ -1008,6 +1019,7 @@ def bulk_unlink_line_rich_menu(input_data: dict) -> dict: @action( name="send_line_narrowcast", + irreversible=True, description="Send messages to a filtered subset of friends (demographics or audience groups). Returns a request_id; poll with get_line_narrowcast_progress.", action_sets=["line_audiences", "line"], input_schema={ diff --git a/app/data/action/integrations/linkedin/linkedin_actions.py b/app/data/action/integrations/linkedin/linkedin_actions.py index 63e23691..a7f4f090 100644 --- a/app/data/action/integrations/linkedin/linkedin_actions.py +++ b/app/data/action/integrations/linkedin/linkedin_actions.py @@ -352,6 +352,7 @@ def get_linkedin_connections(input_data: dict) -> dict: @action( name="send_linkedin_message", + irreversible=True, description="Send a message to LinkedIn users.", action_sets=["linkedin"], input_schema={ @@ -389,6 +390,7 @@ async def send_linkedin_message(input_data: dict) -> dict: @action( name="send_linkedin_connection_request", + irreversible=True, description="Send connection request.", action_sets=["linkedin"], input_schema={ diff --git a/app/data/action/integrations/outlook/outlook_actions.py b/app/data/action/integrations/outlook/outlook_actions.py index 7b03947e..61da8556 100644 --- a/app/data/action/integrations/outlook/outlook_actions.py +++ b/app/data/action/integrations/outlook/outlook_actions.py @@ -8,6 +8,7 @@ @action( name="send_outlook_email", + irreversible=True, description="Send an email via Outlook (Microsoft 365).", action_sets=["outlook_mail", "outlook"], input_schema={ @@ -173,6 +174,7 @@ def search_outlook_emails(input_data: dict) -> dict: @action( name="reply_outlook_email", + irreversible=True, description="Reply to the sender of an email. Sent immediately.", action_sets=["outlook_mail", "outlook"], input_schema={ @@ -217,6 +219,7 @@ def reply_outlook_email(input_data: dict) -> dict: @action( name="reply_all_outlook_email", + irreversible=True, description="Reply-all to an email. Sent immediately.", action_sets=["outlook_mail", "outlook"], input_schema={ @@ -245,6 +248,7 @@ def reply_all_outlook_email(input_data: dict) -> dict: @action( name="forward_outlook_email", + irreversible=True, description="Forward an email to other recipients.", action_sets=["outlook_mail", "outlook"], input_schema={ @@ -451,6 +455,7 @@ def update_outlook_draft(input_data: dict) -> dict: @action( name="send_outlook_draft", + irreversible=True, description="Send a previously-created draft.", action_sets=["outlook_mail", "outlook"], input_schema={ diff --git a/app/data/action/integrations/slack/slack_actions.py b/app/data/action/integrations/slack/slack_actions.py index 4efd6267..a7cd8f15 100644 --- a/app/data/action/integrations/slack/slack_actions.py +++ b/app/data/action/integrations/slack/slack_actions.py @@ -8,6 +8,7 @@ @action( name="send_slack_message", + irreversible=True, description="Send a message to a Slack channel or DM. Pass thread_ts to reply in a thread.", action_sets=["slack_messages", "slack"], input_schema={ @@ -112,6 +113,7 @@ def delete_slack_message(input_data: dict) -> dict: @action( name="send_slack_ephemeral", + irreversible=True, description="Send an ephemeral message visible only to one user in a channel.", action_sets=["slack_messages", "slack"], input_schema={ diff --git a/app/data/action/integrations/telegram/telegram_actions.py b/app/data/action/integrations/telegram/telegram_actions.py index 68c0d4cd..dec9554e 100644 --- a/app/data/action/integrations/telegram/telegram_actions.py +++ b/app/data/action/integrations/telegram/telegram_actions.py @@ -9,6 +9,7 @@ @action( name="send_telegram_bot_message", + irreversible=True, description="Send a text message to a Telegram chat via bot. Use this ONLY when replying to Telegram Bot messages.", action_sets=["telegram_messages", "telegram"], input_schema={ @@ -68,6 +69,7 @@ async def send_telegram_bot_message(input_data: dict) -> dict: @action( name="send_telegram_text_message", + irreversible=True, description="Send a text message via Telegram bot (alias for sendMessage with full options).", action_sets=["telegram_messages"], input_schema={ @@ -311,6 +313,7 @@ async def copy_telegram_message(input_data: dict) -> dict: @action( name="forward_telegram_message", + irreversible=True, description="Forward a message via bot.", action_sets=["telegram_messages", "telegram"], input_schema={ @@ -342,6 +345,7 @@ async def forward_telegram_message(input_data: dict) -> dict: @action( name="forward_telegram_messages", + irreversible=True, description="Forward multiple messages of any kind.", action_sets=["telegram_messages"], input_schema={ @@ -512,6 +516,7 @@ async def send_telegram_chat_action(input_data: dict) -> dict: @action( name="send_telegram_photo", + irreversible=True, description="Send a photo to a Telegram chat via bot.", action_sets=["telegram_media", "telegram"], input_schema={ @@ -539,6 +544,7 @@ async def send_telegram_photo(input_data: dict) -> dict: @action( name="send_telegram_document", + irreversible=True, description="Send a document to a Telegram chat via bot.", action_sets=["telegram_media", "telegram"], input_schema={ @@ -570,6 +576,7 @@ async def send_telegram_document(input_data: dict) -> dict: @action( name="send_telegram_video", + irreversible=True, description="Send a video file via bot.", action_sets=["telegram_media"], input_schema={ @@ -609,6 +616,7 @@ async def send_telegram_video(input_data: dict) -> dict: @action( name="send_telegram_audio", + irreversible=True, description="Send an audio file (music) via bot.", action_sets=["telegram_media"], input_schema={ @@ -640,6 +648,7 @@ async def send_telegram_audio(input_data: dict) -> dict: @action( name="send_telegram_voice", + irreversible=True, description="Send a voice message (OGG opus) via bot.", action_sets=["telegram_media"], input_schema={ @@ -673,6 +682,7 @@ async def send_telegram_voice(input_data: dict) -> dict: @action( name="send_telegram_video_note", + irreversible=True, description="Send a rounded square video note (short circular video).", action_sets=["telegram_media"], input_schema={ @@ -706,6 +716,7 @@ async def send_telegram_video_note(input_data: dict) -> dict: @action( name="send_telegram_animation", + irreversible=True, description="Send an animation (GIF or H.264/MPEG-4 without sound).", action_sets=["telegram_media"], input_schema={ @@ -733,6 +744,7 @@ async def send_telegram_animation(input_data: dict) -> dict: @action( name="send_telegram_sticker", + irreversible=True, description="Send a sticker (.webp / .tgs / .webm).", action_sets=["telegram_media"], input_schema={ @@ -758,6 +770,7 @@ async def send_telegram_sticker(input_data: dict) -> dict: @action( name="send_telegram_location", + irreversible=True, description="Send a geographic location.", action_sets=["telegram_media"], input_schema={ @@ -791,6 +804,7 @@ async def send_telegram_location(input_data: dict) -> dict: @action( name="send_telegram_venue", + irreversible=True, description="Send a venue with name and address.", action_sets=["telegram_media"], input_schema={ @@ -826,6 +840,7 @@ async def send_telegram_venue(input_data: dict) -> dict: @action( name="send_telegram_contact", + irreversible=True, description="Send a phone contact card.", action_sets=["telegram_media"], input_schema={ @@ -859,6 +874,7 @@ async def send_telegram_contact(input_data: dict) -> dict: @action( name="send_telegram_dice", + irreversible=True, description="Send an animated dice / emoji-game (🎲 🎯 🏀 ⚽ 🎳 🎰).", action_sets=["telegram_media"], input_schema={ @@ -884,6 +900,7 @@ async def send_telegram_dice(input_data: dict) -> dict: @action( name="send_telegram_poll", + irreversible=True, description="Send a poll to a chat.", action_sets=["telegram_media"], input_schema={ @@ -964,6 +981,7 @@ async def stop_telegram_poll(input_data: dict) -> dict: @action( name="send_telegram_media_group", + irreversible=True, description="Send a group of photos/videos/audios/documents as an album.", action_sets=["telegram_media"], input_schema={ @@ -2187,6 +2205,7 @@ async def read_telegram_messages(input_data: dict) -> dict: @action( name="send_telegram_user_message", + irreversible=True, description="Send a text message via Telegram user account. IMPORTANT: Use @username (e.g., '@emadtavana7') NOT numeric ID. Use 'self' or 'user' to message the owner's Saved Messages.", action_sets=["telegram_user", "telegram"], input_schema={ @@ -2216,6 +2235,7 @@ async def send_telegram_user_message(input_data: dict) -> dict: @action( name="send_telegram_user_file", + irreversible=True, description="Send a file via Telegram user account.", action_sets=["telegram_user"], input_schema={ diff --git a/app/data/action/integrations/twitter/twitter_actions.py b/app/data/action/integrations/twitter/twitter_actions.py index d450a1e3..c00f97e8 100644 --- a/app/data/action/integrations/twitter/twitter_actions.py +++ b/app/data/action/integrations/twitter/twitter_actions.py @@ -9,6 +9,7 @@ @action( name="post_tweet", + irreversible=True, description="Post a tweet on Twitter/X.", action_sets=["twitter_tweets", "twitter"], input_schema={ @@ -39,6 +40,7 @@ async def post_tweet(input_data: dict) -> dict: @action( name="reply_to_tweet", + irreversible=True, description="Reply to a tweet on Twitter/X.", action_sets=["twitter_tweets", "twitter"], input_schema={ @@ -214,6 +216,7 @@ async def get_twitter_mentions(input_data: dict) -> dict: @action( name="post_quote_tweet", + irreversible=True, description="Post a quote tweet that wraps another tweet with your own commentary.", action_sets=["twitter_tweets", "twitter"], input_schema={ @@ -274,6 +277,7 @@ async def hide_tweet_reply(input_data: dict) -> dict: @action( name="post_tweet_with_media", + irreversible=True, description="Post a tweet that includes already-uploaded media (use upload_twitter_media first to get media_ids).", action_sets=["twitter_tweets", "twitter"], input_schema={ @@ -1001,6 +1005,7 @@ async def list_twitter_list_tweets(input_data: dict) -> dict: @action( name="send_twitter_dm", + irreversible=True, description="Send a one-on-one direct message on Twitter/X (creates the conversation if needed).", action_sets=["twitter_dms", "twitter"], input_schema={ @@ -1027,6 +1032,7 @@ async def send_twitter_dm(input_data: dict) -> dict: @action( name="send_twitter_dm_to_conversation", + irreversible=True, description="Send a DM into an existing conversation by ID.", action_sets=["twitter_dms"], input_schema={ diff --git a/app/data/action/integrations/whatsapp/whatsapp_actions.py b/app/data/action/integrations/whatsapp/whatsapp_actions.py index 6c3fc10b..d5f129ba 100644 --- a/app/data/action/integrations/whatsapp/whatsapp_actions.py +++ b/app/data/action/integrations/whatsapp/whatsapp_actions.py @@ -8,6 +8,7 @@ @action( name="send_whatsapp_web_text_message", + irreversible=True, description="Send a text message via WhatsApp Web.", action_sets=["whatsapp_messages", "whatsapp"], input_schema={ @@ -42,6 +43,7 @@ async def send_whatsapp_web_text_message(input_data: dict) -> dict: @action( name="send_whatsapp_web_media_message", + irreversible=True, description="Send a media file (image / video / audio / document) via WhatsApp Web. Set send_as_sticker / send_as_voice / send_as_document to override the default mode.", action_sets=["whatsapp_messages", "whatsapp"], input_schema={ @@ -102,6 +104,7 @@ async def send_whatsapp_web_media_message(input_data: dict) -> dict: @action( name="send_whatsapp_location", + irreversible=True, description="Send a location pin via WhatsApp Web.", action_sets=["whatsapp_messages", "whatsapp"], input_schema={ @@ -136,6 +139,7 @@ async def send_whatsapp_location(input_data: dict) -> dict: @action( name="reply_whatsapp_message", + irreversible=True, description="Quote-reply to a specific WhatsApp message.", action_sets=["whatsapp_messages", "whatsapp"], input_schema={ @@ -220,6 +224,7 @@ async def delete_whatsapp_message(input_data: dict) -> dict: @action( name="forward_whatsapp_message", + irreversible=True, description="Forward a message to another chat.", action_sets=["whatsapp_messages", "whatsapp"], input_schema={ diff --git a/app/data/action/schedule_task.py b/app/data/action/schedule_task.py index 7fe7f441..7f620f95 100644 --- a/app/data/action/schedule_task.py +++ b/app/data/action/schedule_task.py @@ -149,7 +149,7 @@ async def schedule_task(input_data: dict) -> dict: } # Handle immediate execution — queue_immediate_trigger emits durably - # via TriggerService when the scheduler has one wired (issue #321). + # via TriggerService when the scheduler has one wired. if schedule_expr.lower() == "immediate": return await scheduler.queue_immediate_trigger( name=name, diff --git a/app/data/action/send_message.py b/app/data/action/send_message.py index b6ff597a..f486cc60 100644 --- a/app/data/action/send_message.py +++ b/app/data/action/send_message.py @@ -3,6 +3,7 @@ @action( name="send_message", + irreversible=True, description="Use this action to deliver a detailed text update that will be recorded in the conversation log and event stream. Avoid revealing internal or sensitive information and do not mention conversation identifiers. This action does not perform work; it only communicates status to the user. This action can be executed in parallel with other actions, but do not use multiple send_message actions at the same time as that is redundant - combine messages into one.", default=True, action_sets=["core"], diff --git a/app/data/action/send_message_with_attachment.py b/app/data/action/send_message_with_attachment.py index 2fff4639..1546252d 100644 --- a/app/data/action/send_message_with_attachment.py +++ b/app/data/action/send_message_with_attachment.py @@ -3,6 +3,7 @@ @action( name="send_message_with_attachment", + irreversible=True, description="Send a message to the user with one or more file attachments. Use this when you need to share files (documents, images, reports, etc.) with the user. All files must exist at the specified paths.", default=True, action_sets=["core"], diff --git a/app/living_ui/manager.py b/app/living_ui/manager.py index 2990b329..e41df38d 100644 --- a/app/living_ui/manager.py +++ b/app/living_ui/manager.py @@ -142,7 +142,7 @@ def bind_task_manager( task_manager: TaskManager instance for creating tasks trigger_queue: TriggerQueue instance for firing triggers trigger_service: Optional TriggerService for durable emits - (issue #321); falls back to direct queue puts when None. + ; falls back to direct queue puts when None. """ self._task_manager = task_manager self._trigger_queue = trigger_queue diff --git a/app/scheduler/manager.py b/app/scheduler/manager.py index c0792fa2..99a0e686 100644 --- a/app/scheduler/manager.py +++ b/app/scheduler/manager.py @@ -58,7 +58,7 @@ async def initialize( config_path: Path to scheduler_config.json trigger_queue: TriggerQueue to fire triggers into trigger_service: Optional TriggerService. When provided, fires are - emitted durably with dedup keys (issue #321); when None, falls + emitted durably with dedup keys; when None, falls back to direct queue puts (legacy behavior, used by old tests). """ self._config_path = Path(config_path) diff --git a/app/triggers/__init__.py b/app/triggers/__init__.py index 0032f662..cc1fccbf 100644 --- a/app/triggers/__init__.py +++ b/app/triggers/__init__.py @@ -2,7 +2,7 @@ """ app.triggers -Durable trigger execution (issue #321): typed sources, the SQLite-backed +Durable trigger execution: typed sources, the SQLite-backed TriggerStore, and the TriggerService producer/consumer front door. """ diff --git a/app/triggers/activity_log.py b/app/triggers/activity_log.py new file mode 100644 index 00000000..94b2ddae --- /dev/null +++ b/app/triggers/activity_log.py @@ -0,0 +1,288 @@ +# -*- coding: utf-8 -*- +""" +app.triggers.activity_log + +Activity ledger for irreversible actions. + +One row per attempted irreversible side effect, keyed by a deterministic +idempotency key (session + action + content hash of the inputs): + +- INTENT — written BEFORE the side effect runs +- DONE — written after success, with the stored output + provider ref +- FAILED — written after a failed run (a retake re-records INTENT) + +Lifecycle answers "did this already run?" as a database check instead of +LLM prose-reading: + +- fresh key → record INTENT, execute +- DONE row → return the stored output, skip the handler +- stale INTENT row → a previous attempt was interrupted mid-flight; the + side effect MAY have happened. Refuse once with a warning (the LLM should + verify or confirm with the user), and mark the row so a deliberate retry + afterwards is allowed through. +- FAILED row → retake (re-record INTENT, execute) + +Honest limitation (same as Temporal's): if the LLM regenerates the call +with different wording, the content hash differs and the ledger can't link +the attempts — the guard catches the common case (the same step re-executed +with the same inputs after a crash/redelivery), and the stale-INTENT warning +covers the rest. No current provider accepts an idempotency key, so dedup +is enforced at this ledger, with the provider's returned id stored as an +audit ref. + +Same file and idioms as the trigger store (sessions.db, WAL, short +per-operation connections). +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import sqlite3 +from datetime import datetime, timezone +from pathlib import Path +from typing import Any, Dict, Optional + +from agent_core.core.impl.action.idempotency import GuardDecision + +try: + from app.logger import logger +except Exception: + logger = logging.getLogger(__name__) + logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") + + +STATUS_INTENT = "INTENT" +STATUS_DONE = "DONE" +STATUS_FAILED = "FAILED" + +# Stored outputs larger than this are replaced by a stub (the ledger is for +# dedup + audit, not bulk storage). +MAX_STORED_OUTPUT_BYTES = 50_000 + +# Output keys commonly carrying the provider's id for the side effect +# (email message id, chat message ts, post id) — stored for audit. +PROVIDER_REF_KEYS = ("message_id", "messageId", "id", "ts", "post_id", "email_id") + + +def _now_iso() -> str: + return datetime.now(timezone.utc).isoformat() + + +def make_idem_key( + action_name: str, input_data: Dict[str, Any], session_id: Optional[str] +) -> str: + """Deterministic identity of one irreversible side effect. + + Internal keys (``_``-prefixed, injected by the executor) are excluded so + the hash reflects only the user-visible content of the action. + """ + content = { + k: v for k, v in (input_data or {}).items() if not k.startswith("_") + } + canonical = json.dumps(content, sort_keys=True, default=str) + digest = hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:16] + return f"{session_id or 'conv'}:{action_name}:{digest}" + + +class ActivityLog: + """SQLite persistence for the irreversible-action ledger.""" + + def __init__(self, db_path: Optional[str] = None): + if db_path is None: + from app.config import APP_DATA_PATH + + usage_dir = Path(APP_DATA_PATH) / ".usage" + usage_dir.mkdir(parents=True, exist_ok=True) + db_path = str(usage_dir / "sessions.db") + + self._db_path = db_path + self._init_db() + logger.info(f"[ActivityLog] Initialized at {self._db_path}") + + def _connect(self) -> sqlite3.Connection: + conn = sqlite3.connect(self._db_path) + conn.execute("PRAGMA busy_timeout=5000") + return conn + + def _init_db(self) -> None: + with self._connect() as conn: + conn.execute("PRAGMA journal_mode=WAL") + conn.execute(""" + CREATE TABLE IF NOT EXISTS activity_log ( + idem_key TEXT PRIMARY KEY, + task_id TEXT, + action TEXT NOT NULL, + status TEXT NOT NULL, + provider_ref TEXT, + output_json TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL + ) + """) + conn.commit() + + def get(self, idem_key: str) -> Optional[Dict[str, Any]]: + with self._connect() as conn: + conn.row_factory = sqlite3.Row + row = conn.execute( + "SELECT * FROM activity_log WHERE idem_key = ?", (idem_key,) + ).fetchone() + return dict(row) if row else None + + def record_intent( + self, idem_key: str, action: str, task_id: Optional[str] + ) -> None: + """Upsert the row to INTENT (fresh attempt or deliberate retake).""" + now = _now_iso() + with self._connect() as conn: + conn.execute( + """ + INSERT INTO activity_log + (idem_key, task_id, action, status, created_at, updated_at) + VALUES (?, ?, ?, 'INTENT', ?, ?) + ON CONFLICT(idem_key) DO UPDATE SET + status = 'INTENT', + provider_ref = NULL, + output_json = NULL, + updated_at = excluded.updated_at + """, + (idem_key, task_id, action, now, now), + ) + conn.commit() + + def record_outcome( + self, + idem_key: str, + status: str, + provider_ref: Optional[str], + output_json: Optional[str], + ) -> None: + with self._connect() as conn: + conn.execute( + """ + UPDATE activity_log + SET status = ?, provider_ref = ?, output_json = ?, updated_at = ? + WHERE idem_key = ? + """, + (status, provider_ref, output_json, _now_iso(), idem_key), + ) + conn.commit() + + def mark_uncertain_surfaced(self, idem_key: str) -> None: + """A stale INTENT was surfaced to the LLM once — downgrade to FAILED + so a subsequent deliberate retry is allowed through (retake).""" + with self._connect() as conn: + conn.execute( + """ + UPDATE activity_log + SET status = 'FAILED', updated_at = ? + WHERE idem_key = ? AND status = 'INTENT' + """, + (_now_iso(), idem_key), + ) + conn.commit() + + def clear_all(self) -> None: + with self._connect() as conn: + conn.execute("DELETE FROM activity_log") + conn.commit() + + +class ActivityLogGuard: + """IdempotencyGuard implementation backed by the activity ledger.""" + + def __init__(self, log: ActivityLog): + self._log = log + + def begin( + self, + action_name: str, + input_data: Dict[str, Any], + session_id: Optional[str], + ) -> GuardDecision: + idem_key = make_idem_key(action_name, input_data, session_id) + row = self._log.get(idem_key) + + if row is None or row["status"] == STATUS_FAILED: + # Fresh attempt, or deliberate retake after a failed/uncertain + # run — record intent BEFORE the side effect. + self._log.record_intent(idem_key, action_name, session_id) + return GuardDecision(proceed=True, idem_key=idem_key) + + if row["status"] == STATUS_DONE: + # This exact side effect already completed — return its stored + # output instead of doing it again. + try: + stored = json.loads(row["output_json"]) if row["output_json"] else {} + except (ValueError, TypeError): + stored = {} + stored["_idempotent_replay"] = True + stored["_idempotent_note"] = ( + f"{action_name} was NOT re-executed: an identical run already " + f"completed (provider ref: {row['provider_ref'] or 'n/a'}). " + f"The original output is returned. Do not retry." + ) + logger.info( + f"[ActivityLog] Skipped duplicate {action_name} ({idem_key})" + ) + return GuardDecision( + proceed=False, idem_key=idem_key, stored_output=stored + ) + + # Stale INTENT: a previous attempt was interrupted between starting + # the side effect and recording its outcome. Surface once; the next + # identical attempt (if the LLM/user decides to retry) is allowed. + self._log.mark_uncertain_surfaced(idem_key) + note = ( + f"{action_name} was NOT executed. A previous attempt with these " + f"exact inputs was interrupted before its outcome could be " + f"recorded — the side effect (e.g. the message or email) MAY " + f"have already reached the recipient. Verify first (check the " + f"sent folder / channel history) or confirm with the user. If " + f"you determine it did not go out, run the action again — it " + f"will be allowed through." + ) + logger.warning( + f"[ActivityLog] Uncertain prior attempt for {action_name} " + f"({idem_key}) — surfaced to the agent" + ) + return GuardDecision(proceed=False, idem_key=idem_key, note=note) + + def complete( + self, + idem_key: str, + status: str, + outputs: Optional[Dict[str, Any]], + ) -> None: + ledger_status = STATUS_DONE if status == "success" else STATUS_FAILED + + provider_ref = None + output_json = None + if isinstance(outputs, dict): + for key in PROVIDER_REF_KEYS: + if outputs.get(key): + provider_ref = str(outputs[key]) + break + output_json = json.dumps(outputs, default=str) + if len(output_json) > MAX_STORED_OUTPUT_BYTES: + output_json = json.dumps( + { + "status": outputs.get("status", status), + "note": "output too large for the activity ledger", + } + ) + + self._log.record_outcome(idem_key, ledger_status, provider_ref, output_json) + + +# Global instance (same pattern as get_session_storage / get_trigger_store) +_activity_log: Optional[ActivityLog] = None + + +def get_activity_log() -> ActivityLog: + global _activity_log + if _activity_log is None: + _activity_log = ActivityLog() + return _activity_log diff --git a/app/triggers/router.py b/app/triggers/router.py index fc68f7c7..b048b7b1 100644 --- a/app/triggers/router.py +++ b/app/triggers/router.py @@ -2,8 +2,7 @@ """ app.triggers.router -SessionRouter — decides which session an incoming item belongs to -(issue #321, Phase 3). +SessionRouter — decides which session an incoming item belongs to. This is the ONE routing implementation. It was extracted from AgentBase (`_route_to_session` + context formatters); the second, near-duplicate diff --git a/app/triggers/service.py b/app/triggers/service.py index 67d5e285..7f460710 100644 --- a/app/triggers/service.py +++ b/app/triggers/service.py @@ -2,8 +2,7 @@ """ app.triggers.service -TriggerService — the single producer front door for durable triggers -(issue #321, Primitive A). +TriggerService — the single producer front door for durable triggers. ``emit()`` writes the trigger to the store FIRST (no LLM call, sub-ms), then feeds the in-memory TriggerQueue, which stays as the ordering primitive. The diff --git a/app/triggers/sources.py b/app/triggers/sources.py index 63a726f7..46673e81 100644 --- a/app/triggers/sources.py +++ b/app/triggers/sources.py @@ -2,7 +2,7 @@ """ app.triggers.sources -Typed trigger sources and dedup-key builders (issue #321). +Typed trigger sources and dedup-key builders. Phase 1 defines only the sources migrated to TriggerService so far; Phase 2 extends this enum to every producer and removes the scattered diff --git a/app/triggers/store.py b/app/triggers/store.py index ec786a39..e31785fe 100644 --- a/app/triggers/store.py +++ b/app/triggers/store.py @@ -2,7 +2,7 @@ """ app.triggers.store -Durable trigger store (issue #321, Primitive A). +Durable trigger store. SQLite-backed write-ahead store for triggers: a trigger accepted by TriggerService is INSERTed here as PENDING before it can run, claimed diff --git a/app/ui_layer/controller/ui_controller.py b/app/ui_layer/controller/ui_controller.py index fa3ba19a..e15ac421 100644 --- a/app/ui_layer/controller/ui_controller.py +++ b/app/ui_layer/controller/ui_controller.py @@ -513,7 +513,7 @@ def _update_state_from_event(self, event: UIEvent) -> None: async def _consume_triggers(self) -> None: """Consume triggers and run agent reactions. - Durable lifecycle (issue #321): ``next()`` claims the trigger's store + Durable lifecycle: ``next()`` claims the trigger's store rows (CLAIMED), ``ack()`` settles them when the react cycle completes, ``nack()`` on an exception. A crash or cancellation mid-react leaves the rows CLAIMED, and the next boot's reclaim scan re-delivers them — diff --git a/app/usage/session_storage.py b/app/usage/session_storage.py index ddecf578..21e245b2 100644 --- a/app/usage/session_storage.py +++ b/app/usage/session_storage.py @@ -101,7 +101,7 @@ def _init_db(self) -> None: """) # NOTE: the `triggers` table is owned by app/triggers/store.py - # (durable trigger store, issue #321) — do not touch it here. + # (durable trigger store) — do not touch it here. conn.commit() diff --git a/tests/test_activity_log.py b/tests/test_activity_log.py new file mode 100644 index 00000000..7659e93d --- /dev/null +++ b/tests/test_activity_log.py @@ -0,0 +1,137 @@ +# -*- coding: utf-8 -*- +"""Phase 4 tests: activity ledger + idempotency guard for irreversible +actions (, Primitive C).""" + +from agent_core.core.action.action import Action + +from app.triggers.activity_log import ( + ActivityLog, + ActivityLogGuard, + make_idem_key, +) + + +def make_guard(tmp_path): + log = ActivityLog(db_path=str(tmp_path / "sessions.db")) + return log, ActivityLogGuard(log) + + +INPUTS = {"to": "bob@example.com", "subject": "Q3 report", "body": "attached"} + + +class TestIdemKey: + def test_deterministic_and_content_sensitive(self): + k1 = make_idem_key("send_gmail", INPUTS, "task1") + k2 = make_idem_key("send_gmail", dict(INPUTS), "task1") + assert k1 == k2 + assert k1 != make_idem_key("send_gmail", {**INPUTS, "subject": "Q4"}, "task1") + assert k1 != make_idem_key("send_gmail", INPUTS, "task2") + assert k1 != make_idem_key("send_outlook_email", INPUTS, "task1") + + def test_internal_keys_excluded(self): + # Executor-injected keys (_session_id etc.) must not change identity. + k1 = make_idem_key("send_gmail", INPUTS, "task1") + k2 = make_idem_key( + "send_gmail", {**INPUTS, "_session_id": "task1", "_extra": "x"}, "task1" + ) + assert k1 == k2 + + +class TestGuardLifecycle: + def test_fresh_attempt_records_intent_and_proceeds(self, tmp_path): + log, guard = make_guard(tmp_path) + decision = guard.begin("send_gmail", INPUTS, "task1") + assert decision.proceed + row = log.get(decision.idem_key) + assert row["status"] == "INTENT" + + def test_completed_run_is_never_reexecuted(self, tmp_path): + log, guard = make_guard(tmp_path) + d1 = guard.begin("send_gmail", INPUTS, "task1") + guard.complete( + d1.idem_key, "success", {"status": "success", "message_id": "msg-42"} + ) + + # The crash-redelivery case: the same step re-runs with the same inputs. + d2 = guard.begin("send_gmail", INPUTS, "task1") + assert not d2.proceed + assert d2.stored_output["_idempotent_replay"] is True + assert d2.stored_output["message_id"] == "msg-42" + assert log.get(d1.idem_key)["provider_ref"] == "msg-42" + + def test_crash_window_surfaces_uncertainty_once_then_allows_retry( + self, tmp_path + ): + log, guard = make_guard(tmp_path) + d1 = guard.begin("send_gmail", INPUTS, "task1") + # crash between the send and complete(): row stays INTENT + + log2 = ActivityLog(db_path=str(tmp_path / "sessions.db")) # restart + guard2 = ActivityLogGuard(log2) + + # First re-attempt: blocked with a warning, NOT executed. + d2 = guard2.begin("send_gmail", INPUTS, "task1") + assert not d2.proceed + assert d2.stored_output is None + assert "MAY" in d2.note + + # The agent/user verified and decided to retry: now allowed through. + d3 = guard2.begin("send_gmail", INPUTS, "task1") + assert d3.proceed + assert log2.get(d1.idem_key)["status"] == "INTENT" + + def test_failed_run_can_be_retried(self, tmp_path): + log, guard = make_guard(tmp_path) + d1 = guard.begin("send_gmail", INPUTS, "task1") + guard.complete(d1.idem_key, "error", {"status": "error", "error": "timeout"}) + + d2 = guard.begin("send_gmail", INPUTS, "task1") + assert d2.proceed # failure is retryable; only DONE blocks + + def test_different_content_is_independent(self, tmp_path): + log, guard = make_guard(tmp_path) + d1 = guard.begin("send_gmail", INPUTS, "task1") + guard.complete(d1.idem_key, "success", {"status": "success"}) + + # A genuinely different email must not be blocked. + d2 = guard.begin("send_gmail", {**INPUTS, "to": "carol@example.com"}, "task1") + assert d2.proceed + + def test_oversized_output_is_stubbed(self, tmp_path): + log, guard = make_guard(tmp_path) + d1 = guard.begin("send_gmail", INPUTS, "task1") + guard.complete( + d1.idem_key, "success", {"status": "success", "blob": "x" * 100_000} + ) + row = log.get(d1.idem_key) + assert len(row["output_json"]) < 1000 + assert "too large" in row["output_json"] + + +class TestFlagPlumbing: + def test_irreversible_round_trips_through_action_dict(self): + action = Action( + name="send_gmail", + description="send an email", + action_type="atomic", + irreversible=True, + ) + restored = Action.from_dict(action.to_dict()) + assert restored.irreversible is True + + def test_irreversible_defaults_false(self): + action = Action.from_dict( + {"name": "read_file", "description": "", "type": "atomic"} + ) + assert action.irreversible is False + + def test_flagged_actions_registered(self): + # Import a couple of flagged modules and confirm the registry + # carries the flag (decorator → metadata path). + import app.data.action.send_message # noqa: F401 + from agent_core.core.action_framework.registry import registry_instance + + impls = registry_instance._registry.get("send_message") + assert impls, "send_message not registered" + meta = next(iter(impls.values())).metadata + assert meta.irreversible is True diff --git a/tests/test_trigger_service.py b/tests/test_trigger_service.py index b11acc88..b635eda9 100644 --- a/tests/test_trigger_service.py +++ b/tests/test_trigger_service.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- """Integration tests for TriggerService + TriggerQueue + TriggerStore -(issue #321, Phase 1) — including crash/restart simulations.""" + — including crash/restart simulations.""" import asyncio import heapq diff --git a/tests/test_trigger_store.py b/tests/test_trigger_store.py index f5a5ae33..ed7bf8db 100644 --- a/tests/test_trigger_store.py +++ b/tests/test_trigger_store.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -"""Unit tests for the durable trigger store (issue #321, Phase 1).""" +"""Unit tests for the durable trigger store.""" import sqlite3 import time From 0ed5cd9eccd5dc3f9a8111cc532b13e30590a6bb Mon Sep 17 00:00:00 2001 From: ahmad-ajmal Date: Fri, 12 Jun 2026 03:28:05 +0100 Subject: [PATCH 5/5] retries and backoffs --- app/agent_base.py | 25 +++- app/scheduler/manager.py | 13 ++ app/triggers/activity_log.py | 42 ++++++ app/triggers/service.py | 73 +++++++++- app/triggers/store.py | 56 +++++++- scripts/peek_durable_state.py | 57 ++++++++ tests/test_trigger_lifecycle_polish.py | 187 +++++++++++++++++++++++++ tests/test_trigger_service.py | 8 +- 8 files changed, 449 insertions(+), 12 deletions(-) create mode 100644 scripts/peek_durable_state.py create mode 100644 tests/test_trigger_lifecycle_polish.py diff --git a/app/agent_base.py b/app/agent_base.py index 97bcb9a4..acd657b3 100644 --- a/app/agent_base.py +++ b/app/agent_base.py @@ -3711,15 +3711,38 @@ def step(step_num: int, total: int, message: str) -> None: scheduler_config_path, self.scheduler.reload, name="scheduler_config.json" ) + # Dead-letter surfacing: a trigger that exhausts its retries is work + # that silently stopped — tell the user instead of hiding it. + def _on_dead_letter(trig, _error: str) -> None: + # The raw error is already logged by the service; the user gets + # the what, not the traceback. + desc = (trig.next_action_description or "").strip() + if len(desc) > 120: + desc = desc[:117] + "..." + self.state_manager.record_agent_message( + f"⚠️ A background task trigger failed repeatedly and was " + f'parked: "{desc}". I won\'t retry it automatically — ' + f"ask me to try again if it still matters." + ) + + self.trigger_service.set_dead_letter_handler(_on_dead_letter) + # Rehydrate unfinished durable triggers from the previous run BEFORE # scheduling restored-task resumes: the resume emits below carry # dedup keys, so a rehydrated resume row blocks the duplicate instead - # of double-enqueueing. + # of double-enqueueing. (Trigger-store GC runs inside rehydrate.) try: await self.trigger_service.rehydrate() except Exception as e: logger.warning(f"[RESTORE] Trigger rehydration failed: {e}") + # Ledger housekeeping: stale INTENT rows stop blocking, old settled + # rows age out (payloads can contain message content). + try: + self.activity_log.gc() + except Exception as e: + logger.warning(f"[RESTORE] Activity log GC failed: {e}") + # Resume triggers for tasks restored from previous session await self._schedule_restored_task_triggers() diff --git a/app/scheduler/manager.py b/app/scheduler/manager.py index 99a0e686..78720302 100644 --- a/app/scheduler/manager.py +++ b/app/scheduler/manager.py @@ -649,6 +649,19 @@ async def _fire_schedule(self, schedule: ScheduledTask) -> None: # same fire dedups but the next occurrence does not. dedup_key = scheduled_dedup_key(schedule.id, schedule.next_run or now) + # Built-in schedules (scheduler_config.json) carry their workflow + # type in their custom payload — promote it to the typed source + # so react() classification doesn't depend on the payload["type"] + # fallback (kept only as belt-and-braces for old configs). + payload_type_to_source = { + "memory_processing": TriggerSource.MEMORY, + "proactive_heartbeat": TriggerSource.PROACTIVE_HEARTBEAT, + "proactive_planner": TriggerSource.PROACTIVE_PLANNER, + } + promoted = payload_type_to_source.get(payload.get("type")) + if promoted is not None: + source = promoted + result = await self._trigger_service.emit( TriggerSpec( source=source, diff --git a/app/triggers/activity_log.py b/app/triggers/activity_log.py index 94b2ddae..536564aa 100644 --- a/app/triggers/activity_log.py +++ b/app/triggers/activity_log.py @@ -40,6 +40,7 @@ import json import logging import sqlite3 +import time from datetime import datetime, timezone from pathlib import Path from typing import Any, Dict, Optional @@ -184,6 +185,47 @@ def mark_uncertain_surfaced(self, idem_key: str) -> None: ) conn.commit() + def gc( + self, + intent_ttl_hours: float = 7 * 24, + done_ttl_hours: float = 30 * 24, + ) -> int: + """Boot-time housekeeping for the ledger. + + Stale INTENT rows (an interrupted attempt nobody ever retried) are + downgraded to FAILED so they stop blocking, then aged out with the + rest. Settled rows are deleted after the TTL — ledger payloads can + contain message content, so they must not accumulate forever. + """ + now = time.time() + intent_cutoff = datetime.fromtimestamp( + now - intent_ttl_hours * 3600, tz=timezone.utc + ).isoformat() + done_cutoff = datetime.fromtimestamp( + now - done_ttl_hours * 3600, tz=timezone.utc + ).isoformat() + with self._connect() as conn: + conn.execute( + """ + UPDATE activity_log SET status = 'FAILED', updated_at = ? + WHERE status = 'INTENT' AND updated_at < ? + """, + (_now_iso(), intent_cutoff), + ) + cur = conn.execute( + """ + DELETE FROM activity_log + WHERE status IN ('DONE', 'FAILED') AND updated_at < ? + """, + (done_cutoff,), + ) + conn.commit() + if cur.rowcount: + logger.info( + f"[ActivityLog] GC removed {cur.rowcount} settled ledger row(s)" + ) + return cur.rowcount + def clear_all(self) -> None: with self._connect() as conn: conn.execute("DELETE FROM activity_log") diff --git a/app/triggers/service.py b/app/triggers/service.py index 7f460710..a5f74581 100644 --- a/app/triggers/service.py +++ b/app/triggers/service.py @@ -48,6 +48,15 @@ # scheduler-only behavior to every source). CATCHUP_THRESHOLD_SECONDS = 120 +# Retry policy for triggers whose react cycle raised: exponential backoff +# (30s, 60s, 120s, 240s, capped at 1h), then dead-letter after MAX_ATTEMPTS. +MAX_ATTEMPTS = 5 +BACKOFF_BASE_SECONDS = 30 +BACKOFF_CAP_SECONDS = 3600 + +# Settled rows older than this are garbage-collected at boot. +GC_TTL_HOURS = 7 * 24 + @dataclass class TriggerSpec: @@ -91,8 +100,16 @@ class TriggerService: def __init__(self, store: TriggerStore, queue: TriggerQueue) -> None: self._store = store self._queue = queue + # Optional callback(trigger, error) invoked when a trigger exhausts + # its retries and is parked DEAD — the app layer surfaces it to the + # user (a dead-lettered trigger is work that silently stopped). + self._on_dead_letter = None queue.set_lifecycle_listener(self) + def set_dead_letter_handler(self, handler) -> None: + """Register callback(trigger, error) fired on the DEAD transition.""" + self._on_dead_letter = handler + # ─────────────────────── Producer API ─────────────────────────────────── async def emit(self, spec: TriggerSpec) -> EmitResult: @@ -196,9 +213,52 @@ async def ack(self, trig: Trigger) -> None: self._store.ack([trig.id]) async def nack(self, trig: Trigger, error: str) -> None: - """The react cycle raised before completing.""" - if trig.id is not None: - self._store.fail([trig.id], error=error) + """The react cycle raised before completing — retry with backoff. + + attempts < MAX_ATTEMPTS: the row goes back to PENDING with an + exponential backoff floor and is re-enqueued. Otherwise it is parked + DEAD and surfaced via the dead-letter handler. (react() handles most + of its own errors internally; this path covers consumer-level + failures, so the retry budget is rarely consumed.) + """ + if trig.id is None: + return + row = self._store.get(trig.id) + attempts = row["attempts"] if row else MAX_ATTEMPTS + + if attempts >= MAX_ATTEMPTS: + self._store.mark_dead([trig.id], error=error) + logger.error( + f"[TriggerService] Trigger {trig.id} ({trig.source}) dead-lettered " + f"after {attempts} attempts: {error}" + ) + if self._on_dead_letter: + try: + self._on_dead_letter(trig, error) + except Exception as e: + logger.warning(f"[TriggerService] Dead-letter handler failed: {e}") + return + + backoff = min( + BACKOFF_BASE_SECONDS * (2 ** max(attempts - 1, 0)), BACKOFF_CAP_SECONDS + ) + not_before = time.time() + backoff + self._store.retry(trig.id, not_before, error=error) + logger.warning( + f"[TriggerService] Trigger {trig.id} ({trig.source}) failed " + f"(attempt {attempts}/{MAX_ATTEMPTS}), retrying in {int(backoff)}s: {error}" + ) + retry_trig = Trigger( + fire_at=not_before, + priority=trig.priority, + next_action_description=trig.next_action_description, + payload=dict(trig.payload), + session_id=trig.session_id, + waiting_for_reply=trig.waiting_for_reply, + id=trig.id, + source=trig.source, + ) + await self._queue.put(retry_trig) # ─────────────────────── fire() pass-through ──────────────────────────── @@ -317,6 +377,13 @@ async def rehydrate(self) -> int: f"[TriggerService] Rehydrated {requeued} pending trigger(s) " "from previous run" ) + + # Boot-time housekeeping: drop settled rows past the TTL. + try: + self._store.gc(ttl_hours=GC_TTL_HOURS) + except Exception as e: + logger.warning(f"[TriggerService] Trigger GC failed: {e}") + return requeued # ─────────────────────── Session / reset cleanup ──────────────────────── diff --git a/app/triggers/store.py b/app/triggers/store.py index e31785fe..7ef5bb76 100644 --- a/app/triggers/store.py +++ b/app/triggers/store.py @@ -280,16 +280,36 @@ def ack(self, ids: List[int]) -> None: self._set_status(ids, STATUS_DONE, resolution=RESOLUTION_COMPLETED) def fail(self, ids: List[int], error: Optional[str] = None) -> None: - """The react cycle raised — settle the rows (→ FAILED). - - Phase 5 turns this into retry-with-backoff; until then FAILED is - terminal (matching today's behavior where the consumer logs and - moves on). - """ + """Settle the rows as FAILED (terminal — retries exhausted or + explicitly not retryable).""" self._set_status( ids, STATUS_FAILED, resolution=RESOLUTION_FAILED, last_error=error ) + def retry( + self, row_id: int, not_before: float, error: Optional[str] = None + ) -> None: + """A failed attempt gets another chance: back to PENDING with a + backoff floor. ``attempts`` is preserved (claim increments it).""" + with self._connect() as conn: + conn.execute( + """ + UPDATE triggers + SET status = 'PENDING', not_before = ?, fire_at = ?, + claimed_by = NULL, lease_until = NULL, + last_error = COALESCE(?, last_error), updated_at = ? + WHERE id = ? + """, + (not_before, not_before, error, _now_iso(), row_id), + ) + conn.commit() + + def mark_dead(self, ids: List[int], error: Optional[str] = None) -> None: + """Retries exhausted — park the rows as DEAD (dead-letter).""" + self._set_status( + ids, STATUS_DEAD, resolution=RESOLUTION_FAILED, last_error=error + ) + def supersede(self, ids: List[int], by_id: Optional[int]) -> None: """Rows replaced by a newer same-session trigger (→ DONE, marked).""" self._set_status( @@ -403,6 +423,30 @@ def update_for_fire( conn.commit() return updated + def gc(self, ttl_hours: float = 7 * 24) -> int: + """Delete settled rows (DONE/FAILED/DEAD) older than the TTL. + + ISO-8601 UTC timestamps compare lexicographically, so a string + comparison against the cutoff is correct. + """ + cutoff = datetime.fromtimestamp( + time.time() - ttl_hours * 3600, tz=timezone.utc + ).isoformat() + with self._connect() as conn: + cur = conn.execute( + """ + DELETE FROM triggers + WHERE status IN ('DONE', 'FAILED', 'DEAD') AND updated_at < ? + """, + (cutoff,), + ) + conn.commit() + if cur.rowcount: + logger.info( + f"[TriggerStore] GC removed {cur.rowcount} settled trigger row(s)" + ) + return cur.rowcount + def update_session(self, row_id: int, session_id: str) -> None: """Assign a session to a row (recovery of unrouted parked messages).""" with self._connect() as conn: diff --git a/scripts/peek_durable_state.py b/scripts/peek_durable_state.py new file mode 100644 index 00000000..029d1165 --- /dev/null +++ b/scripts/peek_durable_state.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +"""Dev helper: print the durable-execution state (triggers + activity ledger). + +Usage (from repo root, app can be running — WAL allows concurrent reads): + python scripts/peek_durable_state.py # both tables + python scripts/peek_durable_state.py triggers # one table + python scripts/peek_durable_state.py activity +""" + +import sqlite3 +import sys +from pathlib import Path + +DB = Path(__file__).resolve().parent.parent / "app" / "data" / ".usage" / "sessions.db" + + +def show(title: str, query: str) -> None: + print(f"\n=== {title} ===") + try: + with sqlite3.connect(DB) as conn: + conn.row_factory = sqlite3.Row + rows = conn.execute(query).fetchall() + except sqlite3.OperationalError as e: + print(f"({e} — start the app once so the table is created)") + return + if not rows: + print("(empty)") + return + for r in rows: + print(" | ".join(f"{k}={r[k]}" for k in r.keys())) + + +which = sys.argv[1] if len(sys.argv) > 1 else "all" + +if which in ("all", "triggers"): + show( + f"triggers (newest 20) — {DB}", + """ + SELECT id, source, session_id, status, resolution, attempts, + dedup_key, substr(description, 1, 50) AS description + FROM triggers ORDER BY id DESC LIMIT 20 + """, + ) + show( + "triggers by status", + "SELECT status, COUNT(*) AS count FROM triggers GROUP BY status", + ) + +if which in ("all", "activity"): + show( + "activity_log (newest 10)", + """ + SELECT substr(idem_key, 1, 40) AS idem_key, action, status, + provider_ref, updated_at + FROM activity_log ORDER BY updated_at DESC LIMIT 10 + """, + ) diff --git a/tests/test_trigger_lifecycle_polish.py b/tests/test_trigger_lifecycle_polish.py new file mode 100644 index 00000000..b5222ecf --- /dev/null +++ b/tests/test_trigger_lifecycle_polish.py @@ -0,0 +1,187 @@ +# -*- coding: utf-8 -*- +"""Phase 5 tests: retry with backoff, dead-letter surfacing, and GC for the +trigger store + activity ledger.""" + +import asyncio +import sqlite3 +import time +from datetime import datetime, timedelta, timezone + +from agent_core.core.impl.trigger.queue import TriggerQueue + +from app.triggers import TriggerService, TriggerSpec, TriggerSource +from app.triggers.activity_log import ActivityLog, ActivityLogGuard +from app.triggers.service import BACKOFF_BASE_SECONDS, MAX_ATTEMPTS +from app.triggers.store import TriggerStore + + +def run(coro): + return asyncio.run(coro) + + +def make_stack(tmp_path): + store = TriggerStore(db_path=str(tmp_path / "sessions.db")) + queue = TriggerQueue() + service = TriggerService(store, queue) + return store, queue, service + + +def spec(**overrides): + kwargs = dict( + source=TriggerSource.SCHEDULED, + description="do the thing", + priority=50, + session_id="s1", + ) + kwargs.update(overrides) + return TriggerSpec(**kwargs) + + +def age_row(db_path, row_id, hours, table="triggers", key_col="id"): + """Backdate a row's updated_at so GC sees it as old.""" + old = (datetime.now(timezone.utc) - timedelta(hours=hours)).isoformat() + with sqlite3.connect(db_path) as conn: + conn.execute( + f"UPDATE {table} SET updated_at = ? WHERE {key_col} = ?", (old, row_id) + ) + conn.commit() + + +class TestRetryWithBackoff: + def test_nack_requeues_with_backoff(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + result = await service.emit(spec()) + trig = await asyncio.wait_for(service.next(), timeout=2) + before = time.time() + await service.nack(trig, "boom") + + row = store.get(result.trigger_id) + assert row["status"] == "PENDING" + assert row["not_before"] >= before + BACKOFF_BASE_SECONDS - 1 + assert "boom" in row["last_error"] + # re-enqueued with the backoff as its fire time + triggers = await queue.list_triggers() + assert len(triggers) == 1 + assert triggers[0].fire_at >= before + BACKOFF_BASE_SECONDS - 1 + + run(scenario()) + + def test_backoff_grows_per_attempt(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + result = await service.emit(spec()) + delays = [] + for _ in range(3): + # force the queued retry due now so next() returns it + with sqlite3.connect(store._db_path) as conn: + conn.execute( + "UPDATE triggers SET fire_at = ?, not_before = NULL " + "WHERE id = ?", + (time.time() - 1, result.trigger_id), + ) + conn.commit() + await queue.fire("s1") + trig = await asyncio.wait_for(service.next(), timeout=2) + before = time.time() + await service.nack(trig, "boom") + row = store.get(result.trigger_id) + if row["status"] != "PENDING": + break + delays.append(row["not_before"] - before) + assert len(delays) >= 2 + assert delays[1] > delays[0] # exponential growth + + run(scenario()) + + def test_dead_letter_after_max_attempts(self, tmp_path): + store, queue, service = make_stack(tmp_path) + dead = [] + service.set_dead_letter_handler(lambda trig, err: dead.append((trig, err))) + + async def scenario(): + result = await service.emit(spec()) + for attempt in range(MAX_ATTEMPTS + 1): + with sqlite3.connect(store._db_path) as conn: + conn.execute( + "UPDATE triggers SET fire_at = ? WHERE id = ?", + (time.time() - 1, result.trigger_id), + ) + conn.commit() + await queue.fire("s1") + trig = await asyncio.wait_for(service.next(), timeout=2) + await service.nack(trig, f"boom {attempt}") + row = store.get(result.trigger_id) + if row["status"] == "DEAD": + break + + row = store.get(result.trigger_id) + assert row["status"] == "DEAD" + assert row["attempts"] == MAX_ATTEMPTS + assert len(dead) == 1 + assert dead[0][0].id == result.trigger_id + # dead rows do not rehydrate + store2, queue2, service2 = make_stack(tmp_path) + assert await service2.rehydrate() == 0 + + run(scenario()) + + +class TestTriggerStoreGC: + def test_old_settled_rows_removed_active_kept(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + done = await service.emit(spec(session_id="a")) + trig = await asyncio.wait_for(service.next(), timeout=2) + await service.ack(trig) + pending = await service.emit(spec(session_id="b")) + + age_row(store._db_path, done.trigger_id, hours=8 * 24) + age_row(store._db_path, pending.trigger_id, hours=8 * 24) + + removed = store.gc(ttl_hours=7 * 24) + assert removed == 1 + assert store.get(done.trigger_id) is None + assert store.get(pending.trigger_id) is not None # active never GC'd + + run(scenario()) + + def test_recent_settled_rows_survive(self, tmp_path): + store, queue, service = make_stack(tmp_path) + + async def scenario(): + done = await service.emit(spec()) + trig = await asyncio.wait_for(service.next(), timeout=2) + await service.ack(trig) + assert store.gc(ttl_hours=7 * 24) == 0 + assert store.get(done.trigger_id) is not None + + run(scenario()) + + +class TestActivityLogGC: + def test_stale_intent_downgraded_and_old_rows_removed(self, tmp_path): + db = str(tmp_path / "sessions.db") + log = ActivityLog(db_path=db) + guard = ActivityLogGuard(log) + + d_old_intent = guard.begin("send_gmail", {"to": "a@x.com"}, "t1") + d_old_done = guard.begin("send_gmail", {"to": "b@x.com"}, "t1") + guard.complete(d_old_done.idem_key, "success", {"status": "success"}) + d_fresh = guard.begin("send_gmail", {"to": "c@x.com"}, "t1") + + age_row(db, d_old_intent.idem_key, 8 * 24, "activity_log", "idem_key") + age_row(db, d_old_done.idem_key, 31 * 24, "activity_log", "idem_key") + + log.gc(intent_ttl_hours=7 * 24, done_ttl_hours=30 * 24) + + # week-old INTENT no longer blocks: downgraded to FAILED → retake allowed + assert log.get(d_old_intent.idem_key)["status"] == "FAILED" + retry = guard.begin("send_gmail", {"to": "a@x.com"}, "t1") + assert retry.proceed + # month-old DONE removed; fresh INTENT untouched + assert log.get(d_old_done.idem_key) is None + assert log.get(d_fresh.idem_key)["status"] == "INTENT" diff --git a/tests/test_trigger_service.py b/tests/test_trigger_service.py index b635eda9..e413e4a2 100644 --- a/tests/test_trigger_service.py +++ b/tests/test_trigger_service.py @@ -57,7 +57,10 @@ async def scenario(): run(scenario()) - def test_nack_marks_failed(self, tmp_path): + def test_nack_retries_with_backoff(self, tmp_path): + # Phase 5 contract: a nacked trigger is retried (PENDING + backoff), + # not terminally failed — see test_trigger_lifecycle_polish for the + # full retry/dead-letter ladder. store, queue, service = make_stack(tmp_path) async def scenario(): @@ -65,7 +68,8 @@ async def scenario(): trig = await asyncio.wait_for(service.next(), timeout=2) await service.nack(trig, "RuntimeError: kaboom") row = store.get(result.trigger_id) - assert row["status"] == "FAILED" + assert row["status"] == "PENDING" + assert row["not_before"] > time.time() assert "kaboom" in row["last_error"] run(scenario())