[Bug]: CancelTask racing the task's own completion overwrites COMPLETED with FAILED
Mantenedores costumam responder em até 2 dias
Avaliação
Esta issue ainda não foi avaliada.
Descrição
What happened?
A task can finish on its own while a CancelTask request for it is in flight. When it does, DefaultRequestHandlerV2 (via ActiveTask.cancel) can overwrite the task's terminal COMPLETED state with FAILED. Reproduced on 1.2.1; the same code is on main (43d1eff) and in 1.3.0.
The window is real. ActiveTask.cancel only checks _is_finished, which the consumer sets once it has processed the terminal event. A complete() that the executor has already enqueued, but the consumer has not yet handled, passes that check. So AgentExecutor.cancel() runs against a task that has already finished.
A completed task is overwritten with FAILED
If executor.cancel() raises, ActiveTask.cancel calls _mark_task_as_failed. That method enqueues TASK_STATE_FAILED without checking whether the task is already terminal. Its sibling _write_failed_status does check.
An executor that shares one TaskUpdater between execute and cancel hits this naturally: when cancel() loses to complete(), TaskUpdater.update_status raises RuntimeError("... already in a terminal state"). The connected client has already received COMPLETED and the result over the stream, yet GetTask afterwards reports FAILED.
The spec defines TASK_STATE_COMPLETED as "a terminal state" (§4.1.3 TaskState; a2a.proto: "Indicates that a task has finished successfully. This is a terminal state."). A streaming client is told it is final, because the stream "MUST close when the task reaches a terminal state" (§3.1.2). After that, the stored state should not change, and in particular should not contradict what was streamed.
Expected: a terminal state is never replaced; a failing cancel() on an already-terminal task leaves it COMPLETED.
Related question: what should CancelTask answer in this race?
Even when cancel() does not raise, ActiveTask.cancel re-reads the task and returns it in whatever terminal state it reached. So CancelTask responds successfully with a TASK_STATE_COMPLETED task. The spec doesn't settle this case clearly. §3.1.5 Cancel Task says:
- "success is not guaranteed (e.g., the task might have already completed or failed…)" and "returns its updated state", which supports the current behaviour;
- Outputs: "Updated
Taskwith cancellation status", and the errorTaskNotCancelableError: "The task is not in a cancelable state (e.g., already completed, failed, or canceled)", which supports an error.
The SDK itself answers both ways. The cross-replica path, _cancel_remote, raises TaskNotCancelableError when the reloaded task is not CANCELED; the local path returns the task. Is that difference intended? If not, it would help to pick one.
Repro (self-contained, a2a-sdk 1.2.1, no transport). The executor's cancel() lets the in-flight work finish and complete the task, which deterministically puts the cancel in the window above:
import asyncio
from a2a.server.agent_execution import AgentExecutor, RequestContext
from a2a.server.context import ServerCallContext
from a2a.server.events import EventQueue
from a2a.server.request_handlers import DefaultRequestHandlerV2
from a2a.server.tasks import InMemoryTaskStore, TaskUpdater
from a2a.types import (
AgentCapabilities,
AgentCard,
CancelTaskRequest,
GetTaskRequest,
Message,
Part,
Role,
SendMessageRequest,
Task,
TaskState,
TaskStatus,
)
class FinishesDuringCancel(AgentExecutor):
"""The task's own work completes it while cancel() is in progress."""
def __init__(self, cancel_raises: bool) -> None:
self.cancel_raises = cancel_raises
self.started = asyncio.Event()
self.release = asyncio.Event()
self.completed = asyncio.Event()
async def execute(self, context: RequestContext, event_queue: EventQueue) -> None:
task_id, context_id = context.task_id, context.context_id
await event_queue.enqueue_event(
Task(id=task_id, context_id=context_id, status=TaskStatus(state=TaskState.TASK_STATE_SUBMITTED))
)
updater = TaskUpdater(event_queue, task_id, context_id)
await updater.start_work()
self.started.set()
await self.release.wait()
await updater.complete()
self.completed.set()
async def cancel(self, context: RequestContext, event_queue: EventQueue) -> None:
self.release.set()
await self.completed.wait()
if self.cancel_raises:
# What a shared TaskUpdater raises when cancel() loses the race to complete().
raise RuntimeError("Task is already in a terminal state.")
async def drain(stream) -> None:
async for _ in stream:
pass
async def run(cancel_raises: bool) -> None:
executor = FinishesDuringCancel(cancel_raises)
handler = DefaultRequestHandlerV2(
agent_executor=executor,
task_store=InMemoryTaskStore(),
agent_card=AgentCard(name="repro", capabilities=AgentCapabilities(streaming=True)),
)
ctx = ServerCallContext()
message = Message(role=Role.ROLE_USER, parts=[Part(text="go")], message_id="m1")
stream = handler.on_message_send_stream(SendMessageRequest(message=message), ctx)
first = await anext(stream)
task_id = first.id if isinstance(first, Task) else first.task_id
# Keep reading the stream, as a connected client does.
draining = asyncio.create_task(drain(stream))
await executor.started.wait()
try:
result = await handler.on_cancel_task(CancelTaskRequest(id=task_id), ctx)
outcome = f"returned task in {TaskState.Name(result.status.state)}"
except Exception as e:
outcome = f"raised {type(e).__name__}: {e}"
await draining
stored = await handler.on_get_task(GetTaskRequest(id=task_id), ctx)
print(f"cancel_raises={cancel_raises}: CancelTask {outcome}; stored state {TaskState.Name(stored.status.state)}")
asyncio.run(run(cancel_raises=False))
asyncio.run(run(cancel_raises=True))
Output (3 of 3 runs):
cancel_raises=False: CancelTask returned task in TASK_STATE_COMPLETED; stored state TASK_STATE_COMPLETED
cancel_raises=True: CancelTask raised RuntimeError: Task is already in a terminal state.; stored state TASK_STATE_FAILED
The second line is the bug: the stored state should stay TASK_STATE_COMPLETED. The first line shows the behaviour the related question is about.
Possible fix: in _mark_task_as_failed (or before calling it from cancel), skip the FAILED write when the task is already terminal, as _write_failed_status does.
We hit this in AG2's A2A server, whose executor shares one TaskUpdater between execute and cancel.
Relevant log output
Cancel[<task id>]: Agent cancel failed
Traceback (most recent call last):
File ".../a2a/server/agent_execution/active_task.py", line 834, in cancel
await self._agent_executor.cancel(
File "repro.py", line 54, in cancel
raise RuntimeError("Task is already in a terminal state.")
RuntimeError: Task is already in a terminal state.
Code of Conduct
- I agree to follow this project's Code of Conduct
- Linguagem predominante
- Python
- Estrelas
- 2.2k
- Forks
- 528
- Merge médio
- 3d 9h
- PRs com merge (30d)
- 56
Preparar o ambiente
- Sem Dockerfile nem arquivo Docker Compose
- Tem um modelo de pull request
- Ler o guia de contribuição
Primeiros passos
- Leia a issue inteira e depois o guia de contribuição do projeto.
- Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
- Faça um fork do repositório e trabalhe em uma branch.
- Abra um pull request que referencie o número da issue.
Mais de a2aproject/a2a-python
-
[Bug]: `A2A-Version` validation ignores minor versions and the query parameterTalvez já em andamento @rohityan assumiu há 66 dias. Abertacomponent: server status:awaiting response
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 75/100
a2aproject/a2a-python#1204 · 3 comentários · 1 responsável ·
Mantenedores costumam responder em até 2 dias
-
[Bug]: REST task/request id sanitizationTalvez já em andamento @Linux2010 assumiu há 125 dias. Abertamaintainers-only
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 65/100
a2aproject/a2a-python#805 · 1 comentário ·
Mantenedores costumam responder em até 2 dias
-
[Feat] Major listTalvez já em andamento @rohityan assumiu há 2 dias. Abertamaintainers-only
a2aproject/a2a-python#1398 · 1 reação · 3 responsáveis ·
Mantenedores costumam responder em até 2 dias
-
[Bug]: REST tenant routes are mounted before `path_prefix`, so tenant requests from the SDK client get 404Talvez já em andamento @Revinand assumiu há 3 dias. Abertacomponent: server status:awaiting response
a2aproject/a2a-python#1376 · 1 comentário · 1 responsável ·
Mantenedores costumam responder em até 2 dias
-
DefaultRequestHandler (v2) ignores queue_manager, so the per-task event queue bound (1,024) cannot be configuredTalvez já em andamento @mykytanetipa assumiu há 12 dias. Abertacomponent: server status: needs review
a2aproject/a2a-python#1339 · 2 comentários · 1 responsável ·
Mantenedores costumam responder em até 2 dias
Todas as issues de a2aproject/a2a-python
Issues semelhantes
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 82/100
Mantenedores costumam responder em até 1 dia
-
namespace operations
Dificuldade 1/5 Menos de uma hora Facilidade para iniciantes 65/100
EclipseFdn/open-vsx.org#15057 ·
Mantenedores costumam responder em até 1 dia
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 76/100
FinanceFlash/unvibecode#349 ·
-
Mission statement: why this is free, in the README, Why This Exists and the wiki HomeTalvez já em andamento @ChiefGyk3D assumiu hoje. Aberta
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 76/100
ChiefGyk3D/Clouds-Against-Humanity#348 ·
Mantenedores costumam responder em até 1 dia
-
[Perf] Default --vllm-watermark 0.05 for rollouts (hybrid models re-preempt under KV pressure)Aberta
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 74/100
vllm-project/vime#508 ·
Mantenedores costumam responder em até 4 dias