Mc/golden agent tuneagent#309
Conversation
✱ Stainless preview buildsThis PR will update the openapi python typescript Edit this comment to update them. They will appear in their respective SDK's changelogs. ✅ agentex-sdk-openapi studio · code · diff
✅ agentex-sdk-typescript studio · code · diff
✅ agentex-sdk-python studio · code · diff
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async def merge_task_params(self, task_id: str, patch: dict) -> TaskEntity | None: | ||
| """Atomically merge ``patch`` into ``tasks.params``. Returns the | ||
| updated entity, or ``None`` if no task with ``task_id`` exists. | ||
|
|
||
| Used by live-config flows (e.g. ConfigModal Save → signal + | ||
| DB persistence) so the persisted task row reflects the new | ||
| agent config without waiting for the next task to be created. | ||
| """ | ||
| return await self.task_repository.merge_params(task_id, patch) |
There was a problem hiding this comment.
merge_task_params skips the task_updated stream event
update_task always publishes a task_updated event to the task event stream after writing to the DB. merge_task_params only calls task_repository.merge_params and returns, with no equivalent stream publish. When a caller patches only merge_params (no task_metadata), the if task_metadata is not None branch in the use case is skipped, so update_task is never reached and no event is ever emitted. Any consumer watching the stream — including the Temporal worker the PR description says must receive a signal — will not be notified of the config change.
Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/src/domain/services/task_service.py
Line: 245-253
Comment:
**`merge_task_params` skips the `task_updated` stream event**
`update_task` always publishes a `task_updated` event to the task event stream after writing to the DB. `merge_task_params` only calls `task_repository.merge_params` and returns, with no equivalent stream publish. When a caller patches only `merge_params` (no `task_metadata`), the `if task_metadata is not None` branch in the use case is skipped, so `update_task` is never reached and no event is ever emitted. Any consumer watching the stream — including the Temporal worker the PR description says must receive a signal — will not be notified of the config change.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Signal Endpoint
Before state:
After triggering post request, params successfully updated:
On agentex side, DB updated:
config updated signal also triggered:
worker updated:
Greptile Summary
This PR adds a
merge_paramsfield to thePATCH /tasks/{id}endpoint so that callers can atomically shallow-merge a config patch into the task'sparamsJSONB column without a read-modify-write race. The repository layer uses a singleUPDATE … RETURNINGwith Postgres'||JSONB operator, which is the correct approach.tasks.py,openapi.yaml):merge_params: dict[str, Any] | Noneadded toUpdateTaskRequestwith correct nullability and documentation.task_repository.py):merge_paramsusesCOALESCE(params, '{}'::jsonb) || cast(patch, JSONB)in a single atomic statement — no race with concurrent writers.routes/tasks.py): the by-id handler correctly forwardsmerge_params, but the by-name handler (update_task_by_name) was not updated and silently drops the field.Confidence Score: 4/5
The by-id PATCH path works correctly, but the by-name PUT endpoint silently drops merge_params on any request that supplies it.
The by-name handler (update_task_by_name) was not updated to forward merge_params to the use case, so callers using that endpoint will have the field silently ignored. The core repository logic (atomic JSONB merge) is sound, but the routing gap means the feature is incomplete and could cause silent data-loss for any consumer relying on the by-name path.
agentex/src/api/routes/tasks.py — the by-name handler needs merge_params forwarded.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client participant Router as tasks.py router participant UseCase as TasksUseCase participant Service as AgentTaskService participant Repo as TaskRepository participant DB as Postgres Client->>Router: "PATCH /tasks/{id} {merge_params: {...}}" Router->>UseCase: "update_mutable_fields_on_task(id, merge_params=...)" UseCase->>Service: get_task(id) Service->>Repo: get(id) Repo->>DB: SELECT task DB-->>Repo: TaskORM Repo-->>Service: TaskEntity Service-->>UseCase: TaskEntity alt merge_params provided UseCase->>Service: merge_task_params(task_id, patch) Service->>Repo: merge_params(task_id, patch) Repo->>DB: "UPDATE tasks SET params = COALESCE(params,'{}')::jsonb || patch::jsonb WHERE id=? RETURNING *" DB-->>Repo: updated TaskORM Repo-->>Service: TaskEntity (merged) Service-->>UseCase: TaskEntity (merged) Note over UseCase: No task_updated stream event emitted end alt task_metadata provided UseCase->>Service: update_task(task_entity) Service->>Repo: update(task) Repo->>DB: "UPDATE tasks SET ... WHERE id=?" DB-->>Repo: updated TaskORM Repo-->>Service: TaskEntity Service->>Service: publish task_updated to stream Service-->>UseCase: TaskEntity end UseCase-->>Router: TaskEntity Router-->>Client: Task (200 OK)%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Client participant Router as tasks.py router participant UseCase as TasksUseCase participant Service as AgentTaskService participant Repo as TaskRepository participant DB as Postgres Client->>Router: "PATCH /tasks/{id} {merge_params: {...}}" Router->>UseCase: "update_mutable_fields_on_task(id, merge_params=...)" UseCase->>Service: get_task(id) Service->>Repo: get(id) Repo->>DB: SELECT task DB-->>Repo: TaskORM Repo-->>Service: TaskEntity Service-->>UseCase: TaskEntity alt merge_params provided UseCase->>Service: merge_task_params(task_id, patch) Service->>Repo: merge_params(task_id, patch) Repo->>DB: "UPDATE tasks SET params = COALESCE(params,'{}')::jsonb || patch::jsonb WHERE id=? RETURNING *" DB-->>Repo: updated TaskORM Repo-->>Service: TaskEntity (merged) Service-->>UseCase: TaskEntity (merged) Note over UseCase: No task_updated stream event emitted end alt task_metadata provided UseCase->>Service: update_task(task_entity) Service->>Repo: update(task) Repo->>DB: "UPDATE tasks SET ... WHERE id=?" DB-->>Repo: updated TaskORM Repo-->>Service: TaskEntity Service->>Service: publish task_updated to stream Service-->>UseCase: TaskEntity end UseCase-->>Router: TaskEntity Router-->>Client: Task (200 OK)Comments Outside Diff (1)
agentex/src/api/routes/tasks.py, line 216-218 (link)update_task_by_namehandler does not forwardmerge_paramsto the use case, so any caller that uses the by-name endpoint and suppliesmerge_paramswill 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.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "comments" | Re-trigger Greptile