Wait for the workflow lock before draining queued prompts
This MR fixes a bug in the prompt queueing added in !248098 (merged): a queued prompt drained fast enough to open its websocket before the previous socket, and the Workhorse lock tied to it, had actually been released, so the server rejected the connection and the prompt was silently dropped with no reply.
What changed
duo_agentic_chat_state_manager.vuenow tracks whether a websocket is currently attached to the workflow and blocks sending until it is confirmed closed, instead of trusting the agent'sINPUT_REQUIREDstatus, which arrives before the socket actually closes.- When Workhorse rejects a connection because the lock is still held, the undelivered prompt is put back at the front of the queue and its orphaned bubble is removed from the thread, rather than left stranded with no reply.
- The "busy in another tab" lock is now held only while the flow status shows a
client still attached (
RUNNING,TOOL_CALL_APPROVAL_REQUIRED,PLAN_APPROVAL_REQUIRED); any other status frees it, since an unrecognised status must not strand the queue. requeueUndeliveredPromptnow enqueues the bounced prompt before removing its bubble from the thread. Previously the removal happened first, so a failure partway through the requeue lost the prompt outright; now the worst case is a duplicate bubble instead of a deletion.- Corrected two pre-existing problems in the state manager spec file surfaced while writing the regression tests (see notes for reviewers).
How to test
The natural reproduction is timing dependent and hard to hit on demand, since it needs a queued prompt to drain inside a narrow release window. The two reproductions below are deterministic.
Force the lock (deterministic)
This reproduction exercises the requeue and the lock-status handling, not the drain race: a lock held from outside the app is not the same thing as the client racing its own socket.
- Open Duo Agentic Chat in the GDK and send a prompt that runs for a while, for example "explain this repository in detail".
- While the turn is running, find the lock key:
gdk redis-cli KEYS 'workhorse:duoworkflow:lock:*'. The suffix is the session ID, which is also visible in the chat URL. - Wait for the turn to finish, then hold the lock yourself so the next
connection cannot get it:
gdk redis-cli SET 'workhorse:duoworkflow:lock:<session-id>' held EX 300. - Send another prompt.
- Release the key:
gdk redis-cli DEL 'workhorse:duoworkflow:lock:<session-id>'.
Before: the prompt appears in the thread, gets no reply, and the "responding in another tab or location" notice shows. The prompt is lost.
After: the prompt comes back as an italic "Queued" bubble rather than sitting unanswered in the thread, and the notice shows while the key is held. The queued prompt may not fire on its own when the key is released, for the reason under Known limitation below; sending anything else moves it.
Two tabs (deterministic)
- Open the same chat thread in two browser tabs.
- In tab A, send a prompt that runs for a while.
- In tab B, submit a prompt. It queues and shows the notice.
Before: tab B clears the lock on the very next status poll, within about 3 seconds, fires the prompt into the still-running flow, and the notice comes straight back. The prompt is lost.
After: tab B stays locked until tab A's turn actually finishes, then the queued prompt goes out and is answered.
Observe the ordering
Open DevTools, Network, filter to WS. Before the fix, the queued prompt's connection opens while the previous one is still open and closes with 1013. After the fix, the new connection only starts after the previous one has closed.
Specs
yarn jest ee/spec/frontend/ai/duo_agentic_chat/components/duo_agentic_chat_state_manager_spec.js
yarn jest ee/spec/frontend/ai/duo_agentic_chat/components/prompt_queue_spec.jsKnown limitation
Vue does not fire a watcher when a reactive value is set to the value it
already holds, and the unlock that releases the "busy elsewhere" lock runs
from a watcher on the flow status. When the polled status is identical to what
the component held at the moment the lock was taken, the unlock cannot fire
from that path. That state is reachable when a lock is held from outside the
app while the flow itself sits idle, which is the manual Redis reproduction
above and also what a stale production lock looks like. This was not verified
by a test: an attempt to pin it down was inconclusive because thread hydration
overwrites the flow status from the mocked events response partway through the
example, so it is reported from reading the code rather than confirmed by a
spec. It is left alone because moving the unlock into the Apollo query's
result() hook would fire it on every poll regardless of change, but with a
cache-first fetch policy the skip toggle refetches instantly from cache,
producing an unlock, drain, reject, relock cycle with a flickering notice;
pacing that correctly means separating "the flow looks free" from "we hold the
lock", which is a refactor of the lock state machine rather than a bug fix. It
belongs with the upstream stale-lock issue,
gitlab-org/modelops/applied-ml/code-suggestions/ai-assist#2575,
and the watcher-based unlock predates this MR.
Notes for reviewers
Test fixture corrections
Writing the regression tests surfaced two pre-existing problems in
duo_agentic_chat_state_manager_spec.js:
- Two
beforeEachblocks calledactionSpies.addDuoChatMessage.mockReset(). The spies are shared for the whole file andmockResetstrips the implementation they are defined with, so every later example ran without a working message store. Changed tomockClear(). - Two flow-status fixtures used values that are not in the
DuoWorkflowStatusGraphQL enum ('completed'and lowercase'input_required'). Replaced with the real constants.
One existing example, "sends the next queued prompt once the flow unlocks and is idle", was updated: the requeued prompt is now ahead of the later one in the queue, so it goes out first.
The second commit also renames the status constant the first commit
introduced, DUO_WORKFLOW_UNATTACHED_STATUSES becomes
DUO_WORKFLOW_ATTACHED_STATUSES with an inverted list, so reviewing the two
commits separately will show the allow-list before the deny-list replaces it.