Skip to content

fix: make get_info_for_dispatch abstract to prevent silent notification drops - #1180

Open
ez-lbz wants to merge 2 commits into
a2aproject:mainfrom
ez-lbz:fix/push-dispatch-default-impl
Open

fix: make get_info_for_dispatch abstract to prevent silent notification drops#1180
ez-lbz wants to merge 2 commits into
a2aproject:mainfrom
ez-lbz:fix/push-dispatch-default-impl

Conversation

@ez-lbz

@ez-lbz ez-lbz commented Aug 10, 2026

Copy link
Copy Markdown

What changed

1. Make get_info_for_dispatch abstract on PushNotificationConfigStore

Problem: PushNotificationConfigStore.get_info_for_dispatch() (src/a2a/server/tasks/push_notification_config_store.py) was not @abstractmethod. Its default implementation fell back to get_info(task_id, ServerCallContext()) with a synthetic empty context, which resolves to the empty-string owner partition and returns no configs — silently dropping every push notification in any deployment with multiple owners. A subclass that forgot to override the method only logged a warning per call, so the failure was easy to miss in production.

Fix (src/a2a/server/tasks/push_notification_config_store.py):

  • get_info_for_dispatch() is now @abstractmethod; the silent owner-scoped fallback is removed. Any store implementation that does not provide the cross-owner dispatch read path now fails loudly at instantiation time instead of dropping notifications at runtime.
  • Removed the now-unused module-level logger.
  • Verified both built-in stores already implement the method: InMemoryPushNotificationConfigStore (inmemory_push_notification_config_store.py) and DatabasePushNotificationConfigStore (database_push_notification_config_store.py).

Testing

  • ./.venv/Scripts/python -m pytest tests/server/tasks/test_inmemory_push_notifications.py -q20 passed (includes 2 new tests: an incomplete subclass without get_info_for_dispatch cannot be instantiated; both built-in stores expose a callable dispatch read path).
  • ./.venv/Scripts/python -m pytest tests/server/tasks/test_database_push_notification_config_store.py -q23 passed, 44 skipped (skips are DB-DSN-dependent tests, pre-existing).
  • ./.venv/Scripts/python -m ruff check on modified files: clean.
  • Behavior change: third-party subclasses of PushNotificationConfigStore that do not implement get_info_for_dispatch will now raise TypeError at instantiation instead of silently dropping notifications. This is the intended loud-failure hardening.

@ez-lbz
ez-lbz requested a review from a team as a code owner August 10, 2026 16:09
@github-actions

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/events/event_queue_v2.py 91.79% 91.28% 🔴 -0.51%
src/a2a/server/tasks/push_notification_config_store.py 60.00% 100.00% 🟢 +40.00%
src/a2a/utils/telemetry.py 91.47% 90.70% 🔴 -0.78%
Total 93.00% 92.99% ⚪️ -0.00%

Generated by coverage-comment.yml

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant