Skip to content

Commit 03906c5

Browse files
fix: task cancellation architectural bug
- Fix task_cancel method to properly require agent identification - Previously incorrectly assumed task_name == agent_name and task_id == agent_id - Now correctly requires both agent (owner) and task parameters - Updated method signatures across ACPModule, activities, and service layers - Added test documentation showing correct usage pattern BREAKING CHANGE: task_cancel now requires explicit agent_name/agent_id parameter to identify which agent owns the task being cancelled
1 parent 564bedd commit 03906c5

File tree

4 files changed

+104
-14
lines changed

4 files changed

+104
-14
lines changed

src/agentex/lib/adk/_modules/acp.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,18 +205,22 @@ async def cancel_task(
205205
self,
206206
task_id: str | None = None,
207207
task_name: str | None = None,
208+
agent_id: str | None = None,
209+
agent_name: str | None = None,
208210
trace_id: str | None = None,
209211
parent_span_id: str | None = None,
210212
start_to_close_timeout: timedelta = timedelta(seconds=5),
211213
heartbeat_timeout: timedelta = timedelta(seconds=5),
212214
retry_policy: RetryPolicy = DEFAULT_RETRY_POLICY,
213215
) -> Task:
214216
"""
215-
Cancel a task.
217+
Cancel a task by sending cancel request to the agent that owns the task.
216218
217219
Args:
218-
task_id: The ID of the task to cancel.
219-
task_name: The name of the task to cancel.
220+
task_id: ID of the task to cancel.
221+
task_name: Name of the task to cancel.
222+
agent_id: ID of the agent that owns the task.
223+
agent_name: Name of the agent that owns the task.
220224
trace_id: The trace ID for the task.
221225
parent_span_id: The parent span ID for the task.
222226
start_to_close_timeout: The start to close timeout for the task.
@@ -225,13 +229,19 @@ async def cancel_task(
225229
226230
Returns:
227231
The task entry.
232+
233+
Raises:
234+
ValueError: If neither agent_name nor agent_id is provided,
235+
or if neither task_name nor task_id is provided
228236
"""
229237
if in_temporal_workflow():
230238
return await ActivityHelpers.execute_activity(
231239
activity_name=ACPActivityName.TASK_CANCEL,
232240
request=TaskCancelParams(
233241
task_id=task_id,
234242
task_name=task_name,
243+
agent_id=agent_id,
244+
agent_name=agent_name,
235245
trace_id=trace_id,
236246
parent_span_id=parent_span_id,
237247
),
@@ -244,6 +254,8 @@ async def cancel_task(
244254
return await self._acp_service.task_cancel(
245255
task_id=task_id,
246256
task_name=task_name,
257+
agent_id=agent_id,
258+
agent_name=agent_name,
247259
trace_id=trace_id,
248260
parent_span_id=parent_span_id,
249261
)

src/agentex/lib/core/services/adk/acp/acp.py

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -180,37 +180,70 @@ async def task_cancel(
180180
self,
181181
task_id: str | None = None,
182182
task_name: str | None = None,
183+
agent_id: str | None = None,
184+
agent_name: str | None = None,
183185
trace_id: str | None = None,
184186
parent_span_id: str | None = None,
185187
) -> Task:
188+
"""
189+
Cancel a task by sending cancel request to the agent that owns the task.
190+
191+
Args:
192+
task_id: ID of the task to cancel (passed to agent in params)
193+
task_name: Name of the task to cancel (passed to agent in params)
194+
agent_id: ID of the agent that owns the task
195+
agent_name: Name of the agent that owns the task
196+
trace_id: Trace ID for tracing
197+
parent_span_id: Parent span ID for tracing
198+
199+
Returns:
200+
Task entry representing the cancelled task
201+
202+
Raises:
203+
ValueError: If neither agent_name nor agent_id is provided,
204+
or if neither task_name nor task_id is provided
205+
"""
206+
# Require agent identification
207+
if not agent_name and not agent_id:
208+
raise ValueError("Either agent_name or agent_id must be provided to identify the agent that owns the task")
209+
210+
# Require task identification
211+
if not task_name and not task_id:
212+
raise ValueError("Either task_name or task_id must be provided to identify the task to cancel")
213+
186214
trace = self._tracer.trace(trace_id=trace_id)
187215
async with trace.span(
188216
parent_id=parent_span_id,
189217
name="task_cancel",
190218
input={
191219
"task_id": task_id,
192220
"task_name": task_name,
221+
"agent_id": agent_id,
222+
"agent_name": agent_name,
193223
},
194224
) as span:
195225
heartbeat_if_in_workflow("task cancel")
226+
227+
# Build params for the agent (task identification)
228+
params = {}
229+
if task_id:
230+
params["task_id"] = task_id
196231
if task_name:
232+
params["task_name"] = task_name
233+
234+
# Send cancel request to the correct agent
235+
if agent_name:
197236
json_rpc_response = await self._agentex_client.agents.rpc_by_name(
198-
agent_name=task_name,
237+
agent_name=agent_name,
199238
method="task/cancel",
200-
params={
201-
"task_name": task_name,
202-
},
239+
params=params,
203240
)
204-
elif task_id:
241+
else: # agent_id is provided (validated above)
205242
json_rpc_response = await self._agentex_client.agents.rpc(
206-
agent_id=task_id,
243+
agent_id=agent_id,
207244
method="task/cancel",
208-
params={
209-
"task_id": task_id,
210-
},
245+
params=params,
211246
)
212-
else:
213-
raise ValueError("Either task_name or task_id must be provided")
214247

215248
task_entry = Task.model_validate(json_rpc_response.result)
216249
if span:

src/agentex/lib/core/temporal/activities/adk/acp/acp_activities.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ class EventSendParams(BaseModelWithTraceParams):
4545
class TaskCancelParams(BaseModelWithTraceParams):
4646
task_id: str | None = None
4747
task_name: str | None = None
48+
agent_id: str | None = None
49+
agent_name: str | None = None
4850

4951

5052
class ACPActivities:
@@ -83,4 +85,8 @@ async def task_cancel(self, params: TaskCancelParams) -> Task:
8385
return await self._acp_service.task_cancel(
8486
task_id=params.task_id,
8587
task_name=params.task_name,
88+
agent_id=params.agent_id,
89+
agent_name=params.agent_name,
90+
trace_id=params.trace_id,
91+
parent_span_id=params.parent_span_id,
8692
)

tests/test_task_cancel.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
"""Tests for task cancellation bug fix."""
2+
3+
import os
4+
import pytest
5+
6+
from agentex import AsyncAgentex
7+
from tests.utils import assert_matches_type
8+
from agentex.types import Task
9+
10+
base_url = os.environ.get("TEST_API_BASE_URL", "http://127.0.0.1:4010")
11+
12+
13+
class TestTaskCancelBugFix:
14+
"""Test that task cancellation bug is fixed - agent identification is required."""
15+
parametrize = pytest.mark.parametrize("client", [False, True], indirect=True, ids=["loose", "strict"])
16+
17+
@pytest.mark.skip(reason="Integration test - demonstrates the fix for task cancel bug")
18+
@parametrize
19+
async def test_task_cancel_requires_agent_and_task_identification(self, client: AsyncAgentex) -> None:
20+
"""
21+
Test that demonstrates the task cancellation bug fix.
22+
23+
Previously: task_cancel(task_name="my-task") incorrectly treated task_name as agent_name
24+
Fixed: task_cancel(task_name="my-task", agent_name="my-agent") correctly identifies both
25+
"""
26+
# This test documents the correct usage pattern
27+
# In practice, you would need a real agent and task for this to work
28+
try:
29+
task = await client.agents.cancel_task(
30+
agent_name="test-agent", # REQUIRED: Agent that owns the task
31+
params={
32+
"task_id": "test-task-123" # REQUIRED: Task to cancel
33+
}
34+
)
35+
assert_matches_type(Task, task, path=["response"])
36+
except Exception:
37+
# Expected to fail in test environment without real agents/tasks
38+
# The important thing is that the API now requires both parameters
39+
pass

0 commit comments

Comments
 (0)