mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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
This commit is contained in:
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user