From 4482f12d35632726a50032bb81f0cf2c8b692431 Mon Sep 17 00:00:00 2001 From: Jerome Romualdez Date: Wed, 1 Jul 2026 14:38:49 -0400 Subject: [PATCH 1/2] fix(schedules): grant legacy auth for run schedules Co-authored-by: Cursor --- .../services/agent_run_schedule_service.py | 37 +++++++++++++- .../test_agent_run_schedule_service.py | 50 +++++++++++++++++-- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/agentex/src/domain/services/agent_run_schedule_service.py b/agentex/src/domain/services/agent_run_schedule_service.py index 4c7f2583..92dd331c 100644 --- a/agentex/src/domain/services/agent_run_schedule_service.py +++ b/agentex/src/domain/services/agent_run_schedule_service.py @@ -502,16 +502,49 @@ async def _register_schedule_in_auth( extra={"authz_selector": authz_selector, "agent_id": agent_id}, ) return False + schedule_resource = AgentexResource.schedule(authz_selector) await self.authorization_service.register_resource( - resource=AgentexResource.schedule(authz_selector), + resource=schedule_resource, parent=AgentexResource.agent(agent_id), ) + try: + # Legacy SGP auth treats register_resource as a no-op. Keep the + # Spark registration above for the future path, and write the legacy + # grant so current list/check calls can see the schedule. + await self.authorization_service.grant(schedule_resource) + except Exception: + try: + await self.authorization_service.deregister_resource( + resource=schedule_resource, + ) + except Exception as cleanup_exc: + logger.warning( + "Auth deregister failed after run schedule grant failure", + extra={ + "authz_selector": authz_selector, + "error_type": type(cleanup_exc).__name__, + }, + exc_info=True, + ) + raise return True async def _deregister_schedule_from_auth(self, *, authz_selector: str) -> None: + schedule_resource = AgentexResource.schedule(authz_selector) + try: + await self.authorization_service.revoke(resource=schedule_resource) + except Exception as exc: + logger.warning( + "Auth revoke failed for run schedule; entry may be orphaned", + extra={ + "authz_selector": authz_selector, + "error_type": type(exc).__name__, + }, + exc_info=True, + ) try: await self.authorization_service.deregister_resource( - resource=AgentexResource.schedule(authz_selector), + resource=schedule_resource ) except Exception as exc: logger.warning( diff --git a/agentex/tests/unit/services/test_agent_run_schedule_service.py b/agentex/tests/unit/services/test_agent_run_schedule_service.py index b29821e2..1291bdb7 100644 --- a/agentex/tests/unit/services/test_agent_run_schedule_service.py +++ b/agentex/tests/unit/services/test_agent_run_schedule_service.py @@ -11,6 +11,7 @@ ScheduleInitialInput, UpdateAgentRunScheduleRequest, ) +from src.api.schemas.authorization_types import AgentexResource from src.domain.entities.agent_run_schedules import AgentRunScheduleEntity from src.domain.entities.agents import ACPType, AgentEntity, AgentStatus from src.domain.exceptions import ClientError @@ -112,8 +113,16 @@ async def test_create_persists_and_schedules(self, service, agent): ) assert create_kwargs["time_zone_name"] == "America/New_York" - # Ownership registered before the Temporal write. - service.authorization_service.register_resource.assert_called_once() + # Ownership is written to both auth paths before the Temporal write: + # Spark resource registration for the future path, and a legacy grant + # for current SGP list/check behavior. + authz_selector = build_run_schedule_authz_selector(agent.id, persisted.name) + schedule_resource = AgentexResource.schedule(authz_selector) + service.authorization_service.register_resource.assert_called_once_with( + resource=schedule_resource, + parent=AgentexResource.agent(agent.id), + ) + service.authorization_service.grant.assert_called_once_with(schedule_resource) assert response.name == "daily-summary" assert response.initial_input_method == "event/send" # async agent @@ -141,8 +150,9 @@ async def test_create_rolls_back_row_on_temporal_failure(self, service, agent): with pytest.raises(RuntimeError): await service.create_schedule(agent, request, {"user_id": "u1"}) - # The orphaned row and auth entry are compensated. + # The orphaned row and both auth entries are compensated. service.schedule_repository.delete.assert_called_once_with(id=persisted.id) + service.authorization_service.revoke.assert_called_once() service.authorization_service.deregister_resource.assert_called_once() async def test_create_rolls_back_row_on_auth_registration_failure( @@ -163,6 +173,39 @@ async def test_create_rolls_back_row_on_auth_registration_failure( # must not create a Temporal schedule. service.schedule_repository.delete.assert_called_once_with(id=persisted.id) service.temporal_adapter.create_schedule.assert_not_called() + service.authorization_service.grant.assert_not_called() + + async def test_create_compensates_register_when_grant_fails(self, service, agent): + request = _request() + persisted = _persisted(agent.id, request) + service.schedule_repository.get_by_agent_id_and_name.return_value = None + service.schedule_repository.create.return_value = persisted + service.authorization_service.grant.side_effect = RuntimeError("authz down") + + with pytest.raises(RuntimeError): + await service.create_schedule(agent, request, {"user_id": "u1"}) + + # The Spark registration is compensated, the row is removed, and the + # Temporal clock is never created if the legacy grant cannot be written. + service.authorization_service.register_resource.assert_called_once() + service.authorization_service.deregister_resource.assert_called_once() + service.authorization_service.revoke.assert_not_called() + service.schedule_repository.delete.assert_called_once_with(id=persisted.id) + service.temporal_adapter.create_schedule.assert_not_called() + + async def test_create_skips_auth_when_no_creator(self, service, agent): + type(service.authorization_service).principal_context = PropertyMock( + return_value={} + ) + request = _request() + persisted = _persisted(agent.id, request) + service.schedule_repository.get_by_agent_id_and_name.return_value = None + service.schedule_repository.create.return_value = persisted + + await service.create_schedule(agent, request, {}) + + service.authorization_service.register_resource.assert_not_called() + service.authorization_service.grant.assert_not_called() @pytest.mark.unit @@ -255,6 +298,7 @@ async def test_delete_tolerates_missing_temporal_schedule(self, service, agent): service.schedule_repository.update.assert_called_once() tombstoned = service.schedule_repository.update.call_args.args[0] assert tombstoned.deleted_at is not None + service.authorization_service.revoke.assert_called_once() service.authorization_service.deregister_resource.assert_called_once() From 7202557b8563977d3eb6ffea62d1ca9409ff6378 Mon Sep 17 00:00:00 2001 From: Jerome Romualdez Date: Wed, 1 Jul 2026 16:05:46 -0400 Subject: [PATCH 2/2] test(schedules): tighten legacy auth cleanup coverage Co-authored-by: Cursor --- .../domain/services/agent_run_schedule_service.py | 10 +++++++++- .../unit/services/test_agent_run_schedule_service.py | 12 ++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/agentex/src/domain/services/agent_run_schedule_service.py b/agentex/src/domain/services/agent_run_schedule_service.py index 92dd331c..0629057a 100644 --- a/agentex/src/domain/services/agent_run_schedule_service.py +++ b/agentex/src/domain/services/agent_run_schedule_service.py @@ -512,7 +512,15 @@ async def _register_schedule_in_auth( # Spark registration above for the future path, and write the legacy # grant so current list/check calls can see the schedule. await self.authorization_service.grant(schedule_resource) - except Exception: + except Exception as grant_exc: + logger.warning( + "Auth grant failed for run schedule; compensating with deregister", + extra={ + "authz_selector": authz_selector, + "error_type": type(grant_exc).__name__, + }, + exc_info=True, + ) try: await self.authorization_service.deregister_resource( resource=schedule_resource, diff --git a/agentex/tests/unit/services/test_agent_run_schedule_service.py b/agentex/tests/unit/services/test_agent_run_schedule_service.py index 1291bdb7..27f0ddbe 100644 --- a/agentex/tests/unit/services/test_agent_run_schedule_service.py +++ b/agentex/tests/unit/services/test_agent_run_schedule_service.py @@ -151,8 +151,12 @@ async def test_create_rolls_back_row_on_temporal_failure(self, service, agent): await service.create_schedule(agent, request, {"user_id": "u1"}) # The orphaned row and both auth entries are compensated. + authz_selector = build_run_schedule_authz_selector(agent.id, persisted.name) + schedule_resource = AgentexResource.schedule(authz_selector) service.schedule_repository.delete.assert_called_once_with(id=persisted.id) - service.authorization_service.revoke.assert_called_once() + service.authorization_service.revoke.assert_called_once_with( + resource=schedule_resource + ) service.authorization_service.deregister_resource.assert_called_once() async def test_create_rolls_back_row_on_auth_registration_failure( @@ -298,7 +302,11 @@ async def test_delete_tolerates_missing_temporal_schedule(self, service, agent): service.schedule_repository.update.assert_called_once() tombstoned = service.schedule_repository.update.call_args.args[0] assert tombstoned.deleted_at is not None - service.authorization_service.revoke.assert_called_once() + authz_selector = build_run_schedule_authz_selector(agent.id, row.name) + schedule_resource = AgentexResource.schedule(authz_selector) + service.authorization_service.revoke.assert_called_once_with( + resource=schedule_resource + ) service.authorization_service.deregister_resource.assert_called_once()