From a59b9515655cbb9fbc04ed2304594f634660cbe1 Mon Sep 17 00:00:00 2001 From: Ethan Date: Fri, 15 May 2026 17:36:48 +1000 Subject: [PATCH] test: skip stale notification chatd flakes (#25376) These chatd tests are flaking for the same stale control-notification race tracked by CODAGT-353, so this change skips the newly reflaking advisor-chain and `TestPatchChatMessage/ChangesModel` tests and rewrites the older `TODO(hugodutka)` skips to point at the same root cause. This keeps the known flakes documented consistently until the chatd notification-flow refactor lands. Closes CODAGT-427 Closes https://github.com/coder/internal/issues/1510 --- coderd/exp_chats_test.go | 8 ++++++++ coderd/x/chatd/chatd_test.go | 14 ++++++++++++-- enterprise/coderd/x/chatd/chatd_test.go | 8 ++++++-- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index 1ddd86be8c..61720a426c 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -7761,6 +7761,14 @@ func TestPatchChatMessage(t *testing.T) { t.Run("ChangesModel", func(t *testing.T) { t.Parallel() + // TODO(CODAGT-353): Re-enable this test after the chatd notification flow + // refactor gives workers enough causal information to distinguish stale + // control NOTIFY messages from real interrupts. The current design reuses + // the same status notification shape for wake-only and interrupt intents, + // so a stale NOTIFY can cancel a new processChat run. This subtest hits the + // same root cause via the persistInterruptedStep ownership gate, where a + // late insert from the previous turn regresses chats.last_model_config_id. + t.Skip("skipped until chatd notification flow refactor handles stale control notifications") ctx := testutil.Context(t, testutil.WaitLong) client := newChatClient(t) diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index e8430ee232..ed90c601d5 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -1100,8 +1100,12 @@ func TestRootExploreChatExcludesWebSearchProviderToolAtRuntime(t *testing.T) { func TestExploreChatSendMessageCannotMutateMCPSnapshot(t *testing.T) { t.Parallel() - // TODO(hugodutka): Unskip when chatd is free of race conditions. - t.Skip("skipped due to inherent race condition; see https://github.com/coder/internal/issues/1493") + // TODO(CODAGT-353): Re-enable this test after the chatd notification flow + // refactor gives workers enough causal information to distinguish stale + // control NOTIFY messages from real interrupts. The current design reuses + // the same status notification shape for wake-only and interrupt intents, + // so a stale NOTIFY can cancel a new processChat run. + t.Skip("skipped until chatd notification flow refactor handles stale control notifications") db, ps := dbtestutil.NewDB(t) ctx := testutil.Context(t, testutil.WaitLong) @@ -10104,6 +10108,12 @@ func TestAdvisorGating_ExploreSubagent(t *testing.T) { // message, losing the context the outer model had been building on. func TestAdvisorChainMode_SnapshotKeepsFullHistory(t *testing.T) { t.Parallel() + // TODO(CODAGT-353): Re-enable this test after the chatd notification flow + // refactor gives workers enough causal information to distinguish stale + // control NOTIFY messages from real interrupts. The current design reuses + // the same status notification shape for wake-only and interrupt intents, + // so a stale NOTIFY can cancel a new processChat run. + t.Skip("skipped until chatd notification flow refactor handles stale control notifications") db, ps := dbtestutil.NewDB(t) ctx := testutil.Context(t, testutil.WaitLong) diff --git a/enterprise/coderd/x/chatd/chatd_test.go b/enterprise/coderd/x/chatd/chatd_test.go index 97b63cba03..f94df3be8d 100644 --- a/enterprise/coderd/x/chatd/chatd_test.go +++ b/enterprise/coderd/x/chatd/chatd_test.go @@ -1426,8 +1426,12 @@ func TestSubscribeRelayDialCanceledOnFastCompletion(t *testing.T) { // condition where the relay is too slow. func TestSubscribeRelayEstablishedMidStream(t *testing.T) { t.Parallel() - // TODO(hugodutka): Unskip when chatd is free of race conditions. - t.Skip("skipped due to inherent race condition; see https://github.com/coder/internal/issues/1455") + // TODO(CODAGT-353): Re-enable this test after the chatd notification flow + // refactor gives workers enough causal information to distinguish stale + // control NOTIFY messages from real interrupts. The current design reuses + // the same status notification shape for wake-only and interrupt intents, + // so a stale NOTIFY can cancel a new processChat run. + t.Skip("skipped until chatd notification flow refactor handles stale control notifications") db, ps := dbtestutil.NewDB(t) workerID := uuid.New()