From 982f7adbd8bf6214250f2c3fbeee623e336614fc Mon Sep 17 00:00:00 2001 From: Connor Moss Date: Thu, 10 Sep 2026 16:42:59 -0400 Subject: [PATCH 1/3] fix(server): return the stored id when creating a push notification config Both create handlers returned the caller's request object rather than what the store persisted. The in-memory store defaults an empty id to the task id on the caller's object, so the id survived; the database store copies first and defaults on its private copy, so the response carried no id and reading the config back with it failed validation. Normalize the id in the handler before set_info so the response matches what was stored on every backend. --- .../default_request_handler.py | 6 +++ .../default_request_handler_v2.py | 6 +++ .../test_default_request_handler.py | 44 +++++++++++++++++++ .../test_default_request_handler_v2.py | 43 ++++++++++++++++++ 4 files changed, 99 insertions(+) diff --git a/src/a2a/server/request_handlers/default_request_handler.py b/src/a2a/server/request_handlers/default_request_handler.py index 384fd5e85..883e8558d 100644 --- a/src/a2a/server/request_handlers/default_request_handler.py +++ b/src/a2a/server/request_handlers/default_request_handler.py @@ -547,6 +547,12 @@ async def on_create_task_push_notification_config( await self._reject_unsafe_push_url(params.url) + # Stores default an empty id to the task id, but only the in-memory + # store does so on the caller's object. Normalize here so the returned + # config carries the id that was persisted, on every store. + if not params.id: + params.id = task_id + await self._push_config_store.set_info( task_id, params, diff --git a/src/a2a/server/request_handlers/default_request_handler_v2.py b/src/a2a/server/request_handlers/default_request_handler_v2.py index 59996236e..5c067b067 100644 --- a/src/a2a/server/request_handlers/default_request_handler_v2.py +++ b/src/a2a/server/request_handlers/default_request_handler_v2.py @@ -385,6 +385,12 @@ async def on_create_task_push_notification_config( # noqa: D102 await self._reject_unsafe_push_url(params.url) + # Stores default an empty id to the task id, but only the in-memory + # store does so on the caller's object. Normalize here so the returned + # config carries the id that was persisted, on every store. + if not params.id: + params.id = task_id + await self._push_config_store.set_info( task_id, params, diff --git a/tests/server/request_handlers/test_default_request_handler.py b/tests/server/request_handlers/test_default_request_handler.py index f9a2a0fb3..b13a6a1e2 100644 --- a/tests/server/request_handlers/test_default_request_handler.py +++ b/tests/server/request_handlers/test_default_request_handler.py @@ -1935,6 +1935,50 @@ async def test_set_task_push_notification_config_task_not_found(agent_card): mock_push_store.set_info.assert_not_awaited() +@pytest.mark.asyncio +@pytest.mark.parametrize('store_kind', ['inmemory', 'database']) +async def test_create_task_push_notification_config_returns_stored_id( + agent_card, store_kind +): + """Test on_create_task_push_notification_config returns the id that was stored.""" + if store_kind == 'database': + from a2a.server.tasks.database_push_notification_config_store import ( + DatabasePushNotificationConfigStore, + ) + from sqlalchemy.ext.asyncio import create_async_engine + + engine = create_async_engine( + 'sqlite+aiosqlite:///file:pushid?mode=memory&cache=shared&uri=true' + ) + push_config_store = DatabasePushNotificationConfigStore(engine=engine) + else: + push_config_store = InMemoryPushNotificationConfigStore() + + task = create_sample_task() + task_store = InMemoryTaskStore() + context = create_server_call_context() + await task_store.save(task, context) + + request_handler = DefaultRequestHandler( + agent_executor=MockAgentExecutor(), + task_store=task_store, + push_config_store=push_config_store, + agent_card=agent_card, + ) + params = TaskPushNotificationConfig( + task_id=task.id, + url='http://example.com', + ) + + response = await request_handler.on_create_task_push_notification_config( + params, context + ) + + stored = await push_config_store.get_info(task.id, context) + assert response.id == task.id + assert [config.id for config in stored] == [response.id] + + @pytest.mark.asyncio async def test_get_task_push_notification_config_no_store(agent_card): """Test on_get_task_push_notification_config when _push_config_store is None.""" diff --git a/tests/server/request_handlers/test_default_request_handler_v2.py b/tests/server/request_handlers/test_default_request_handler_v2.py index 2eb7e4725..08c91d424 100644 --- a/tests/server/request_handlers/test_default_request_handler_v2.py +++ b/tests/server/request_handlers/test_default_request_handler_v2.py @@ -511,6 +511,49 @@ async def test_set_task_push_notification_config_task_not_found(): mock_push_store.set_info.assert_not_awaited() +@pytest.mark.asyncio +@pytest.mark.parametrize('store_kind', ['inmemory', 'database']) +async def test_create_task_push_notification_config_returns_stored_id( + store_kind, +): + """Test on_create_task_push_notification_config returns the id that was stored.""" + if store_kind == 'database': + from a2a.server.tasks.database_push_notification_config_store import ( + DatabasePushNotificationConfigStore, + ) + from sqlalchemy.ext.asyncio import create_async_engine + + engine = create_async_engine( + 'sqlite+aiosqlite:///file:pushidv2?mode=memory&cache=shared&uri=true' + ) + push_config_store = DatabasePushNotificationConfigStore(engine=engine) + else: + push_config_store = InMemoryPushNotificationConfigStore() + + task = create_sample_task() + task_store = InMemoryTaskStore() + context = create_server_call_context() + await task_store.save(task, context) + + request_handler = DefaultRequestHandlerV2( + agent_executor=MockAgentExecutor(), + task_store=task_store, + push_config_store=push_config_store, + agent_card=create_default_agent_card(), + ) + params = TaskPushNotificationConfig( + task_id=task.id, url='http://example.com' + ) + + response = await request_handler.on_create_task_push_notification_config( + params, context + ) + + stored = await push_config_store.get_info(task.id, context) + assert response.id == task.id + assert [config.id for config in stored] == [response.id] + + @pytest.mark.asyncio async def test_get_task_push_notification_config_no_store(): """Test on_get_task_push_notification_config when _push_config_store is None.""" From d7da2479ddf516aab4c082d98fbcf7f1b2598c8b Mon Sep 17 00:00:00 2001 From: Connor Moss Date: Thu, 1 Oct 2026 11:10:54 -0400 Subject: [PATCH 2/3] refactor(server): normalize push configs in set_info and return the stored config --- .../default_request_handler.py | 10 +--- .../default_request_handler_v2.py | 10 +--- ...database_push_notification_config_store.py | 4 +- ...inmemory_push_notification_config_store.py | 21 +++++--- .../tasks/push_notification_config_store.py | 9 +++- .../test_default_request_handler.py | 5 +- .../test_default_request_handler_v2.py | 39 ++++++++++++++- ...database_push_notification_config_store.py | 49 +++++++++++++++---- .../tasks/test_inmemory_push_notifications.py | 38 ++++++++++++-- 9 files changed, 141 insertions(+), 44 deletions(-) diff --git a/src/a2a/server/request_handlers/default_request_handler.py b/src/a2a/server/request_handlers/default_request_handler.py index df6590964..1110110d4 100644 --- a/src/a2a/server/request_handlers/default_request_handler.py +++ b/src/a2a/server/request_handlers/default_request_handler.py @@ -560,20 +560,12 @@ async def on_create_task_push_notification_config( await self._reject_unsafe_push_url(params.url) - # Stores default an empty id to the task id, but only the in-memory - # store does so on the caller's object. Normalize here so the returned - # config carries the id that was persisted, on every store. - if not params.id: - params.id = task_id - - await self._push_config_store.set_info( + return await self._push_config_store.set_info( task_id, params, context, ) - return params - @validate_request_params @validate( lambda self: self._agent_card.capabilities.push_notifications, diff --git a/src/a2a/server/request_handlers/default_request_handler_v2.py b/src/a2a/server/request_handlers/default_request_handler_v2.py index 1a967fcd9..4f35d05d2 100644 --- a/src/a2a/server/request_handlers/default_request_handler_v2.py +++ b/src/a2a/server/request_handlers/default_request_handler_v2.py @@ -497,20 +497,12 @@ async def on_create_task_push_notification_config( # noqa: D102 await self._reject_unsafe_push_url(params.url) - # Stores default an empty id to the task id, but only the in-memory - # store does so on the caller's object. Normalize here so the returned - # config carries the id that was persisted, on every store. - if not params.id: - params.id = task_id - - await self._push_config_store.set_info( + return await self._push_config_store.set_info( task_id, params, context, ) - return params - @validate_request_params @validate( lambda self: self._agent_card.capabilities.push_notifications, diff --git a/src/a2a/server/tasks/database_push_notification_config_store.py b/src/a2a/server/tasks/database_push_notification_config_store.py index 697b75542..521455f95 100644 --- a/src/a2a/server/tasks/database_push_notification_config_store.py +++ b/src/a2a/server/tasks/database_push_notification_config_store.py @@ -283,7 +283,7 @@ async def set_info( task_id: str, notification_config: TaskPushNotificationConfig, context: ServerCallContext, - ) -> None: + ) -> TaskPushNotificationConfig: """Sets or updates the push notification configuration for a task.""" await self._ensure_initialized() owner = self.owner_resolver(context) @@ -291,6 +291,7 @@ async def set_info( # Create a copy of the config using proto CopyFrom config_to_save = TaskPushNotificationConfig() config_to_save.CopyFrom(notification_config) + config_to_save.task_id = task_id if not config_to_save.id: config_to_save.id = task_id @@ -303,6 +304,7 @@ async def set_info( config_to_save.id, owner, ) + return config_to_save async def _select_configs( self, diff --git a/src/a2a/server/tasks/inmemory_push_notification_config_store.py b/src/a2a/server/tasks/inmemory_push_notification_config_store.py index f8b0b151b..93e775edb 100644 --- a/src/a2a/server/tasks/inmemory_push_notification_config_store.py +++ b/src/a2a/server/tasks/inmemory_push_notification_config_store.py @@ -40,31 +40,38 @@ async def set_info( task_id: str, notification_config: TaskPushNotificationConfig, context: ServerCallContext, - ) -> None: + ) -> TaskPushNotificationConfig: """Sets or updates the push notification configuration for a task in memory.""" owner = self.owner_resolver(context) + stored = TaskPushNotificationConfig() + stored.CopyFrom(notification_config) + stored.task_id = task_id + if not stored.id: + stored.id = task_id + with self.lock: owner_infos = self._push_notification_infos.setdefault(owner, {}) if task_id not in owner_infos: owner_infos[task_id] = [] - if not notification_config.id: - notification_config.id = task_id - # Remove existing config with the same ID for config in owner_infos[task_id]: - if config.id == notification_config.id: + if config.id == stored.id: owner_infos[task_id].remove(config) break - owner_infos[task_id].append(notification_config) + owner_infos[task_id].append(stored) logger.debug( 'Push notification config for task %s with config id %s for owner %s saved/updated.', task_id, - notification_config.id, + stored.id, owner, ) + result = TaskPushNotificationConfig() + result.CopyFrom(stored) + return result + async def get_info( self, task_id: str, diff --git a/src/a2a/server/tasks/push_notification_config_store.py b/src/a2a/server/tasks/push_notification_config_store.py index e1e65c3fb..62091e98d 100644 --- a/src/a2a/server/tasks/push_notification_config_store.py +++ b/src/a2a/server/tasks/push_notification_config_store.py @@ -18,8 +18,13 @@ async def set_info( task_id: str, notification_config: TaskPushNotificationConfig, context: ServerCallContext, - ) -> None: - """Sets or updates the push notification configuration for a task.""" + ) -> TaskPushNotificationConfig: + """Sets or updates the push notification configuration for a task. + + Implementations MUST NOT mutate notification_config. They store a + copy with task_id set to the given task and an empty id defaulted to + the task id, and return that stored configuration. + """ @abstractmethod async def get_info( diff --git a/tests/server/request_handlers/test_default_request_handler.py b/tests/server/request_handlers/test_default_request_handler.py index 0920b0069..1e82fcb63 100644 --- a/tests/server/request_handlers/test_default_request_handler.py +++ b/tests/server/request_handlers/test_default_request_handler.py @@ -1984,6 +1984,8 @@ async def test_create_task_push_notification_config_returns_stored_id( ): """Test on_create_task_push_notification_config returns the id that was stored.""" if store_kind == 'database': + pytest.importorskip('sqlalchemy') + pytest.importorskip('aiosqlite') from a2a.server.tasks.database_push_notification_config_store import ( DatabasePushNotificationConfigStore, ) @@ -2018,7 +2020,8 @@ async def test_create_task_push_notification_config_returns_stored_id( stored = await push_config_store.get_info(task.id, context) assert response.id == task.id - assert [config.id for config in stored] == [response.id] + assert list(stored) == [response] + assert params.id == '', 'the request object must not be mutated' @pytest.mark.asyncio diff --git a/tests/server/request_handlers/test_default_request_handler_v2.py b/tests/server/request_handlers/test_default_request_handler_v2.py index ed1f5555c..c1dad2b6e 100644 --- a/tests/server/request_handlers/test_default_request_handler_v2.py +++ b/tests/server/request_handlers/test_default_request_handler_v2.py @@ -520,6 +520,8 @@ async def test_create_task_push_notification_config_returns_stored_id( ): """Test on_create_task_push_notification_config returns the id that was stored.""" if store_kind == 'database': + pytest.importorskip('sqlalchemy') + pytest.importorskip('aiosqlite') from a2a.server.tasks.database_push_notification_config_store import ( DatabasePushNotificationConfigStore, ) @@ -553,7 +555,8 @@ async def test_create_task_push_notification_config_returns_stored_id( stored = await push_config_store.get_info(task.id, context) assert response.id == task.id - assert [config.id for config in stored] == [response.id] + assert list(stored) == [response] + assert params.id == '', 'the request object must not be mutated' @pytest.mark.asyncio @@ -1753,6 +1756,40 @@ async def test_on_message_send_with_push_notification(): ) +@pytest.mark.asyncio +async def test_on_message_send_stores_inline_push_config_under_its_task(): + task_store = InMemoryTaskStore() + push_store = InMemoryPushNotificationConfigStore() + + request_handler = DefaultRequestHandlerV2( + agent_executor=HelloAgentExecutor(), + task_store=task_store, + push_config_store=push_store, + agent_card=create_default_agent_card(), + ) + # SendMessageConfiguration carries neither task_id nor id for the config. + params = SendMessageRequest( + message=Message( + role=Role.ROLE_USER, + message_id='msg_push_inline', + parts=[Part(text='Hi')], + ), + configuration=SendMessageConfiguration( + task_push_notification_config=TaskPushNotificationConfig( + url='http://example.com/webhook' + ) + ), + ) + + context = create_server_call_context() + result = await request_handler.on_message_send(params, context) + + stored = await push_store.get_info(result.id, context) + assert [(config.task_id, config.id) for config in stored] == [ + (result.id, result.id) + ] + + @pytest.mark.asyncio async def test_on_message_send_with_empty_push_notification_config_does_not_call_set_info(): task_store = InMemoryTaskStore() diff --git a/tests/server/tasks/test_database_push_notification_config_store.py b/tests/server/tasks/test_database_push_notification_config_store.py index 72cf39c99..5ae9aa012 100644 --- a/tests/server/tasks/test_database_push_notification_config_store.py +++ b/tests/server/tasks/test_database_push_notification_config_store.py @@ -204,7 +204,9 @@ async def test_set_and_get_info_single_config( ): """Test setting and retrieving a single configuration.""" task_id = 'task-1' - config = TaskPushNotificationConfig(id='config-1', url='http://example.com') + config = TaskPushNotificationConfig( + task_id=task_id, id='config-1', url='http://example.com' + ) await db_store_parameterized.set_info(task_id, config, MINIMAL_CALL_CONTEXT) retrieved_configs = await db_store_parameterized.get_info( @@ -215,6 +217,26 @@ async def test_set_and_get_info_single_config( assert retrieved_configs[0] == config +@pytest.mark.asyncio +async def test_set_info_returns_the_stored_config_without_mutating_input( + db_store_parameterized: DatabasePushNotificationConfigStore, +): + """set_info returns what it stored and leaves the caller's config alone.""" + task_id = 'task-normalize' + config = TaskPushNotificationConfig(url='http://example.com') + + stored = await db_store_parameterized.set_info( + task_id, config, MINIMAL_CALL_CONTEXT + ) + + assert (stored.task_id, stored.id) == (task_id, task_id) + assert (config.task_id, config.id) == ('', '') + retrieved_configs = await db_store_parameterized.get_info( + task_id, MINIMAL_CALL_CONTEXT + ) + assert retrieved_configs == [stored] + + @pytest.mark.asyncio async def test_set_and_get_info_multiple_configs( db_store_parameterized: DatabasePushNotificationConfigStore, @@ -306,8 +328,12 @@ async def test_delete_info_specific_config( ): """Test deleting a single, specific configuration.""" task_id = 'task-1' - config1 = TaskPushNotificationConfig(id='config-1', url='http://a.com') - config2 = TaskPushNotificationConfig(id='config-2', url='http://b.com') + config1 = TaskPushNotificationConfig( + task_id=task_id, id='config-1', url='http://a.com' + ) + config2 = TaskPushNotificationConfig( + task_id=task_id, id='config-2', url='http://b.com' + ) await db_store_parameterized.set_info( task_id, config1, MINIMAL_CALL_CONTEXT @@ -372,7 +398,10 @@ async def test_data_is_encrypted_in_db( """Verify that the data stored in the database is actually encrypted.""" task_id = 'encrypted-task' config = TaskPushNotificationConfig( - id='config-1', url='http://secret.url', token='secret-token' + task_id=task_id, + id='config-1', + url='http://secret.url', + token='secret-token', ) plain_json = MessageToJson(config) @@ -481,7 +510,7 @@ async def test_custom_table_name( task_id = 'custom-table-task' config = TaskPushNotificationConfig( - id='config-1', url='http://custom.url' + task_id=task_id, id='config-1', url='http://custom.url' ) # This will create the table on first use @@ -530,10 +559,10 @@ async def test_set_and_get_info_multiple_configs_no_key( task_id = 'task-1' config1 = TaskPushNotificationConfig( - id='config-1', url='http://example.com/1' + task_id=task_id, id='config-1', url='http://example.com/1' ) config2 = TaskPushNotificationConfig( - id='config-2', url='http://example.com/2' + task_id=task_id, id='config-2', url='http://example.com/2' ) await store.set_info(task_id, config1, MINIMAL_CALL_CONTEXT) @@ -560,7 +589,7 @@ async def test_data_is_not_encrypted_in_db_if_no_key_is_set( task_id = 'task-1' config = TaskPushNotificationConfig( - id='config-1', url='http://example.com/1' + task_id=task_id, id='config-1', url='http://example.com/1' ) plain_json = MessageToJson(config) @@ -594,7 +623,9 @@ async def test_decryption_fallback_for_unencrypted_data( await unencrypted_store.initialize() task_id = 'mixed-encryption-task' - config = TaskPushNotificationConfig(id='config-1', url='http://plain.url') + config = TaskPushNotificationConfig( + task_id=task_id, id='config-1', url='http://plain.url' + ) await unencrypted_store.set_info(task_id, config, MINIMAL_CALL_CONTEXT) # 2. Try to read with the encryption-enabled store from the fixture diff --git a/tests/server/tasks/test_inmemory_push_notifications.py b/tests/server/tasks/test_inmemory_push_notifications.py index 0a53352f8..8acae7d9d 100644 --- a/tests/server/tasks/test_inmemory_push_notifications.py +++ b/tests/server/tasks/test_inmemory_push_notifications.py @@ -44,8 +44,11 @@ def _create_sample_push_config( url: str = 'http://example.com/callback', config_id: str = 'cfg1', token: str | None = None, + task_id: str = '', ) -> TaskPushNotificationConfig: - return TaskPushNotificationConfig(id=config_id, url=url, token=token) + return TaskPushNotificationConfig( + id=config_id, url=url, token=token, task_id=task_id + ) class SampleUser(User): @@ -103,7 +106,9 @@ def test_constructor_stores_client(self) -> None: async def test_set_info_adds_new_config(self) -> None: task_id = 'task_new' - config = _create_sample_push_config(url='http://new.url/callback') + config = _create_sample_push_config( + task_id=task_id, url='http://new.url/callback' + ) await self.config_store.set_info(task_id, config, MINIMAL_CALL_CONTEXT) @@ -112,17 +117,38 @@ async def test_set_info_adds_new_config(self) -> None: ) self.assertEqual(retrieved, [config]) + async def test_set_info_returns_the_stored_config_without_mutating_input( + self, + ) -> None: + task_id = 'task_normalize' + config = TaskPushNotificationConfig(url='http://normalize.url/callback') + + stored = await self.config_store.set_info( + task_id, config, MINIMAL_CALL_CONTEXT + ) + + assert (stored.task_id, stored.id) == (task_id, task_id) + assert (config.task_id, config.id) == ('', '') + retrieved = await self.config_store.get_info( + task_id, MINIMAL_CALL_CONTEXT + ) + self.assertEqual(retrieved, [stored]) + async def test_set_info_appends_to_existing_config(self) -> None: task_id = 'task_update' initial_config = _create_sample_push_config( - url='http://initial.url/callback', config_id='cfg_initial' + task_id=task_id, + url='http://initial.url/callback', + config_id='cfg_initial', ) await self.config_store.set_info( task_id, initial_config, MINIMAL_CALL_CONTEXT ) updated_config = _create_sample_push_config( - url='http://updated.url/callback', config_id='cfg_updated' + task_id=task_id, + url='http://updated.url/callback', + config_id='cfg_updated', ) await self.config_store.set_info( task_id, updated_config, MINIMAL_CALL_CONTEXT @@ -164,7 +190,9 @@ async def test_set_info_without_config_id(self) -> None: async def test_get_info_existing_config(self) -> None: task_id = 'task_get_exist' - config = _create_sample_push_config(url='http://get.this/callback') + config = _create_sample_push_config( + task_id=task_id, url='http://get.this/callback' + ) await self.config_store.set_info(task_id, config, MINIMAL_CALL_CONTEXT) retrieved_config = await self.config_store.get_info( From f99fe21ba0169788b4331a0d570f1ba35908c76f Mon Sep 17 00:00:00 2001 From: Connor Moss Date: Thu, 1 Oct 2026 11:17:08 -0400 Subject: [PATCH 3/3] test(server): dispose the sqlite engine in the create-config tests --- tests/server/request_handlers/test_default_request_handler.py | 3 +++ .../server/request_handlers/test_default_request_handler_v2.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tests/server/request_handlers/test_default_request_handler.py b/tests/server/request_handlers/test_default_request_handler.py index 1e82fcb63..7962ea5ea 100644 --- a/tests/server/request_handlers/test_default_request_handler.py +++ b/tests/server/request_handlers/test_default_request_handler.py @@ -2019,6 +2019,9 @@ async def test_create_task_push_notification_config_returns_stored_id( ) stored = await push_config_store.get_info(task.id, context) + if store_kind == 'database': + await engine.dispose() + assert response.id == task.id assert list(stored) == [response] assert params.id == '', 'the request object must not be mutated' diff --git a/tests/server/request_handlers/test_default_request_handler_v2.py b/tests/server/request_handlers/test_default_request_handler_v2.py index c1dad2b6e..3fc70cdfd 100644 --- a/tests/server/request_handlers/test_default_request_handler_v2.py +++ b/tests/server/request_handlers/test_default_request_handler_v2.py @@ -554,6 +554,9 @@ async def test_create_task_push_notification_config_returns_stored_id( ) stored = await push_config_store.get_info(task.id, context) + if store_kind == 'database': + await engine.dispose() + assert response.id == task.id assert list(stored) == [response] assert params.id == '', 'the request object must not be mutated'