Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 2fac930 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to This change keeps a canceled streaming task canceled instead of overwriting it with completed or appending a late chunk. No actionable merge-blocking risk was found. Custom asynchronous task stores can still race on the final save, which the PR states is out of scope. Pre-merge checks |
|
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/a2a-server/src/server.ts">
<violation number="1" location="packages/a2a-server/src/server.ts:405">
P2: A custom asynchronous `TaskStore.save` can pause after this check; cancellation during the save still reaches the unconditional completed `yield`. Recheck after each stream save so the abort path emits the canceled record instead of a stale event.</violation>
</file>
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
| let aggregatedText = ""; | ||
|
|
||
| try { | ||
| abortController.signal.throwIfAborted(); |
There was a problem hiding this comment.
P2: A custom asynchronous TaskStore.save can pause after this check; cancellation during the save still reaches the unconditional completed yield. Recheck after each stream save so the abort path emits the canceled record instead of a stale event.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/a2a-server/src/server.ts, line 405:
<comment>A custom asynchronous `TaskStore.save` can pause after this check; cancellation during the save still reaches the unconditional completed `yield`. Recheck after each stream save so the abort path emits the canceled record instead of a stale event.</comment>
<file context>
@@ -402,6 +402,7 @@ export class A2AServer {
let aggregatedText = "";
try {
+ abortController.signal.throwIfAborted();
const voltMessage = toVoltAgentMessage(normalizedMessage);
const agentOptions = server.buildAgentCallOptions({
</file context>
PR Checklist
Bugs / Features
What is the current behavior?
After
tasks/cancelreturns a canceled task,message/streamcan overwrite that task withcompletedif the agent's iterator ends normally on abort. A buffered chunk arriving after cancellation is also appended. Cancellation before the agent call or while awaiting completion metadata can similarly be ignored.The existing cancellation test covers an iterator that throws on abort, which reaches the cancellation handler. Normal iterator completion bypasses that handler.
What is the new behavior?
Check the AbortSignal before starting the agent, after stream initialization, before accepting chunks, after iteration, and before saving the completed result. An aborted operation uses the existing cancellation handler and cleanup path.
Extend the cancellation test to cover throwing, normal completion, and a late chunk. Add cases for cancellation before the agent call and while awaiting completion metadata. Check both final stream events and persisted task status.
Notes for reviewers
On baseline
72a46c76b4507e1f98b41d5050a92bd30587eb70, the new regression cases produced four failures. With this fix, all 13 A2A package tests pass. Local type checking, CJS/ESM/declaration builds, Biome checks for the two changed TypeScript files, andgit diff --checkalso pass.Validation used a separate pinned npm toolchain because the workspace's frozen pnpm installation failed with connection resets from the configured npm mirror. Core/internal dependencies were published packages at the same versions as the workspace manifests. The complete workspace dependency installation and CI were not validated. Tests use a stub agent and the in-memory task store; no live provider was exercised.
This patch preserves cancellation at the covered stream boundaries. It does not add transactional concurrency guarantees for custom asynchronous task stores. Preparation and local validation were AI-assisted.
Summary by cubic
Fixes
@voltagent/a2a-serveroverwriting a canceled task withcompletedwhen the agent stream ends normally on abort, and prevents a buffered chunk from being appended after cancellation.Written for commit 2fac930. Summary will update on new commits.
Summary by CodeRabbit