Skip to content

Five of eight subscribable notification events never fire #166

Description

@gregoryfoster

Rewritten 2026-08-19 in a backlog staleness pass. The original tracked a parity gap between single-Watch and cascade archive notifications. #191 collapsed the entity, so there is no cascade and no second path — but the investigation found the gap is far wider than archive: five of the eight subscribable notification events never fire at all. Retitled and rescoped. Original body preserved at the bottom.

The defect

WatchEventType has eight members. Only three are ever constructed anywhere in src/:

Event Title in UI Dispatches?
CHANGE_DETECTED Change yessrc/workers/pipeline.py
WATCH_ERROR Error yessrc/workers/fetch_commands.py
WATCH_RECOVERED Recovered yessrc/workers/fetch_commands.py
WATCH_CREATED Created never
WATCH_PAUSED Paused never
WATCH_RESUMED Resumed never
WATCH_ARCHIVED Archived never
WATCH_DELETED Deleted never

There are exactly three dispatch_event_notifications(...) call sites in the entire codebase, and they cover the first three rows.

Why this is worse than a missing feature

All eight events are fully dressed and operator-selectable:

  • EVENT_TITLES (src/core/notifications/events.py) gives all eight human-readable titles and explicitly "drives the Subscribe checkbox order in the notification form".
  • src/dashboard/forms.py exposes ALL_EVENT_TYPE_VALUES = [e.value for e in WatchEventType] — every one is tickable.
  • default_templates.py ships default bodies for the dead ones (e.g. "Watch archived: {{ item_url }}").
  • preview_fixtures.py has fixtures for them, so the preview renders convincingly.

So an operator can subscribe a template to Created / Paused / Resumed / Archived / Deleted, watch the preview produce a plausible message, save it, and never receive anything. The preview actively vouches for a capability that does not exist.

This is the same failure shape as the inert diff toggles #221 removed (fed by canned fixtures, hard-coded empty in production) and as #39's aria-modal with no focus management: a surface asserting something untrue. It should be resolved the same way — make it real or take it off the surface.

The audit trail already exists

Every dead event has a corresponding audit write, so the dispatch hooks are all in places that already run:

Notification event Existing audit site
WATCH_ARCHIVED EventType.WATCHED_ITEM_ARCHIVEDarchive_watched_item, src/api/routes/watched_items.py
WATCH_DELETED EventType.WATCHED_ITEM_DELETED — same module (written before the delete, survives it)
WATCH_PAUSED / WATCH_RESUMED EventType.WATCHED_ITEM_PAUSED / _RESUMEDset_watched_item_active, src/core/watched_items.py, already the single owner of both transitions for API and dashboard (#228)
WATCH_CREATED EventType.WATCHED_ITEM_CREATEDsrc/api/routes/watched_items.py

set_watched_item_active is the cleanest case: it already returns True only when the value actually changed, so it has exactly the no-op suppression a dispatch needs.

Decide per event — this is the work

Not every dead event deserves to become live. Each needs a call:

  1. WATCH_ARCHIVED / WATCH_DELETED — operator-initiated, low-volume, high-consequence. Strongest case for dispatching. Note the original issue's flood concern is void: Collapse WatchedItem ↔ Watch into a single monitored entity #191 means one archive = one item = one notification.
  2. WATCH_PAUSED / WATCH_RESUMED — dispatch from set_watched_item_active. But note a reconciled item's pause state is registry-owned and 409s locally (Consume registry announcements and reconcile watched_items; drop the Archiver SDK #254), so consider whether an announcement-driven is_active flip should notify differently from an operator's click, or at all.
  3. WATCH_CREATED — the weakest case. Since Roll back bare-URL WatchedItems: require an Archiver InfoItem link #251/Consume registry announcements and reconcile watched_items; drop the Archiver SDK #254 the reconcile is the creation path, and POST /api/v1/watched-items currently has no caller (Archiver retired its provisioning call in archiver#158). A "created" notification would fire on registry reconciliation, i.e. on Archiver's schedule, not a human's. Probably remove rather than implement.

Whatever is not implemented must come off the subscribable list — leaving a tickable checkbox that does nothing is the actual bug.

Scope

  • Decide per-event: dispatch, or remove from WatchEventType / EVENT_TITLES / ALL_EVENT_TYPE_VALUES / default_templates / preview_fixtures.
  • Implement dispatch at the existing audit sites for those kept.
  • Remove default templates and preview fixtures for those dropped, so the preview cannot vouch for a dead event.
  • Migration concern: existing notification_templates rows may already subscribe to dropped events in their events array. Decide whether to strip them in a data migration or leave them inert-but-harmless — do not skip this, it is the one part that touches live rows.
  • A guard test asserting every value in ALL_EVENT_TYPE_VALUES has a dispatch site. That is what stops this recurring — the defect arose because nothing tied the subscribable list to the emitting code.

Acceptance

  • Every subscribable event either dispatches or is not subscribable
  • Guard test ties ALL_EVENT_TYPE_VALUES to actual dispatch sites
  • Preview offers only events that can fire
  • Existing template rows referencing removed events are handled deliberately
  • Audit behaviour unchanged (audits are a separate concern from notifications and stay as-is)

Original body (2026-05-18) — premise removed by #191

Context

CR follow-up from #161 (item 7 in the round-1 review of docs/plans/2026-05-17-watched-item-crud-ui-plan.md).

The single-Watch danger-zone archive route at src/dashboard/routes.py:766-782 fires _dispatch_archive_notification for the archived Watch — operators get a notification when a Watch is archived.

The bulk WatchedItem archive (via src/api/routes/watched_items.py:80-118, called from both the API and the dashboard wrapper) cascades is_active=False, is_archived=True onto every child Watch and emits a WATCH_ARCHIVED audit row per child — but does not dispatch notifications. Operators watching for archive events on those Watches won't be notified.

This was a deliberate v1 omission to avoid notification floods on bulk archive (e.g. a WatchedItem with 30 children would fire 30 notifications), but it leaves operators with an inconsistent signal: single-Watch archive → notified; bulk WatchedItem archive → silent.

Scope

Pick a strategy:

  1. Dispatch per-child notifications inline during cascade — accept the flood risk; matches the single-Watch path.
  2. Emit one aggregate notification at the WatchedItem level ("Archived WatchedItem X — cascaded to N child Watches") — quieter, requires a new event type.
  3. Suppress per-child notifications during cascade, document the gap — keep current behavior, add explicit note in operator docs.

Recommend (2) — one event per cascade, with the child count in the payload.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions