Skip to content

Commit ca3ef0d

Browse files
fix(server): allow recovery after status event validation fails (#1365)
## Problem `TaskUpdater.update_status()` sets the terminal flag before constructing the status event. An invalid timestamp, message, or metadata can then raise an exception before anything reaches the queue. For example, after a `COMPLETED` update with an invalid timestamp, calling `failed()` to report the error raises a `RuntimeError` saying the task is already terminal. The updater never published the completion event. ## Fix Construct the complete `TaskStatusUpdateEvent` before setting the terminal flag, so a construction error leaves the updater able to report a failure. The flag is still set under the existing lock, before enqueueing. ## Tests - Add recovery tests for invalid timestamps across all four terminal states and for invalid metadata. - Verify that `DefaultRequestHandlerV2` returns and stores a `FAILED` task after the agent catches a status validation error. Local validation on Python 3.10: - `./scripts/lint.sh` passes (Ruff, formatting, and ty). - `UV_OFFLINE=1 uv run pytest --cov=a2a --cov-report=term --cov-fail-under=88`: **2,312 passed, 183 skipped, 3 xfailed, 1 xpassed**, with **92.95% coverage**. The 183 skips are 182 PostgreSQL/MySQL tests without configured test databases and one SQLite row-lock test. The existing xfail/xpass cases reference #869. The first online runs hit the cross-version fixture's three-second startup timeout while uv downloaded or resolved legacy SDK dependencies; the full run passes with those dependencies cached and offline mode enabled. GitHub CI for the updated commit is pending. --------- Co-authored-by: Mateusz Iwaniuk <45974414+Iwaniukooo11@users.noreply.github.com>
1 parent c70403a commit ca3ef0d

3 files changed

Lines changed: 109 additions & 9 deletions

File tree

‎src/a2a/server/tasks/task_updater.py‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,6 @@ async def update_status(
8484
raise RuntimeError(
8585
f'Task {self.task_id} is already in a terminal state.'
8686
)
87-
if state in self._terminal_states:
88-
self._terminal_state_reached = True
8987

9088
# Create proto timestamp from datetime
9189
ts = Timestamp()
@@ -101,14 +99,15 @@ async def update_status(
10199
status.message.CopyFrom(message)
102100
status.timestamp.CopyFrom(ts)
103101

104-
await self.event_queue.enqueue_event(
105-
TaskStatusUpdateEvent(
106-
task_id=self.task_id,
107-
context_id=self.context_id,
108-
metadata=metadata,
109-
status=status,
110-
)
102+
event = TaskStatusUpdateEvent(
103+
task_id=self.task_id,
104+
context_id=self.context_id,
105+
metadata=metadata,
106+
status=status,
111107
)
108+
if state in self._terminal_states:
109+
self._terminal_state_reached = True
110+
await self.event_queue.enqueue_event(event)
112111

113112
async def add_artifact( # noqa: PLR0913
114113
self,

‎tests/server/request_handlers/test_default_request_handler_v2.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,66 @@ def create_server_call_context() -> ServerCallContext:
129129
return ServerCallContext(user=UnauthenticatedUser())
130130

131131

132+
class _StatusValidationRecoveryAgent(AgentExecutor):
133+
async def execute(
134+
self, context: RequestContext, event_queue: EventQueue
135+
) -> None:
136+
assert context.message is not None
137+
await event_queue.enqueue_event(
138+
new_task_from_user_message(context.message)
139+
)
140+
updater = TaskUpdater(
141+
event_queue,
142+
str(context.task_id or ''),
143+
str(context.context_id or ''),
144+
)
145+
await updater.start_work()
146+
try:
147+
await updater.update_status(
148+
TaskState.TASK_STATE_COMPLETED,
149+
timestamp='not-a-timestamp',
150+
)
151+
except ValueError:
152+
await updater.failed()
153+
154+
async def cancel(
155+
self, context: RequestContext, event_queue: EventQueue
156+
) -> None:
157+
pass
158+
159+
160+
@pytest.mark.asyncio
161+
async def test_status_validation_failure_returns_failed_task() -> None:
162+
store = InMemoryTaskStore()
163+
handler = DefaultRequestHandlerV2(
164+
agent_executor=_StatusValidationRecoveryAgent(),
165+
task_store=store,
166+
agent_card=create_default_agent_card(),
167+
)
168+
context = create_server_call_context()
169+
try:
170+
result = await asyncio.wait_for(
171+
handler.on_message_send(
172+
SendMessageRequest(
173+
message=Message(
174+
role=Role.ROLE_USER,
175+
message_id='validation-recovery',
176+
parts=[Part(text='test status recovery')],
177+
)
178+
),
179+
context,
180+
),
181+
timeout=5,
182+
)
183+
assert isinstance(result, Task)
184+
assert result.status.state == TaskState.TASK_STATE_FAILED
185+
stored = await store.get(result.id, context)
186+
assert stored is not None
187+
assert stored.status.state == TaskState.TASK_STATE_FAILED
188+
finally:
189+
await handler.aclose()
190+
191+
132192
def test_init_default_dependencies():
133193
"""Test that default dependencies are created if not provided."""
134194
agent_executor = MockAgentExecutor()

‎tests/server/tasks/test_task_updater.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,47 @@ async def test_update_status_final(
115115
assert event.status.state == TaskState.TASK_STATE_COMPLETED
116116

117117

118+
@pytest.mark.asyncio
119+
@pytest.mark.parametrize(
120+
'state',
121+
[
122+
TaskState.TASK_STATE_COMPLETED,
123+
TaskState.TASK_STATE_CANCELED,
124+
TaskState.TASK_STATE_FAILED,
125+
TaskState.TASK_STATE_REJECTED,
126+
],
127+
)
128+
async def test_timestamp_validation_failure_allows_terminal_retry(
129+
task_updater: TaskUpdater, event_queue: AsyncMock, state: TaskState
130+
) -> None:
131+
with pytest.raises(ValueError):
132+
await task_updater.update_status(state, timestamp='not-a-timestamp')
133+
134+
event_queue.enqueue_event.assert_not_awaited()
135+
await task_updater.failed()
136+
137+
event_queue.enqueue_event.assert_awaited_once()
138+
event = event_queue.enqueue_event.call_args.args[0]
139+
assert event.status.state == TaskState.TASK_STATE_FAILED
140+
141+
142+
@pytest.mark.asyncio
143+
async def test_metadata_validation_failure_allows_terminal_retry(
144+
task_updater: TaskUpdater, event_queue: AsyncMock
145+
) -> None:
146+
with pytest.raises(ValueError):
147+
await task_updater.update_status(
148+
TaskState.TASK_STATE_COMPLETED, metadata={'bad': object()}
149+
)
150+
151+
event_queue.enqueue_event.assert_not_awaited()
152+
await task_updater.failed()
153+
154+
event_queue.enqueue_event.assert_awaited_once()
155+
event = event_queue.enqueue_event.call_args.args[0]
156+
assert event.status.state == TaskState.TASK_STATE_FAILED
157+
158+
118159
@pytest.mark.asyncio
119160
async def test_add_artifact_with_custom_id_and_name(
120161
task_updater: TaskUpdater, event_queue: AsyncMock, sample_parts: list[Part]

0 commit comments

Comments
 (0)