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-python studio · code · diff
✅ agentex-sdk-typescript studio · code · diff
✅ agentex-sdk-openapi 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}
| if merge_params: | ||
| merged = await self.task_service.merge_task_params(id, merge_params) | ||
| if merged is not None: | ||
| return merged | ||
| return task |
There was a problem hiding this comment.
When merge_params is provided, merge_task_params returns None only if the task row wasn't found by the UPDATE ... WHERE id = ? — meaning the task was deleted between the RUNNING guard above and the DB write. In that case the signal was already dispatched to Temporal (the workflow proceeds with the new config), but the code silently falls through to return task, handing the caller a 200 with the pre-signal entity and never surfacing that the DB write failed. The caller has no way to distinguish "merge succeeded" from "merge was silently dropped".
Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/src/domain/use_cases/tasks_use_case.py
Line: 235-239
Comment:
**Silent stale-entity return when DB merge is skipped**
When `merge_params` is provided, `merge_task_params` returns `None` only if the task row wasn't found by the `UPDATE ... WHERE id = ?` — meaning the task was deleted between the RUNNING guard above and the DB write. In that case the signal was already dispatched to Temporal (the workflow proceeds with the new config), but the code silently falls through to `return task`, handing the caller a 200 with the *pre-signal* entity and never surfacing that the DB write failed. The caller has no way to distinguish "merge succeeded" from "merge was silently dropped".
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
POST /tasks/{task_id}/signalendpoint that dispatches a named Temporal signal to a running task's workflow, with an optional shallow-merge of the task'sparamsJSONB column so the DB stays in sync with any live config change.SignalTaskRequestcarriessignal_name, an optionalpayloadforwarded as-is to the Temporal signal handler, and an optionalmerge_paramsdict for the DB patch.TaskRepository.merge_paramsuses Postgres'||JSONB operator inside a singleUPDATE … RETURNINGto avoid a read-modify-write race;COALESCEguards against a NULL existing column.TasksUseCase.signal_taskvalidates the task is RUNNING, dispatches the Temporal signal, then conditionally applies the DB patch — but silently returns a stale entity when the DB update finds no matching row (see inline comments).Confidence Score: 3/5
The core JSONB merge and route wiring are solid, but two defects in the signaling path need fixing before merging: the use case silently swallows a failed DB write and returns stale data, and the Temporal adapter unconditionally forwards None as a signal argument which will break handlers with no parameters.
The merge_params silent-fallback means a caller can get a 200 response showing the old params even though the DB update never landed — the Temporal workflow proceeds with new config but the stored task row does not. Separately, adapter_temporal.py always calls handle.signal(signal, arg) even when arg=None, sending a JSON-null positional argument to any signal handler that declares no parameters; this PR is the first consumer of signal_workflow, so this path is now live.
agentex/src/domain/use_cases/tasks_use_case.py (silent stale-entity return) and agentex/src/adapters/temporal/adapter_temporal.py (unconditional None arg in handle.signal)
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant Router as POST /tasks/{id}/signal participant UseCase as TasksUseCase participant TaskSvc as AgentTaskService participant TemporalAdapter participant Temporal participant Repo as TaskRepository participant DB as PostgreSQL Client->>Router: signal_name, payload?, merge_params? Router->>UseCase: signal_task(id, signal_name, payload, merge_params) UseCase->>TaskSvc: get_task(id) TaskSvc->>DB: "SELECT task WHERE id=?" DB-->>TaskSvc: TaskEntity TaskSvc-->>UseCase: "task (status=RUNNING guard)" UseCase->>TemporalAdapter: "signal_workflow(workflow_id=id, signal=signal_name, arg=payload)" TemporalAdapter->>Temporal: handle.signal(signal_name, arg) Temporal-->>TemporalAdapter: OK TemporalAdapter-->>UseCase: void alt merge_params provided UseCase->>TaskSvc: merge_task_params(id, patch) TaskSvc->>Repo: merge_params(task_id, patch) Repo->>DB: "UPDATE tasks SET params = COALESCE(params, {})::jsonb || patch::jsonb WHERE id=? RETURNING *" DB-->>Repo: updated row or None Repo-->>TaskSvc: TaskEntity or None TaskSvc-->>UseCase: merged entity or silent fallback to stale task end UseCase-->>Router: TaskEntity Router-->>Client: Task 200 OKPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "Merge branch 'main' of github.com:scalea..." | Re-trigger Greptile