Skip to content

fix: enforce terminal state transitions and serialize cancel check-then-act - #453

Open
ez-lbz wants to merge 2 commits into
a2aproject:mainfrom
ez-lbz:fix/state-machine-cancel-race
Open

fix: enforce terminal state transitions and serialize cancel check-then-act#453
ez-lbz wants to merge 2 commits into
a2aproject:mainfrom
ez-lbz:fix/state-machine-cancel-race

Conversation

@ez-lbz

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

Copy link
Copy Markdown
Contributor

What changed

1. Terminal state transitions are enforced in TaskProjection

Problem: TaskProjection.ApplyStatus unconditionally overwrote current.Status, and ApplyMessage unconditionally appended to history. A task in a terminal state (COMPLETED/CANCELED/FAILED/REJECTED) could be overwritten back to SUBMITTED or accept further messages via direct state assignment, whenever events bypassed the request-level terminal checks (e.g. a misbehaving agent handler emitting events after a terminal status).

Fix (src/A2A/Server/TaskProjection.cs):

  • Added GuardNotTerminal, called from both ApplyStatus and ApplyMessage.
  • Terminal tasks now throw A2AException with A2AErrorCode.UnsupportedOperation (-32004, the same code A2AServer uses for terminal-state rejections) instead of being mutated.
  • Artifact updates on terminal tasks remain allowed (the projection only guards status/message mutations).

2. CancelTaskAsync check-then-act is serialized under the per-task lock

Problem: CancelTaskAsync read the task state and checked terminal status with no lock between the check and the state transition. Two concurrent cancel requests could both pass the check and both run the cancel handler (TOCTOU), and cancel could race with message/send mutations.

Fix (src/A2A/Server/A2AServer.cs):

  • The entire cancel operation — terminal check, background-CTS signalling, cancel-handler event application, and final state read — now runs under _notifier.AcquireTaskLockAsync(request.Id), the same per-task lock ApplyEventAsync and SubscribeToTaskAsync use.
  • The loser of a concurrent-cancel race re-reads the now-terminal task and fails with TaskNotCancelable instead of double-cancelling.
  • ApplyEventAsync was refactored into a lock-acquiring wrapper plus ApplyEventUnderLockAsync (the locked body), so the cancel path can apply events without re-acquiring the non-reentrant per-task semaphore (which would deadlock).

Testing

  • dotnet test tests/A2A.UnitTests --framework net8.0434 passed, 0 failed (baseline 420, +14 new regression tests: status-overwrite and message-append rejection for all four terminal states, same-terminal-state re-emission rejection, artifact-update tolerance on terminal tasks, and a two-concurrent-cancel test asserting exactly one succeeds while the other gets TaskNotCancelable).
  • dotnet test tests/A2A.AspNetCore.UnitTests --framework net8.088 passed, 0 failed (unchanged).
  • Behavior change: any agent handler that emits a status update or message after a task reaches a terminal state now gets an UnsupportedOperation error (previously silently applied). Well-behaved handlers are unaffected; cancel still transitions WORKINGCANCELED as before.

@ez-lbz
ez-lbz force-pushed the fix/state-machine-cancel-race branch from 1c26cc0 to 3c8fd2c Compare August 11, 2026 13:37
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