From 6c62d8f5e6c362bec23411fd67c26799a44405f9 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 6 Apr 2026 22:04:48 -0400 Subject: [PATCH] fix(coderd/x/chatd): fix flaky TestAwaitSubagentCompletion/CompletesViaPubsub (#24066) ## Fix flaky TestAwaitSubagentCompletion/CompletesViaPubsub Fixes coder/internal#1435 ### Root Cause During `createParentChildChats`, the processor publishes notifications on `ChatStreamNotifyChannel(child.ID)` via PostgreSQL `LISTEN/NOTIFY`. After `drainInflight()` returns, these stale notifications can still be buffered in the pgListener's `NotifyChan()`. When `awaitSubagentCompletion` subscribes and a stale notification is dispatched between `setChatStatus(Waiting)` and `insertAssistantMessage`, `checkSubagentCompletion` sees `done=true` (status is `Waiting`) but returns an empty report because the message hasn't been committed yet. ### Fix Swap the order: insert the assistant message **before** transitioning the status to `Waiting`. This guarantees the report is committed before the status makes the chat appear complete to `checkSubagentCompletion`. ### Verification - 50 consecutive runs of the specific test: all pass - 10 runs of the full `TestAwaitSubagentCompletion` suite: all pass - 20 runs with `-race`: all pass > Generated by Coder Agents --- coderd/x/chatd/subagent_internal_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/coderd/x/chatd/subagent_internal_test.go b/coderd/x/chatd/subagent_internal_test.go index b61276d478..144addbd08 100644 --- a/coderd/x/chatd/subagent_internal_test.go +++ b/coderd/x/chatd/subagent_internal_test.go @@ -1055,12 +1055,17 @@ func TestAwaitSubagentCompletion(t *testing.T) { require.NoError(t, err) defer cancelProbe() - // Transition the child first, then publish once the - // durable completion state is observable. Pubsub only - // wakes the waiter; it does not guarantee the report is - // visible in the same instant as the notification. - setChatStatus(ctx, t, db, child.ID, database.ChatStatusWaiting, "") + // Insert the message BEFORE transitioning to Waiting. + // Stale PG LISTEN/NOTIFY notifications from the + // processor's earlier run can still be buffered in the + // pgListener after drainInflight returns. If such a + // notification is dispatched between setChatStatus and + // insertAssistantMessage, checkSubagentCompletion would + // see done=true (Waiting) with an empty report. By + // inserting the message first, the report is guaranteed + // to be committed before the status makes it visible. insertAssistantMessage(ctx, t, db, child.ID, model.ID, "pubsub result") + setChatStatus(ctx, t, db, child.ID, database.ChatStatusWaiting, "") require.EventuallyWithT(t, func(c *assert.CollectT) { chat, report, done, err := server.checkSubagentCompletion(ctx, child.ID) require.NoError(c, err)