fix: enforce first-persisted terminal state under the task lock (#401) - #449
Open
chopmob-cloud wants to merge 2 commits into
Open
fix: enforce first-persisted terminal state under the task lock (#401)#449chopmob-cloud wants to merge 2 commits into
chopmob-cloud wants to merge 2 commits into
Conversation
…roject#401) CancelTaskAsync and TryTransitionToFailedAsync check IsTerminal before taking the per-task lock, so a concurrent writer can persist a terminal state in the window before the forced write is applied, letting a background failure overwrite Completed/Canceled with Failed or a cancellation overwrite Completed/Failed with Canceled. Re-check the current state under the lock in ApplyEventAsync (the single choke point every terminal write funnels through) and drop a status update that would move an already-terminal task to a different state. Adds two deterministic regression tests, one per forced writer. Signed-off-by: AlgoVoi <chopmob@gmail.com>
chopmob-cloud
force-pushed
the
fix/terminal-state-precedence-401
branch
from
August 6, 2026 16:05
86dbee9 to
bc24e17
Compare
Signed-off-by: AlgoVoi <chopmob@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #401. Prevents a racing forced terminal transition (background failure or
cancellation) from overwriting a terminal state another writer has already
persisted. Adds an atomic "first persisted terminal state wins" guard inside
A2AServer.ApplyEventAsync, under the per-task lock.Why
CancelTaskAsyncandTryTransitionToFailedAsyncboth checkIsTerminalbeforetaking the per-task lock. A concurrent writer can persist a terminal state in the
window between that optimistic check and the locked apply, so the forced writer
then overwrites it:
CompletedorCanceledtask withFailed.CompletedorFailedtask withCanceled, so atask the agent actually completed is reported as canceled.
How
ApplyEventAsyncis the single choke point through which every terminal write ispersisted, and it already holds the per-task lock. Re-reading the current state
under that lock and dropping a status update that would move an already-terminal
task to a different state closes the race for every forced writer at once,
including any added later, rather than patching each call site:
The existing pre-lock checks are kept as fast-path rejections for the common
(non-racing) case. The guard is scoped to status-update events, which are the
vector used by both forced terminal writers.
Tests
Two deterministic regression tests in
tests/A2A.UnitTests/GitHubIssues/Issue401.cs,one per forced writer. Each drives the race window between the pre-lock check and
the locked apply (the cancel test gates the cancel handler; the background-failure
test arms a task store that injects a terminal state on the drain's under-lock read
and signals when to await the drain safely). Both fail without the guard
(
Actual: CanceledandActual: Failed) and pass with it.Validation
Clean container, base main, both target frameworks:
plus the 2 new tests). No regressions.
runs. With the guard reverted, both fail with the expected actuals, confirming they
exercise the fix.
Closes #401.