Skip to content

Commit 8c25c64

Browse files
authored
fix(tasks): preserve task_metadata in combined update and forward merge_params by name (#336)
1 parent 9cc71fb commit 8c25c64

4 files changed

Lines changed: 116 additions & 6 deletions

File tree

agentex/src/api/routes/tasks.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,9 @@ async def update_task_by_name(
214214
task_use_case: DTaskUseCase,
215215
) -> Task:
216216
updated_task_entity = await task_use_case.update_mutable_fields_on_task(
217-
name=task_name, task_metadata=request.task_metadata
217+
name=task_name,
218+
task_metadata=request.task_metadata,
219+
merge_params=request.merge_params,
218220
)
219221
return Task.model_validate(updated_task_entity)
220222

agentex/src/domain/use_cases/tasks_use_case.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,12 @@ async def update_mutable_fields_on_task(
113113
if task_metadata is None and merge_params is None:
114114
return task_entity
115115

116-
if task_metadata is not None:
117-
task_entity.task_metadata = task_metadata
118-
119116
# `merge_params` is a separate atomic JSONB shallow-merge so concurrent
120117
# callers don't overwrite each other's fields (vs reading→mutating→writing
121-
# the whole params dict on task_entity). Falls through to a normal save
122-
# only when task_metadata also changed.
118+
# the whole params dict on task_entity). Run it first so the refreshed
119+
# entity it returns becomes the base we apply `task_metadata` on top of;
120+
# otherwise the `task_entity = merged` reassignment would discard an
121+
# in-memory metadata change made before the merge.
123122
if merge_params:
124123
merged = await self.task_service.merge_task_params(
125124
task_entity.id, merge_params
@@ -128,6 +127,7 @@ async def update_mutable_fields_on_task(
128127
task_entity = merged
129128

130129
if task_metadata is not None:
130+
task_entity.task_metadata = task_metadata
131131
task_entity = await self.task_service.update_task(task=task_entity)
132132

133133
return task_entity

agentex/tests/integration/api/tasks/test_tasks_api.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,87 @@ async def test_update_task_by_name_endpoint_success(
911911
assert response_data["task_metadata"]["configuration"]["version"] == "2.0.0"
912912
assert response_data["task_metadata"]["metrics"]["complexity_score"] == 75
913913

914+
async def test_update_task_by_name_forwards_merge_params(
915+
self, isolated_client, isolated_repositories
916+
):
917+
"""PUT /tasks/name/{task_name} must forward merge_params, not drop it."""
918+
# Given - a task with existing params
919+
agent_repo = isolated_repositories["agent_repository"]
920+
agent = AgentEntity(
921+
id=orm_id(),
922+
name="merge-params-by-name-agent",
923+
description="Agent for merge_params by name testing",
924+
acp_url="http://test-acp:8000",
925+
acp_type=ACPType.SYNC,
926+
)
927+
await agent_repo.create(agent)
928+
929+
task_repo = isolated_repositories["task_repository"]
930+
task = TaskEntity(
931+
id=orm_id(),
932+
name="task-for-merge-params-by-name",
933+
status=TaskStatus.RUNNING,
934+
status_reason="Test task for merge_params by name endpoint",
935+
params={"model": "gpt-4", "temperature": 0.2},
936+
)
937+
created_task = await task_repo.create(agent_id=agent.id, task=task)
938+
939+
# When - update by name supplying only merge_params
940+
update_payload = {"merge_params": {"temperature": 0.9, "max_tokens": 1024}}
941+
response = await isolated_client.put(
942+
f"/tasks/name/{created_task.name}", json=update_payload
943+
)
944+
945+
# Then - the patch is shallow-merged into the existing params
946+
assert response.status_code == 200
947+
response_data = response.json()
948+
assert response_data["params"] == {
949+
"model": "gpt-4",
950+
"temperature": 0.9,
951+
"max_tokens": 1024,
952+
}
953+
954+
async def test_update_task_metadata_and_merge_params_together(
955+
self, isolated_client, isolated_repositories
956+
):
957+
"""Supplying both task_metadata and merge_params must persist both."""
958+
# Given - a task with existing params and metadata
959+
agent_repo = isolated_repositories["agent_repository"]
960+
agent = AgentEntity(
961+
id=orm_id(),
962+
name="merge-params-and-metadata-agent",
963+
description="Agent for combined update testing",
964+
acp_url="http://test-acp:8000",
965+
acp_type=ACPType.SYNC,
966+
)
967+
await agent_repo.create(agent)
968+
969+
task_repo = isolated_repositories["task_repository"]
970+
task = TaskEntity(
971+
id=orm_id(),
972+
name="task-for-combined-update",
973+
status=TaskStatus.RUNNING,
974+
status_reason="Test task for combined update endpoint",
975+
params={"model": "gpt-4"},
976+
task_metadata={"initial": "metadata"},
977+
)
978+
created_task = await task_repo.create(agent_id=agent.id, task=task)
979+
980+
# When - update by id supplying both fields at once
981+
update_payload = {
982+
"task_metadata": {"stage": "tuned"},
983+
"merge_params": {"temperature": 0.7},
984+
}
985+
response = await isolated_client.put(
986+
f"/tasks/{created_task.id}", json=update_payload
987+
)
988+
989+
# Then - neither field is silently discarded
990+
assert response.status_code == 200
991+
response_data = response.json()
992+
assert response_data["task_metadata"] == {"stage": "tuned"}
993+
assert response_data["params"] == {"model": "gpt-4", "temperature": 0.7}
994+
914995
async def test_list_tasks_includes_task_metadata_field(
915996
self, isolated_client, isolated_repositories
916997
):

agentex/tests/unit/use_cases/test_tasks_use_case.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,33 @@ async def test_update_metadata_by_name(
432432
# Then
433433
assert updated.task_metadata == {"via": "name"}
434434

435+
async def test_update_metadata_and_merge_params_both_persist(
436+
self, tasks_use_case, task_service, agent_repository, sample_agent
437+
):
438+
"""Supplying both task_metadata and merge_params must persist both fields.
439+
440+
Regression: the merge previously reassigned task_entity to the merged
441+
result, discarding the in-memory task_metadata before the final write.
442+
"""
443+
# Given
444+
await create_or_get_agent(agent_repository, sample_agent)
445+
task = await task_service.create_task(
446+
agent=sample_agent,
447+
task_name="combined-update-test",
448+
task_params={"model": "gpt-4"},
449+
)
450+
451+
# When
452+
updated = await tasks_use_case.update_mutable_fields_on_task(
453+
id=task.id,
454+
task_metadata={"stage": "tuned"},
455+
merge_params={"temperature": 0.7},
456+
)
457+
458+
# Then
459+
assert updated.task_metadata == {"stage": "tuned"}
460+
assert updated.params == {"model": "gpt-4", "temperature": 0.7}
461+
435462
async def test_update_metadata_on_deleted_task_raises(
436463
self, tasks_use_case, task_service, agent_repository, sample_agent
437464
):

0 commit comments

Comments
 (0)