Skip to content

Commit 13eea7a

Browse files
feat: update task configs in agentex db on turn (#309)
When pre-existing PATCH endpoint is supplied with "task_id" params, shallow merges changed configs into agentex backend: Clicking "Save" in "Tune Agent" produces this PATCH <img width="706" height="232" alt="Screenshot 2026-06-18 at 6 01 03 PM" src="https://github.com/user-attachments/assets/cf7d9e85-5971-4956-8c31-7ed0ebad2a4c" /> Querying agentex backend to see if system prompt is updated: <img width="2144" height="216" alt="image" src="https://github.com/user-attachments/assets/2b5c4df1-bc0b-442f-9dd3-6de803542b8b" /> <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR wires a new `merge_params` field through the full API stack — schema, route, use case, service, and repository — so callers can shallow-merge a dict into `tasks.params` without overwriting the entire column. The repository layer uses PostgreSQL's `||` JSONB operator inside an `UPDATE … RETURNING` statement for a safe, atomic merge. - **Repository (`merge_params`)**: Atomic JSONB merge via `COALESCE(params, '{}') || patch` is correct; `expire_on_commit=False` makes the post-commit `model_validate` safe without an extra `refresh()` call. - **Use case (`update_mutable_fields_on_task`)**: Two prior-thread issues remain open — when both `task_metadata` and `merge_params` are supplied, the `task_entity = merged` reassignment discards the in-memory `task_metadata` mutation before the final `update_task` write; and the `update_task_by_name` route handler does not forward `merge_params` (flagged in the outside-diff comment). - **Minor inconsistency**: the early-return guard uses `is None` while the execution branch uses truthiness (`if merge_params:`), causing an empty-dict `{}` to silently bypass the merge. <details><summary><h3>Confidence Score: 3/5</h3></summary> Not yet safe to merge — two functional defects flagged in prior review rounds remain unaddressed in the code. When both `task_metadata` and `merge_params` are supplied, `task_entity = merged` overwrites the in-memory metadata update before the final `update_task` write, so the caller's `task_metadata` change is silently discarded. Separately, the `update_task_by_name` route handler does not forward `merge_params`, silently dropping it for callers using the name-based endpoint. Both defects were flagged in prior review comments and are still present in the current diff. `agentex/src/domain/use_cases/tasks_use_case.py` (dual-field update logic) and `agentex/src/api/routes/tasks.py` (`update_task_by_name` handler) need the most attention before merge. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex/src/domain/repositories/task_repository.py | Adds `merge_params` method using PostgreSQL JSONB `||` operator for an atomic shallow-merge update; correctly handles NULL coalescing and uses RETURNING to avoid a second SELECT. | | agentex/src/domain/services/task_service.py | Thin delegation method `merge_task_params` added; no stream event emitted (intentional per developer). | | agentex/src/domain/use_cases/tasks_use_case.py | Two unresolved P1s from prior threads remain: `task_entity = merged` overwrites in-memory task_metadata; `if merge_params:` inconsistent with `is None` guard. | | agentex/src/api/routes/tasks.py | `update_task` (by-id) correctly forwards `merge_params`; `update_task_by_name` still omits it. | | agentex/src/api/schemas/tasks.py | Adds `merge_params: dict[str, Any] | None` to `UpdateTaskRequest` with clear docstring. | | agentex/openapi.yaml | OpenAPI spec updated to document `merge_params` with correct `anyOf: [object, null]` schema. | </details> </details> <!-- greptile_failed_comments --> <details open><summary><h3>Comments Outside Diff (1)</h3></summary> 1. `agentex/src/api/routes/tasks.py`, line 216-218 ([link](https://github.com/scaleapi/scale-agentex/blob/4369c55ac1f53428561f71011cebd98a7e6b8264/agentex/src/api/routes/tasks.py#L216-L218)) <a href="#"><img alt="P1" src="https://greptile-static-assets.s3.amazonaws.com/badges/p1.svg?v=9" align="top"></a> The `update_task_by_name` handler does not forward `merge_params` to the use case, so any caller that uses the by-name endpoint and supplies `merge_params` will silently have the field dropped. The by-id handler (a few lines above) correctly passes it, but the by-name handler was not updated in this PR. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: agentex/src/api/routes/tasks.py Line: 216-218 Comment: The `update_task_by_name` handler does not forward `merge_params` to the use case, so any caller that uses the by-name endpoint and supplies `merge_params` will silently have the field dropped. The by-id handler (a few lines above) correctly passes it, but the by-name handler was not updated in this PR. How can I resolve this? If you propose a fix, please make it concise. ````` </details> <a href="https://app.greptile.com/api/ide/cursor?prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0APath%3A%20agentex%2Fsrc%2Fapi%2Froutes%2Ftasks.py%0ALine%3A%20216-218%0A%0AComment%3A%0AThe%20%60update_task_by_name%60%20handler%20does%20not%20forward%20%60merge_params%60%20to%20the%20use%20case%2C%20so%20any%20caller%20that%20uses%20the%20by-name%20endpoint%20and%20supplies%20%60merge_params%60%20will%20silently%20have%20the%20field%20dropped.%20The%20by-id%20handler%20%28a%20few%20lines%20above%29%20correctly%20passes%20it%2C%20but%20the%20by-name%20handler%20was%20not%20updated%20in%20this%20PR.%0A%0A%60%60%60suggestion%0A%20%20%20%20updated_task_entity%20%3D%20await%20task_use_case.update_mutable_fields_on_task%28%0A%20%20%20%20%20%20%20%20name%3Dtask_name%2C%20task_metadata%3Drequest.task_metadata%2C%20merge_params%3Drequest.merge_params%0A%20%20%20%20%29%0A%60%60%60%0A%0AHow%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20please%20make%20it%20concise.&pr=309&platform=github"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCursorDark.svg?v=3"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCursor.svg?v=3"><img alt="Fix in Cursor" src="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCursor.svg?v=3" height="20"></picture></a> <a href="https://app.greptile.com/ide/claude-code?prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0APath%3A%20agentex%2Fsrc%2Fapi%2Froutes%2Ftasks.py%0ALine%3A%20216-218%0A%0AComment%3A%0AThe%20%60update_task_by_name%60%20handler%20does%20not%20forward%20%60merge_params%60%20to%20the%20use%20case%2C%20so%20any%20caller%20that%20uses%20the%20by-name%20endpoint%20and%20supplies%20%60merge_params%60%20will%20silently%20have%20the%20field%20dropped.%20The%20by-id%20handler%20%28a%20few%20lines%20above%29%20correctly%20passes%20it%2C%20but%20the%20by-name%20handler%20was%20not%20updated%20in%20this%20PR.%0A%0A%60%60%60suggestion%0A%20%20%20%20updated_task_entity%20%3D%20await%20task_use_case.update_mutable_fields_on_task%28%0A%20%20%20%20%20%20%20%20name%3Dtask_name%2C%20task_metadata%3Drequest.task_metadata%2C%20merge_params%3Drequest.merge_params%0A%20%20%20%20%29%0A%60%60%60%0A%0AHow%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20please%20make%20it%20concise.&repo=scaleapi%2Fscale-agentex&pr=309&platform=github"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInClaudeDark.svg?v=3"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInClaude.svg?v=3"><img alt="Fix in Claude Code" src="https://greptile-static-assets.s3.amazonaws.com/badges/FixInClaude.svg?v=3" height="20"></picture></a> <a href="https://app.greptile.com/api/ide/codex?prompt=IMPORTANT%3A%20Work%20in%20the%20repository%20%22scaleapi%2Fscale-agentex%22%20on%20the%20existing%20branch%20%22mc%2Fgolden-agent-tuneagent%22.%20Checkout%20that%20branch%20%E2%80%94%20do%20NOT%20create%20a%20new%20branch%20or%20open%20a%20new%20PR.%20Push%20your%20changes%20to%20%22mc%2Fgolden-agent-tuneagent%22.%0A%0AThis%20is%20a%20comment%20left%20during%20a%20code%20review.%0APath%3A%20agentex%2Fsrc%2Fapi%2Froutes%2Ftasks.py%0ALine%3A%20216-218%0A%0AComment%3A%0AThe%20%60update_task_by_name%60%20handler%20does%20not%20forward%20%60merge_params%60%20to%20the%20use%20case%2C%20so%20any%20caller%20that%20uses%20the%20by-name%20endpoint%20and%20supplies%20%60merge_params%60%20will%20silently%20have%20the%20field%20dropped.%20The%20by-id%20handler%20%28a%20few%20lines%20above%29%20correctly%20passes%20it%2C%20but%20the%20by-name%20handler%20was%20not%20updated%20in%20this%20PR.%0A%0A%60%60%60suggestion%0A%20%20%20%20updated_task_entity%20%3D%20await%20task_use_case.update_mutable_fields_on_task%28%0A%20%20%20%20%20%20%20%20name%3Dtask_name%2C%20task_metadata%3Drequest.task_metadata%2C%20merge_params%3Drequest.merge_params%0A%20%20%20%20%29%0A%60%60%60%0A%0AHow%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20please%20make%20it%20concise.&repo=scaleapi%2Fscale-agentex&pr=309&platform=github"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCodexDark.svg?v=3"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCodex.svg?v=3"><img alt="Fix in Codex" src="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCodex.svg?v=3" height="20"></picture></a> </details> <!-- /greptile_failed_comments --> <sub>Reviews (5): Last reviewed commit: ["Merge branch &#39;main&#39; into mc/golden-agent..."](2a5f83b) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=37742683)</sub> <!-- /greptile_comment -->
1 parent 8c060f1 commit 13eea7a

6 files changed

Lines changed: 87 additions & 6 deletions

File tree

agentex/openapi.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6696,6 +6696,13 @@ components:
66966696
type: object
66976697
- type: 'null'
66986698
title: If provided, replaces task_metadata with this value
6699+
merge_params:
6700+
anyOf:
6701+
- additionalProperties: true
6702+
type: object
6703+
- type: 'null'
6704+
title: Optional shallow-merge patch applied to the task's params column.
6705+
Top-level keys overwrite; pass full nested objects to change subfields.
66996706
type: object
67006707
title: UpdateTaskRequest
67016708
ValidationError:

agentex/src/api/routes/tasks.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,9 @@ async def update_task(
193193
task_use_case: DTaskUseCase,
194194
) -> Task:
195195
updated_task_entity = await task_use_case.update_mutable_fields_on_task(
196-
id=task_id, task_metadata=request.task_metadata
196+
id=task_id,
197+
task_metadata=request.task_metadata,
198+
merge_params=request.merge_params,
197199
)
198200
return Task.model_validate(updated_task_entity)
199201

agentex/src/api/schemas/tasks.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,14 @@ class UpdateTaskRequest(BaseModel):
7777
None,
7878
title="If provided, replaces task_metadata with this value",
7979
)
80+
merge_params: dict[str, Any] | None = Field(
81+
None,
82+
title=(
83+
"Optional shallow-merge patch applied to the task's params column. "
84+
"Top-level keys overwrite; pass full nested objects to change "
85+
"subfields."
86+
),
87+
)
8088

8189

8290
class TaskStatusReasonRequest(BaseModel):

agentex/src/domain/repositories/task_repository.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
from typing import Annotated, Literal
44

55
from fastapi import Depends
6-
from sqlalchemy import distinct, func, select, update
6+
from sqlalchemy import cast, distinct, func, select, update
7+
from sqlalchemy.dialects.postgresql import JSONB
78
from sqlalchemy.orm import selectinload
89
from src.adapters.crud_store.adapter_postgres import (
910
ColumnPrimitiveValue,
@@ -213,6 +214,46 @@ async def update(self, task: TaskEntity) -> TaskEntity:
213214
# Return with agents populated
214215
return TaskEntity.model_validate(modified_orm)
215216

217+
async def merge_params(self, task_id: str, patch: dict) -> TaskEntity | None:
218+
"""Atomically shallow-merge ``patch`` into the task's ``params``
219+
JSONB column. Returns the updated entity, or ``None`` if no task
220+
with that id exists.
221+
222+
Uses Postgres' ``||`` operator on JSONB so the read-modify-write
223+
is collapsed to a single statement (no race with concurrent
224+
writers). ``patch`` keys overwrite existing keys at the top
225+
level; nested objects are NOT deep-merged — pass the full nested
226+
replacement if you need to change a subfield.
227+
228+
Intended for surfaces that need ``tasks.params`` to reflect the
229+
latest agent config after a live edit (see
230+
``TasksUseCase.update_mutable_fields_on_task``); not a
231+
general-purpose updater.
232+
"""
233+
234+
async with (
235+
self.start_async_db_session(True) as session,
236+
async_sql_exception_handler(),
237+
):
238+
# ``COALESCE(params, '{}'::jsonb)`` so a NULL existing value
239+
# doesn't poison the concat to NULL. Both operands cast to
240+
# JSONB explicitly so Postgres picks the JSONB ``||`` operator
241+
# (not the text concat overload).
242+
existing = func.coalesce(TaskORM.params, cast({}, JSONB))
243+
merged = existing.op("||", return_type=JSONB)(cast(patch, JSONB))
244+
stmt = (
245+
update(TaskORM)
246+
.where(TaskORM.id == task_id)
247+
.values(params=merged)
248+
.returning(TaskORM)
249+
)
250+
result = await session.execute(stmt)
251+
row = result.scalar_one_or_none()
252+
await session.commit()
253+
if row is None:
254+
return None
255+
return TaskEntity.model_validate(row)
256+
216257
async def transition_status(
217258
self,
218259
task_id: str,

agentex/src/domain/services/task_service.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,15 @@ async def update_task(self, task: TaskEntity) -> TaskEntity:
242242

243243
return updated_task
244244

245+
async def merge_task_params(self, task_id: str, patch: dict) -> TaskEntity | None:
246+
"""Atomically shallow-merge ``patch`` into ``tasks.params``. Returns
247+
the updated entity, or ``None`` if no task with ``task_id`` exists.
248+
249+
Lets callers update agent config on the task row in place; the agent
250+
picks up the new values when it next reads ``task.params``.
251+
"""
252+
return await self.task_repository.merge_params(task_id, patch)
253+
245254
async def delete_task(self, id: str | None = None, name: str | None = None) -> None:
246255
"""
247256
Delete a task from the repository.

agentex/src/domain/use_cases/tasks_use_case.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ async def update_mutable_fields_on_task(
9494
id: str | None = None,
9595
name: str | None = None,
9696
task_metadata: dict[str, Any] | None = None,
97+
merge_params: dict[str, Any] | None = None,
9798
) -> TaskEntity:
9899
"""Update mutable fields on a task entity. This is used by our API since not all fields should be mutable."""
99100

@@ -108,15 +109,28 @@ async def update_mutable_fields_on_task(
108109
else:
109110
raise ItemDoesNotExist(f"Task {name} not found")
110111

111-
# if no mutations are provided, don't do anything
112-
if task_metadata is None:
112+
# No-op if neither field was supplied.
113+
if task_metadata is None and merge_params is None:
113114
return task_entity
114115

115116
if task_metadata is not None:
116117
task_entity.task_metadata = task_metadata
117118

118-
updated_task_entity = await self.task_service.update_task(task=task_entity)
119-
return updated_task_entity
119+
# `merge_params` is a separate atomic JSONB shallow-merge so concurrent
120+
# 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.
123+
if merge_params:
124+
merged = await self.task_service.merge_task_params(
125+
task_entity.id, merge_params
126+
)
127+
if merged is not None:
128+
task_entity = merged
129+
130+
if task_metadata is not None:
131+
task_entity = await self.task_service.update_task(task=task_entity)
132+
133+
return task_entity
120134

121135
async def _transition_to_terminal(
122136
self,

0 commit comments

Comments
 (0)