Conversation
bytecii
left a comment
There was a problem hiding this comment.
Maybe we can also add some unit tests for this. Thanks!
| question: tempMessageContent, | ||
| task_id: nextTaskId, | ||
| attaches: improveAttaches, | ||
| }); |
There was a problem hiding this comment.
Should we do something like this?
if (res?.code === 410) {
// Re-establish SSE + task lock via POST /chat
await chatStore.startTask(nextTaskId, ...);
}
There was a problem hiding this comment.
I think we need that because when the improve endpoint returns 410, we fall back to startTask() which reestablishes both the SSE consumer and the task lock.
ea25ee1 to
4cc1802
Compare
Yep, for sure. I added test cases for this. |
| def supplement(id: str, data: SupplementChat): | ||
| chat_logger.info("Chat supplement requested", extra={"task_id": id}) | ||
| task_lock = get_task_lock(id) |
There was a problem hiding this comment.
Why we still use get_task_lock in supplement, stop, add_task
There was a problem hiding this comment.
We keep get_task_lock() on supplement/stop/add_task because they only make sense with an existing active session; creating a new lock there would silently drop the action (no SSE consumer). improve is user-facing, so we return 410 and let the client reconnect via POST /chat to re-create both lock + SSE.
504dea0 to
479c5e1
Compare
Description
Fixes “task lock not found” errors by making lock acquisition/release idempotent and improving retry handling. This prevents failed runs when a lock record is missing or already released.
What is the purpose of this pull request?