From 9d28026b1c5bdf6a0f601170d9de3dfa35017f77 Mon Sep 17 00:00:00 2001 From: Quay Robot Date: Tue, 12 May 2026 13:09:32 +0000 Subject: [PATCH 1/3] fix(backend): remove initialPrompt skip that blocked display name fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sessions intermittently showed raw session IDs instead of friendly display names. The AG-UI proxy's triggerDisplayNameGenerationIfNeeded skipped messages matching initialPrompt (agui_proxy.go:1005-1009), which was intended to prevent double-generation since CreateSession already fires GenerateDisplayNameAsync for initialPrompt sessions. However, this also blocked the fallback path: when CreateSession's async generation failed (API timeout, Vertex errors), Path B in the AG-UI proxy couldn't retry because it skipped the very message it needed to generate from. The skip was redundant — two existing guards already prevent double-generation: 1. ShouldGenerateDisplayName() returns false if displayName is already set 2. updateSessionDisplayNameInternal() has a first-write-wins race guard Removing the skip lets the AG-UI proxy naturally retry display name generation when the runner POSTs the initial prompt to /agui/run, without risk of overwrites. Includes 4 regression tests for triggerDisplayNameGenerationIfNeeded covering: initialPrompt not skipped, skip when name already set, skip when no user message, and skip when DynamicClient is nil. ## Test plan - [x] All 26 websocket package tests pass - [x] gofmt and go vet clean - [ ] Verify in staging: create sessions with initialPrompt, confirm display name appears even when Vertex API is slow/unavailable Fixes #1561 Co-Authored-By: Claude Opus 4.6 --- components/backend/handlers/sessions.go | 5 +- components/backend/websocket/agui_proxy.go | 9 +- .../backend/websocket/agui_proxy_test.go | 137 ++++++++++++++++++ 3 files changed, 142 insertions(+), 9 deletions(-) mode change 100644 => 100755 components/backend/websocket/agui_proxy.go mode change 100644 => 100755 components/backend/websocket/agui_proxy_test.go diff --git a/components/backend/handlers/sessions.go b/components/backend/handlers/sessions.go index f49613ee5..a97a03601 100755 --- a/components/backend/handlers/sessions.go +++ b/components/backend/handlers/sessions.go @@ -1203,9 +1203,8 @@ func CreateSession(c *gin.Context) { // This ensures consistent behavior whether sessions are created via API or kubectl. // Trigger async display name generation when initialPrompt is provided - // but no explicit displayName was set. The AG-UI proxy skips the - // initialPrompt message, so sessions created with only an initialPrompt - // (e.g., from the new-session page) would never get a generated name. + // but no explicit displayName was set. The AG-UI proxy also generates + // on the first /agui/run message as a fallback if this call fails. if strings.TrimSpace(req.InitialPrompt) != "" && strings.TrimSpace(req.DisplayName) == "" { spec, ok := created.Object["spec"].(map[string]interface{}) if ok { diff --git a/components/backend/websocket/agui_proxy.go b/components/backend/websocket/agui_proxy.go old mode 100644 new mode 100755 index 2289bea15..fdea10715 --- a/components/backend/websocket/agui_proxy.go +++ b/components/backend/websocket/agui_proxy.go @@ -1002,12 +1002,9 @@ func triggerDisplayNameGenerationIfNeeded(projectName, sessionName string, messa return } - // Skip if this message is the auto-sent initialPrompt - initialPrompt, _, _ := unstructured.NestedString(spec, "initialPrompt") - if initialPrompt != "" && strings.TrimSpace(userMessage) == strings.TrimSpace(initialPrompt) { - return - } - + // Fixes #1561: removed initialPrompt skip that blocked fallback when + // CreateSession's async generation failed. ShouldGenerateDisplayName + // already prevents double-generation by checking if displayName is set. if !handlers.ShouldGenerateDisplayName(spec) { return } diff --git a/components/backend/websocket/agui_proxy_test.go b/components/backend/websocket/agui_proxy_test.go old mode 100644 new mode 100755 index f54c4f1bc..6bc1441f5 --- a/components/backend/websocket/agui_proxy_test.go +++ b/components/backend/websocket/agui_proxy_test.go @@ -1,15 +1,22 @@ package websocket import ( + "context" "fmt" "net/http" "net/http/httptest" "testing" + "time" "ambient-code-backend/handlers" + "ambient-code-backend/tests/test_utils" + "ambient-code-backend/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" k8sfake "k8s.io/client-go/kubernetes/fake" ) @@ -204,3 +211,133 @@ func TestDefaultRunnerPort_Constant(t *testing.T) { t.Errorf("Expected DefaultRunnerPort=8001, got %d", handlers.DefaultRunnerPort) } } + +// --- triggerDisplayNameGenerationIfNeeded tests (regression for #1561) --- + +func setupDisplayNameTest(t *testing.T, spec map[string]interface{}) (cleanup func()) { + t.Helper() + + oldDynamic := handlers.DynamicClient + oldK8sProjects := handlers.K8sClientProjects + oldGVRFunc := handlers.GetAgenticSessionV1Alpha1Resource + + agenticSessionGVR := schema.GroupVersionResource{ + Group: "vteam.ambient-code", + Version: "v1alpha1", + Resource: "agenticsessions", + } + handlers.GetAgenticSessionV1Alpha1Resource = func() schema.GroupVersionResource { + return agenticSessionGVR + } + + fakeClients := test_utils.NewFakeClientSet() + handlers.DynamicClient = fakeClients.GetDynamicClient() + handlers.K8sClientProjects = fakeClients.GetK8sClient() + + err := test_utils.CreateAgenticSessionInFakeClient( + handlers.DynamicClient, "test-project", "test-session", spec, + ) + if err != nil { + t.Fatalf("Failed to create test session: %v", err) + } + + return func() { + handlers.DynamicClient = oldDynamic + handlers.K8sClientProjects = oldK8sProjects + handlers.GetAgenticSessionV1Alpha1Resource = oldGVRFunc + } +} + +func getDisplayName(t *testing.T, dc dynamic.Interface) string { + t.Helper() + gvr := handlers.GetAgenticSessionV1Alpha1Resource() + item, err := dc.Resource(gvr).Namespace("test-project").Get( + context.Background(), "test-session", metav1.GetOptions{}, + ) + if err != nil { + t.Fatalf("Failed to get session: %v", err) + } + dn, _, _ := unstructured.NestedString(item.Object, "spec", "displayName") + return dn +} + +func TestTriggerDisplayName_InitialPromptNotSkipped(t *testing.T) { + // Regression test for #1561: messages matching initialPrompt must NOT + // be skipped when displayName is empty. Before the fix, the AG-UI + // proxy returned early for initialPrompt messages, blocking the + // fallback path when CreateSession's async generation failed. + cleanup := setupDisplayNameTest(t, map[string]interface{}{ + "initialPrompt": "Help me debug auth", + }) + defer cleanup() + + msgs := []types.Message{ + {ID: "msg-1", Role: "user", Content: "Help me debug auth"}, + } + + // Should not panic or skip — it will attempt GenerateDisplayNameAsync + // which will fail (no API key) but that's expected in test env. + triggerDisplayNameGenerationIfNeeded("test-project", "test-session", msgs) + + // Give the async goroutine a moment to attempt the update + time.Sleep(50 * time.Millisecond) + + // The display name won't be set (no API key), but the function + // must not have returned early at the (now-removed) initialPrompt check. + // We verify this indirectly: if the function still had the skip, + // it would never call GenerateDisplayNameAsync. The absence of a + // panic and the function completing is the primary assertion. +} + +func TestTriggerDisplayName_SkipsWhenNameAlreadySet(t *testing.T) { + cleanup := setupDisplayNameTest(t, map[string]interface{}{ + "initialPrompt": "Help me debug auth", + "displayName": "Debug Auth Middleware", + }) + defer cleanup() + + msgs := []types.Message{ + {ID: "msg-1", Role: "user", Content: "Help me debug auth"}, + } + + triggerDisplayNameGenerationIfNeeded("test-project", "test-session", msgs) + + // displayName should remain unchanged — ShouldGenerateDisplayName + // returns false when displayName is already set. + dn := getDisplayName(t, handlers.DynamicClient) + if dn != "Debug Auth Middleware" { + t.Errorf("Expected displayName to remain %q, got %q", "Debug Auth Middleware", dn) + } +} + +func TestTriggerDisplayName_SkipsWhenNoUserMessage(t *testing.T) { + cleanup := setupDisplayNameTest(t, map[string]interface{}{ + "initialPrompt": "Help me debug auth", + }) + defer cleanup() + + // Only assistant messages — no user content to generate from + msgs := []types.Message{ + {ID: "msg-1", Role: "assistant", Content: "I'll help you debug"}, + } + + triggerDisplayNameGenerationIfNeeded("test-project", "test-session", msgs) + + dn := getDisplayName(t, handlers.DynamicClient) + if dn != "" { + t.Errorf("Expected empty displayName, got %q", dn) + } +} + +func TestTriggerDisplayName_SkipsWhenDynamicClientNil(t *testing.T) { + oldDynamic := handlers.DynamicClient + handlers.DynamicClient = nil + defer func() { handlers.DynamicClient = oldDynamic }() + + msgs := []types.Message{ + {ID: "msg-1", Role: "user", Content: "Help me debug auth"}, + } + + // Should return early without panic when DynamicClient is nil + triggerDisplayNameGenerationIfNeeded("test-project", "test-session", msgs) +} From 066ec483c0e041fee7ded2067ba9305e2413789e Mon Sep 17 00:00:00 2001 From: Quay Robot Date: Tue, 12 May 2026 13:12:27 +0000 Subject: [PATCH 2/3] fix(backend): remove issue reference comment from agui_proxy.go Co-Authored-By: Claude Opus 4.6 --- components/backend/websocket/agui_proxy.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/components/backend/websocket/agui_proxy.go b/components/backend/websocket/agui_proxy.go index fdea10715..441ab7f53 100755 --- a/components/backend/websocket/agui_proxy.go +++ b/components/backend/websocket/agui_proxy.go @@ -1002,9 +1002,6 @@ func triggerDisplayNameGenerationIfNeeded(projectName, sessionName string, messa return } - // Fixes #1561: removed initialPrompt skip that blocked fallback when - // CreateSession's async generation failed. ShouldGenerateDisplayName - // already prevents double-generation by checking if displayName is set. if !handlers.ShouldGenerateDisplayName(spec) { return } From 8cb172b062e439919f754cba49169051c64a0acc Mon Sep 17 00:00:00 2001 From: Quay Robot Date: Tue, 12 May 2026 13:17:17 +0000 Subject: [PATCH 3/3] test(backend): strengthen regression test with function variable mock Make GenerateDisplayNameAsync a package-level function variable (matching the existing pattern used for DynamicClient, GetGitHubToken, etc.) so the regression test can swap it and assert it was actually called. Co-Authored-By: Claude Opus 4.6 --- components/backend/handlers/display_name.go | 2 +- .../backend/websocket/agui_proxy_test.go | 25 ++++++++----------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/components/backend/handlers/display_name.go b/components/backend/handlers/display_name.go index 3b3bc9524..90fd11133 100755 --- a/components/backend/handlers/display_name.go +++ b/components/backend/handlers/display_name.go @@ -80,7 +80,7 @@ func ValidateDisplayName(name string) string { // - Gracefully handles session deletion during generation (checks IsNotFound) // - No cancellation mechanism exists; goroutine runs to completion or timeout // - Safe for backend restarts: orphaned goroutines will timeout naturally -func GenerateDisplayNameAsync(projectName, sessionName, userMessage string, sessionCtx SessionContext) { +var GenerateDisplayNameAsync = func(projectName, sessionName, userMessage string, sessionCtx SessionContext) { go func() { defer func() { if r := recover(); r != nil { diff --git a/components/backend/websocket/agui_proxy_test.go b/components/backend/websocket/agui_proxy_test.go index 6bc1441f5..7e8c0c129 100755 --- a/components/backend/websocket/agui_proxy_test.go +++ b/components/backend/websocket/agui_proxy_test.go @@ -6,7 +6,6 @@ import ( "net/http" "net/http/httptest" "testing" - "time" "ambient-code-backend/handlers" "ambient-code-backend/tests/test_utils" @@ -262,31 +261,27 @@ func getDisplayName(t *testing.T, dc dynamic.Interface) string { } func TestTriggerDisplayName_InitialPromptNotSkipped(t *testing.T) { - // Regression test for #1561: messages matching initialPrompt must NOT - // be skipped when displayName is empty. Before the fix, the AG-UI - // proxy returned early for initialPrompt messages, blocking the - // fallback path when CreateSession's async generation failed. cleanup := setupDisplayNameTest(t, map[string]interface{}{ "initialPrompt": "Help me debug auth", }) defer cleanup() + called := false + oldFn := handlers.GenerateDisplayNameAsync + handlers.GenerateDisplayNameAsync = func(projectName, sessionName, userMessage string, sessionCtx handlers.SessionContext) { + called = true + } + defer func() { handlers.GenerateDisplayNameAsync = oldFn }() + msgs := []types.Message{ {ID: "msg-1", Role: "user", Content: "Help me debug auth"}, } - // Should not panic or skip — it will attempt GenerateDisplayNameAsync - // which will fail (no API key) but that's expected in test env. triggerDisplayNameGenerationIfNeeded("test-project", "test-session", msgs) - // Give the async goroutine a moment to attempt the update - time.Sleep(50 * time.Millisecond) - - // The display name won't be set (no API key), but the function - // must not have returned early at the (now-removed) initialPrompt check. - // We verify this indirectly: if the function still had the skip, - // it would never call GenerateDisplayNameAsync. The absence of a - // panic and the function completing is the primary assertion. + if !called { + t.Error("Expected GenerateDisplayNameAsync to be called for initialPrompt message when displayName is empty") + } } func TestTriggerDisplayName_SkipsWhenNameAlreadySet(t *testing.T) {