From dcde3da74c6705983549399dead59c35266fff33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sun, 31 May 2026 11:14:23 +0100 Subject: [PATCH] feat: surface an inline notice when an agent turn produces no output (#1179) --- apps/backend/cmd/mock-agent/scenarios.go | 14 + .../task/handlers/process_handlers_test.go | 3 + .../internal/task/repository/interface.go | 1 + .../task/repository/sqlite/message.go | 38 +-- .../task/repository/sqlite/message_test.go | 38 +++ .../internal/task/service/service_turns.go | 73 ++++- .../task/service/service_turns_output_test.go | 262 ++++++++++++++++++ apps/web/e2e/tests/chat/empty-turn.spec.ts | 78 ++++++ .../e2e/tests/chat/mobile-empty-turn.spec.ts | 52 ++++ apps/web/lib/types/backend.ts | 2 + .../lib/ws/handlers/empty-turn-notice.test.ts | 161 +++++++++++ apps/web/lib/ws/handlers/empty-turn-notice.ts | 132 +++++++++ apps/web/lib/ws/handlers/turns.ts | 3 + docs/specs/INDEX.md | 1 + docs/specs/ui/empty-turn-notice.md | 49 ++++ 15 files changed, 877 insertions(+), 30 deletions(-) create mode 100644 apps/backend/internal/task/service/service_turns_output_test.go create mode 100644 apps/web/e2e/tests/chat/empty-turn.spec.ts create mode 100644 apps/web/e2e/tests/chat/mobile-empty-turn.spec.ts create mode 100644 apps/web/lib/ws/handlers/empty-turn-notice.test.ts create mode 100644 apps/web/lib/ws/handlers/empty-turn-notice.ts create mode 100644 docs/specs/ui/empty-turn-notice.md diff --git a/apps/backend/cmd/mock-agent/scenarios.go b/apps/backend/cmd/mock-agent/scenarios.go index 13bfccb9fb..f60132a309 100644 --- a/apps/backend/cmd/mock-agent/scenarios.go +++ b/apps/backend/cmd/mock-agent/scenarios.go @@ -38,6 +38,20 @@ var scenarioRegistry = map[string]func(e *emitter){ "review-cumulative-setup": scenarioReviewCumulativeSetup, "symlink-file-setup": scenarioSymlinkFileSetup, "markdown-table": scenarioMarkdownTable, + "empty-turn": scenarioEmptyTurn, +} + +// scenarioEmptyTurn emits no content and no tool calls, so the turn ends +// cleanly with no agent output. Reproduces the case where an agent treats a +// prompt (e.g. an unsupported slash command) as a no-op and returns an empty +// end_turn. Drives the frontend "empty turn" notice. +func scenarioEmptyTurn(e *emitter) { + _ = e + // Stay "running" for a few seconds rather than returning instantly. The + // empty-turn notice is driven by the live turn.completed event, so the turn + // must outlast the client's initial WS subscribe (especially on mobile, + // where a fast auto-start turn can finish before the chat subscribes). + fixedDelay(3000) } // emitPredefinedScenario dispatches to a named e2e scenario. diff --git a/apps/backend/internal/task/handlers/process_handlers_test.go b/apps/backend/internal/task/handlers/process_handlers_test.go index beb8582e92..5f4a884ab0 100644 --- a/apps/backend/internal/task/handlers/process_handlers_test.go +++ b/apps/backend/internal/task/handlers/process_handlers_test.go @@ -187,6 +187,9 @@ func (m *mockRepository) UpdateMessage(ctx context.Context, message *models.Mess func (m *mockRepository) ListMessages(ctx context.Context, sessionID string) ([]*models.Message, error) { return nil, nil } +func (m *mockRepository) ListMessagesByTurnID(ctx context.Context, turnID string) ([]*models.Message, error) { + return nil, nil +} func (m *mockRepository) ListMessagesPaginated(ctx context.Context, sessionID string, opts models.ListMessagesOptions) ([]*models.Message, bool, error) { return nil, false, nil } diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index b9ad14800f..003932993a 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -104,6 +104,7 @@ type MessageRepository interface { FindMessageByPendingIDAndQuestion(ctx context.Context, sessionID, pendingID, questionID string) (*models.Message, error) UpdateMessage(ctx context.Context, message *models.Message) error ListMessages(ctx context.Context, sessionID string) ([]*models.Message, error) + ListMessagesByTurnID(ctx context.Context, turnID string) ([]*models.Message, error) ListMessagesPaginated(ctx context.Context, sessionID string, opts models.ListMessagesOptions) ([]*models.Message, bool, error) SearchMessages(ctx context.Context, sessionID string, opts models.SearchMessagesOptions) ([]*models.Message, error) DeleteMessage(ctx context.Context, id string) error diff --git a/apps/backend/internal/task/repository/sqlite/message.go b/apps/backend/internal/task/repository/sqlite/message.go index 7ce576ad3e..2c8c7972a0 100644 --- a/apps/backend/internal/task/repository/sqlite/message.go +++ b/apps/backend/internal/task/repository/sqlite/message.go @@ -90,32 +90,24 @@ func (r *Repository) ListMessages(ctx context.Context, sessionID string) ([]*mod return nil, err } defer func() { _ = rows.Close() }() + msgs, _, err := scanMessageRows(rows, 0) + return msgs, err +} - var result []*models.Message - for rows.Next() { - message := &models.Message{} - var requestsInput int - var messageType string - var metadataJSON string - err := rows.Scan(&message.ID, &message.TaskSessionID, &message.TaskID, &message.TurnID, &message.AuthorType, &message.AuthorID, &message.Content, &requestsInput, &messageType, &metadataJSON, &message.CreatedAt) - if err != nil { - return nil, err - } - message.RequestsInput = requestsInput == 1 - message.Type = models.MessageType(messageType) - - if metadataJSON != "" && metadataJSON != "{}" { - if err := json.Unmarshal([]byte(metadataJSON), &message.Metadata); err != nil { - return nil, fmt.Errorf("failed to deserialize message metadata: %w", err) - } - } - - result = append(result, message) - } - if err := rows.Err(); err != nil { +// ListMessagesByTurnID returns all messages for a single turn ordered by +// creation time. Backed by idx_messages_turn_id, so it reads only the turn's +// own rows rather than the whole session's history. +func (r *Repository) ListMessagesByTurnID(ctx context.Context, turnID string) ([]*models.Message, error) { + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(` + SELECT id, task_session_id, task_id, turn_id, author_type, author_id, content, requests_input, type, metadata, created_at + FROM task_session_messages WHERE turn_id = ? ORDER BY created_at ASC + `), turnID) + if err != nil { return nil, err } - return result, nil + defer func() { _ = rows.Close() }() + msgs, _, err := scanMessageRows(rows, 0) + return msgs, err } // ListMessagesPaginated returns messages for a session ordered by creation time with pagination. diff --git a/apps/backend/internal/task/repository/sqlite/message_test.go b/apps/backend/internal/task/repository/sqlite/message_test.go index e851fe98a2..1fd21e5863 100644 --- a/apps/backend/internal/task/repository/sqlite/message_test.go +++ b/apps/backend/internal/task/repository/sqlite/message_test.go @@ -20,6 +20,44 @@ func insertMsgWithType(t *testing.T, repo *Repository, id, sessionID, turnID, ms } } +func TestListMessagesByTurnID(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + now := time.Now().UTC() + seedForMsgTest(t, repo, "task-T", "sess-T", "turn-1") + seedForMsgTest(t, repo, "task-T2", "sess-T", "turn-2") + + // Two messages on turn-1 (out of insertion order to check created_at sort) + // and one on turn-2 in the same session. + insertMsgWithType(t, repo, "m-b", "sess-T", "turn-1", "message", now.Add(2*time.Second)) + insertMsgWithType(t, repo, "m-a", "sess-T", "turn-1", "tool_call", now) + insertMsgWithType(t, repo, "m-other", "sess-T", "turn-2", "message", now.Add(time.Second)) + + got, err := repo.ListMessagesByTurnID(ctx, "turn-1") + if err != nil { + t.Fatalf("ListMessagesByTurnID: %v", err) + } + if len(got) != 2 { + t.Fatalf("expected 2 messages for turn-1, got %d", len(got)) + } + if got[0].ID != "m-a" || got[1].ID != "m-b" { + t.Errorf("expected [m-a, m-b] ordered by created_at, got [%s, %s]", got[0].ID, got[1].ID) + } + for _, m := range got { + if m.TurnID != "turn-1" { + t.Errorf("message %s has turn_id %q, want turn-1", m.ID, m.TurnID) + } + } + + empty, err := repo.ListMessagesByTurnID(ctx, "turn-missing") + if err != nil { + t.Fatalf("ListMessagesByTurnID(missing): %v", err) + } + if len(empty) != 0 { + t.Errorf("expected no messages for unknown turn, got %d", len(empty)) + } +} + func TestCountToolCallMessagesBySession_Empty(t *testing.T) { repo := newRepoForSessionTests(t) got, err := repo.CountToolCallMessagesBySession(context.Background(), nil) diff --git a/apps/backend/internal/task/service/service_turns.go b/apps/backend/internal/task/service/service_turns.go index eaaa21107c..9ab3f4ef67 100644 --- a/apps/backend/internal/task/service/service_turns.go +++ b/apps/backend/internal/task/service/service_turns.go @@ -7,6 +7,7 @@ import ( "fmt" "path/filepath" "sort" + "strings" "time" "github.com/google/uuid" @@ -42,7 +43,8 @@ func (s *Service) StartTurn(ctx context.Context, sessionID string) (*models.Turn return nil, err } - s.publishTurnEvent(events.TurnStarted, turn) + // had_output is only meaningful on turn.completed; omit it from turn.started. + s.publishTurnEvent(events.TurnStarted, turn, nil) s.logger.Debug("started turn", zap.String("turn_id", turn.ID), @@ -80,7 +82,8 @@ func (s *Service) CompleteTurn(ctx context.Context, turnID string) error { return nil } - s.publishTurnEvent(events.TurnCompleted, turn) + hadOutput := s.turnHadOutput(ctx, turn) + s.publishTurnEvent(events.TurnCompleted, turn, &hadOutput) s.logger.Debug("completed turn", zap.String("turn_id", turnID), @@ -148,7 +151,11 @@ func (s *Service) AbandonOpenTurns(ctx context.Context, sessionID string) error zap.String("turn_id", turn.ID), zap.Error(err)) } else { - s.publishTurnEvent(events.TurnCompleted, refreshed) + // Abandoned turns are orphans swept on resume, not live completions. + // Report had_output=true so the frontend never shows an "empty turn" + // notice for them — only genuine live completions should trigger it. + hadOutput := true + s.publishTurnEvent(events.TurnCompleted, refreshed, &hadOutput) } s.logger.Info("abandoned orphan turn on session resume", zap.String("turn_id", turn.ID), @@ -187,8 +194,12 @@ func (s *Service) getOrStartTurn(ctx context.Context, sessionID string) (*models return s.StartTurn(ctx, sessionID) } -// publishTurnEvent publishes a turn event to the event bus. -func (s *Service) publishTurnEvent(eventType string, turn *models.Turn) { +// publishTurnEvent publishes a turn event to the event bus. hadOutput reports +// whether the turn produced any agent output; it is only meaningful for +// turn.completed events (the frontend uses it to surface an "empty turn" +// notice). Pass nil for turn.started so the field is omitted entirely rather +// than carrying a misleading "false" on a turn that has not completed. +func (s *Service) publishTurnEvent(eventType string, turn *models.Turn, hadOutput *bool) { if s.eventBus == nil { return } @@ -196,7 +207,7 @@ func (s *Service) publishTurnEvent(eventType string, turn *models.Turn) { s.logger.Warn("publishTurnEvent: turn is nil, skipping", zap.String("event_type", eventType)) return } - _ = s.eventBus.Publish(context.Background(), eventType, bus.NewEvent(eventType, "task-service", map[string]interface{}{ + payload := map[string]interface{}{ "id": turn.ID, "session_id": turn.TaskSessionID, "task_id": turn.TaskID, @@ -205,7 +216,55 @@ func (s *Service) publishTurnEvent(eventType string, turn *models.Turn) { "metadata": turn.Metadata, "created_at": turn.CreatedAt, "updated_at": turn.UpdatedAt, - })) + } + if hadOutput != nil { + payload["had_output"] = *hadOutput + } + _ = s.eventBus.Publish(context.Background(), eventType, bus.NewEvent(eventType, "task-service", payload)) +} + +// turnHadOutput reports whether a completed turn produced any agent output. +// It reads only the turn's own persisted messages (indexed by turn_id; all +// agent text/tool messages are written before the turn is completed, so the DB +// is authoritative even for streaming agents whose text is drained via chunk +// events). A read failure defaults to true so a transient DB error never +// produces a spurious "empty turn" notice. +func (s *Service) turnHadOutput(ctx context.Context, turn *models.Turn) bool { + msgs, err := s.messages.ListMessagesByTurnID(ctx, turn.ID) + if err != nil { + s.logger.Debug("failed to list messages for had_output; assuming output", + zap.String("turn_id", turn.ID), + zap.Error(err)) + return true + } + return turnHadAgentOutput(msgs, turn.ID) +} + +// turnHadAgentOutput returns true when any message belonging to turnID is +// agent-authored, user-visible output: a tool call, a native plan/todo, a +// permission or clarification prompt (both render inline in chat), or a +// non-empty text response. It is an allowlist on purpose — incidental, +// non-answer messages the runtime attaches to a turn (lifecycle "status" / +// "script_execution" notices, logs, progress, thinking) must NOT count, so a +// turn that only emits those (or nothing at all) is still treated as empty. +// User messages never count. +func turnHadAgentOutput(msgs []*models.Message, turnID string) bool { + for _, m := range msgs { + if m == nil || m.TurnID != turnID || m.AuthorType != models.MessageAuthorAgent { + continue + } + switch m.Type { + case models.MessageTypeToolCall, models.MessageTypeToolEdit, models.MessageTypeToolRead, + models.MessageTypeToolExecute, models.MessageTypeAgentPlan, models.MessageTypeTodo, + models.MessageTypePermissionRequest, models.MessageTypeClarificationRequest: + return true + case models.MessageTypeMessage, models.MessageTypeContent, "": + if strings.TrimSpace(m.Content) != "" { + return true + } + } + } + return false } // GetGitSnapshots retrieves git snapshots for a session diff --git a/apps/backend/internal/task/service/service_turns_output_test.go b/apps/backend/internal/task/service/service_turns_output_test.go new file mode 100644 index 0000000000..98dfdf683d --- /dev/null +++ b/apps/backend/internal/task/service/service_turns_output_test.go @@ -0,0 +1,262 @@ +package service + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/task/models" +) + +// lastTurnCompletedHadOutput returns the had_output flag of the most recently +// published turn.completed event, and whether such an event was found. +func lastTurnCompletedHadOutput(t *testing.T, eventBus *MockEventBus) (bool, bool) { + t.Helper() + published := eventBus.GetPublishedEvents() + for i := len(published) - 1; i >= 0; i-- { + ev := published[i] + if ev.Type != events.TurnCompleted { + continue + } + data, ok := ev.Data.(map[string]interface{}) + if !ok { + t.Fatalf("turn.completed event data is not a map: %T", ev.Data) + } + hadOutput, ok := data["had_output"].(bool) + if !ok { + t.Fatalf("turn.completed event missing had_output bool: %v", data["had_output"]) + } + return hadOutput, true + } + return false, false +} + +func TestCompleteTurn_PublishesHadOutput(t *testing.T) { + tests := []struct { + name string + msgs []*models.Message + want bool + }{ + { + name: "empty turn with only launch notices", + msgs: []*models.Message{ + {Type: models.MessageTypeScriptExecution, Content: "Environment prepared"}, + {Type: models.MessageTypeStatus, Content: "Started agent Mock"}, + }, + want: false, + }, + { + name: "turn with an agent text response", + msgs: []*models.Message{ + {Type: models.MessageTypeMessage, Content: "Here is the answer"}, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, eventBus, repo := createTestService(t) + ctx := context.Background() + setupTestTask(t, repo) + sessionID := setupTestSession(t, repo) + + turn := &models.Turn{ + ID: "turn-output", + TaskSessionID: sessionID, + TaskID: "task-123", + StartedAt: time.Now().UTC(), + } + if err := repo.CreateTurn(ctx, turn); err != nil { + t.Fatalf("CreateTurn: %v", err) + } + for i, m := range tt.msgs { + m.ID = fmt.Sprintf("msg-%d", i) + m.TaskSessionID = sessionID + m.TaskID = "task-123" + m.TurnID = turn.ID + m.AuthorType = models.MessageAuthorAgent + if err := repo.CreateMessage(ctx, m); err != nil { + t.Fatalf("CreateMessage: %v", err) + } + } + + eventBus.ClearEvents() + if err := svc.CompleteTurn(ctx, turn.ID); err != nil { + t.Fatalf("CompleteTurn: %v", err) + } + + got, found := lastTurnCompletedHadOutput(t, eventBus) + if !found { + t.Fatal("expected a turn.completed event to be published") + } + if got != tt.want { + t.Errorf("had_output = %v, want %v", got, tt.want) + } + }) + } +} + +// AbandonOpenTurns sweeps orphan turns on resume; it must report had_output=true +// so the frontend never shows an empty-turn notice for a swept orphan. +func TestAbandonOpenTurns_PublishesHadOutputTrue(t *testing.T) { + svc, eventBus, repo := createTestService(t) + ctx := context.Background() + setupTestTask(t, repo) + sessionID := setupTestSession(t, repo) + + turn := &models.Turn{ + ID: "turn-orphan", + TaskSessionID: sessionID, + TaskID: "task-123", + StartedAt: time.Now().UTC(), + } + if err := repo.CreateTurn(ctx, turn); err != nil { + t.Fatalf("CreateTurn: %v", err) + } + + eventBus.ClearEvents() + if err := svc.AbandonOpenTurns(ctx, sessionID); err != nil { + t.Fatalf("AbandonOpenTurns: %v", err) + } + + got, found := lastTurnCompletedHadOutput(t, eventBus) + if !found { + t.Fatal("expected a turn.completed event to be published") + } + if !got { + t.Error("had_output = false, want true (orphan sweep must suppress the notice)") + } +} + +func TestTurnHadAgentOutput(t *testing.T) { + const turnID = "turn-1" + + agentMsg := func(t models.MessageType, content string) *models.Message { + return &models.Message{TurnID: turnID, AuthorType: models.MessageAuthorAgent, Type: t, Content: content} + } + + tests := []struct { + name string + msgs []*models.Message + want bool + }{ + { + name: "no messages", + msgs: nil, + want: false, + }, + { + name: "only a user prompt", + msgs: []*models.Message{ + {TurnID: turnID, AuthorType: models.MessageAuthorUser, Type: models.MessageTypeMessage, Content: "/pr-fixup"}, + }, + want: false, + }, + { + name: "agent text response", + msgs: []*models.Message{agentMsg(models.MessageTypeMessage, "Here is the answer")}, + want: true, + }, + { + name: "agent default-typed text (empty type)", + msgs: []*models.Message{agentMsg("", "Done")}, + want: true, + }, + { + name: "agent message with only whitespace content", + msgs: []*models.Message{agentMsg(models.MessageTypeMessage, " \n ")}, + want: false, + }, + { + name: "agent tool call", + msgs: []*models.Message{agentMsg(models.MessageTypeToolCall, "")}, + want: true, + }, + { + name: "agent content chunk", + msgs: []*models.Message{agentMsg(models.MessageTypeContent, "partial")}, + want: true, + }, + { + name: "agent plan", + msgs: []*models.Message{agentMsg(models.MessageTypeAgentPlan, "")}, + want: true, + }, + { + name: "todo is visible output", + msgs: []*models.Message{agentMsg(models.MessageTypeTodo, "")}, + want: true, + }, + { + name: "a permission request is visible output", + msgs: []*models.Message{agentMsg(models.MessageTypePermissionRequest, "")}, + want: true, + }, + { + name: "a clarification request is visible output", + msgs: []*models.Message{agentMsg(models.MessageTypeClarificationRequest, "")}, + want: true, + }, + { + name: "lifecycle status notices are not output", + msgs: []*models.Message{agentMsg(models.MessageTypeStatus, "Started agent Mock")}, + want: false, + }, + { + name: "script execution notices are not output", + msgs: []*models.Message{agentMsg(models.MessageTypeScriptExecution, "Environment prepared")}, + want: false, + }, + { + name: "only thinking is not output", + msgs: []*models.Message{agentMsg(models.MessageTypeThinking, "hmm")}, + want: false, + }, + { + name: "only a log line is not output", + msgs: []*models.Message{agentMsg(models.MessageTypeLog, "debug")}, + want: false, + }, + { + name: "only progress is not output", + msgs: []*models.Message{agentMsg(models.MessageTypeProgress, "50%")}, + want: false, + }, + { + name: "an empty first turn with only launch notices", + msgs: []*models.Message{ + {TurnID: turnID, AuthorType: models.MessageAuthorUser, Type: models.MessageTypeMessage, Content: "/e2e:empty-turn"}, + agentMsg(models.MessageTypeScriptExecution, "Environment prepared"), + agentMsg(models.MessageTypeStatus, "Started agent Mock"), + }, + want: false, + }, + { + name: "output belongs to a different turn", + msgs: []*models.Message{ + {TurnID: "other-turn", AuthorType: models.MessageAuthorAgent, Type: models.MessageTypeMessage, Content: "elsewhere"}, + }, + want: false, + }, + { + name: "noise plus real output", + msgs: []*models.Message{ + agentMsg(models.MessageTypeThinking, "let me think"), + agentMsg(models.MessageTypeLog, "log"), + agentMsg(models.MessageTypeToolCall, ""), + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := turnHadAgentOutput(tt.msgs, turnID); got != tt.want { + t.Errorf("turnHadAgentOutput() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/apps/web/e2e/tests/chat/empty-turn.spec.ts b/apps/web/e2e/tests/chat/empty-turn.spec.ts new file mode 100644 index 0000000000..0b6f6b98b0 --- /dev/null +++ b/apps/web/e2e/tests/chat/empty-turn.spec.ts @@ -0,0 +1,78 @@ +import { type Page } from "@playwright/test"; +import { test, expect } from "../../fixtures/test-base"; +import type { SeedData } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { SessionPage } from "../../pages/session-page"; + +const HINT = /without the leading slash/i; +const MOCK_OUTPUT = "This is a simple mock response for e2e testing."; + +/** + * Seed a task whose auto-started first turn runs `scenario`, then open its chat. + * The empty-turn notice is live-only, so the triggering turn is the auto-started + * one (a multi-second `/e2e:empty-turn`) — the client subscribes while it runs + * and receives the live turn.completed that drives the notice. Returns the page + * without waiting for idle, so the assertion can observe the notice as it fires. + */ +async function seedAndOpenChat( + testPage: Page, + apiClient: ApiClient, + seedData: SeedData, + title: string, + description: string, +): Promise { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description, + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + return session; +} + +test.describe("Empty-turn notice", () => { + test.describe.configure({ retries: 1 }); + + test("shows a slash-command hint when a /command produces no output", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedAndOpenChat( + testPage, + apiClient, + seedData, + "Empty Turn Hint", + "/e2e:empty-turn", + ); + + await expect(session.chat).toContainText(HINT, { timeout: 30_000 }); + }); + + test("does not show a hint when the turn produces output", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedAndOpenChat( + testPage, + apiClient, + seedData, + "Empty Turn No Hint", + "/e2e:simple-message", + ); + + await expect(session.chat).toContainText(MOCK_OUTPUT, { timeout: 30_000 }); + await session.waitForChatIdle(); + await expect(session.chat).not.toContainText(HINT); + }); +}); diff --git a/apps/web/e2e/tests/chat/mobile-empty-turn.spec.ts b/apps/web/e2e/tests/chat/mobile-empty-turn.spec.ts new file mode 100644 index 0000000000..c7a520f711 --- /dev/null +++ b/apps/web/e2e/tests/chat/mobile-empty-turn.spec.ts @@ -0,0 +1,52 @@ +import { type Page } from "@playwright/test"; +import { test, expect } from "../../fixtures/test-base"; +import type { SeedData } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { SessionPage } from "../../pages/session-page"; + +const HINT = /without the leading slash/i; + +/** + * Mobile parity for the empty-turn notice: the notice is a regular chat message, + * so it must render in the mobile session chat too. The triggering turn is the + * auto-started first turn (a multi-second `/e2e:empty-turn`) rather than a + * `sendMessage` follow-up — the live turn.completed reliably reaches the mobile + * client while the turn is still running. + */ +async function seedAndOpenChat( + testPage: Page, + apiClient: ApiClient, + seedData: SeedData, + title: string, +): Promise { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/e2e:empty-turn", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + return session; +} + +test.describe("Mobile empty-turn notice", () => { + test.describe.configure({ retries: 1, timeout: 120_000 }); + + test("shows the slash-command hint in the mobile chat", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedAndOpenChat(testPage, apiClient, seedData, "Mobile Empty Turn Hint"); + + await expect(session.chat).toContainText(HINT, { timeout: 30_000 }); + }); +}); diff --git a/apps/web/lib/types/backend.ts b/apps/web/lib/types/backend.ts index c97912e3c6..7fa3d38259 100644 --- a/apps/web/lib/types/backend.ts +++ b/apps/web/lib/types/backend.ts @@ -401,6 +401,8 @@ export type TurnEventPayload = { started_at: string; completed_at?: string; metadata?: Record; + /** Whether the completed turn produced any agent output. Only set on turn.completed. */ + had_output?: boolean; created_at: string; updated_at: string; }; diff --git a/apps/web/lib/ws/handlers/empty-turn-notice.test.ts b/apps/web/lib/ws/handlers/empty-turn-notice.test.ts new file mode 100644 index 0000000000..efd4508d4e --- /dev/null +++ b/apps/web/lib/ws/handlers/empty-turn-notice.test.ts @@ -0,0 +1,161 @@ +import { describe, it, expect } from "vitest"; +import { + parseSlashCommand, + isKnownCommand, + emptyTurnNoticeText, + computeEmptyTurnNotice, + type EmptyTurnNoticeInput, +} from "./empty-turn-notice"; +import type { AvailableCommand } from "@/lib/state/slices/session-runtime/types"; +import type { Message } from "@/lib/types/http"; +import { sessionId, taskId } from "@/lib/types/http"; + +function userMessage(turnId: string, content: string): Message { + return { + id: `user-${turnId}`, + session_id: sessionId("sess-1"), + task_id: taskId("task-1"), + turn_id: turnId, + author_type: "user", + content, + type: "message", + created_at: "2026-05-30T00:00:00Z", + }; +} + +function baseInput(overrides: Partial = {}): EmptyTurnNoticeInput { + return { + sessionId: "sess-1", + taskId: "task-1", + turnId: "turn-1", + hadOutput: false, + isEphemeralSurface: false, + messages: [userMessage("turn-1", "/pr-fixup please")], + availableCommands: [], + now: "2026-05-30T00:00:01Z", + ...overrides, + }; +} + +describe("parseSlashCommand", () => { + it.each<[string | undefined, string | null]>([ + [undefined, null], + ["", null], + ["hello world", null], + ["/pr-fixup", "pr-fixup"], + [" /pr-fixup ", "pr-fixup"], + ["/pr-fixup arg1 arg2", "pr-fixup"], + ["/PR-Fixup", "PR-Fixup"], + ["/", null], + ["/ ", null], + ])("parses %o → %o", (input, expected) => { + expect(parseSlashCommand(input)).toBe(expected); + }); +}); + +describe("isKnownCommand", () => { + const available: AvailableCommand[] = [{ name: "commit" }, { name: "pr-fixup" }]; + + it("matches a bare command name", () => { + expect(isKnownCommand("pr-fixup", available)).toBe(true); + }); + it("matches case-insensitively", () => { + expect(isKnownCommand("PR-Fixup", available)).toBe(true); + }); + it("returns false for an unknown command", () => { + expect(isKnownCommand("deploy", available)).toBe(false); + }); + it("returns false when there are no commands", () => { + expect(isKnownCommand("pr-fixup", undefined)).toBe(false); + expect(isKnownCommand("pr-fixup", [])).toBe(false); + }); +}); + +describe("emptyTurnNoticeText", () => { + it("generic notice for a non-slash prompt", () => { + expect(emptyTurnNoticeText(null, false)).toBe( + "The agent finished without producing any output.", + ); + }); + it("unknown-command notice mentions resending without the slash", () => { + const text = emptyTurnNoticeText("pr-fixup", false); + expect(text).toContain("`/pr-fixup` isn't a command this agent recognizes"); + expect(text).toContain("without the leading slash"); + }); + it("known-but-empty notice mentions resending without the slash", () => { + const text = emptyTurnNoticeText("pr-fixup", true); + expect(text).toContain("`/pr-fixup` ran but produced no output"); + expect(text).toContain("without the leading slash"); + }); +}); + +describe("computeEmptyTurnNotice", () => { + it("returns null when the turn produced output", () => { + expect(computeEmptyTurnNotice(baseInput({ hadOutput: true }))).toBeNull(); + }); + + it("returns null when had_output is undefined (older backend)", () => { + expect(computeEmptyTurnNotice(baseInput({ hadOutput: undefined }))).toBeNull(); + }); + + it("returns null on quick-chat / config-chat surfaces", () => { + expect(computeEmptyTurnNotice(baseInput({ isEphemeralSurface: true }))).toBeNull(); + }); + + it("returns null when a notice already exists for this turn (once per turn)", () => { + const existing: Message = { + id: "empty-turn-turn-1", + session_id: sessionId("sess-1"), + task_id: taskId("task-1"), + turn_id: "turn-1", + author_type: "agent", + content: "x", + type: "status", + created_at: "2026-05-30T00:00:00Z", + }; + const input = baseInput({ messages: [userMessage("turn-1", "/pr-fixup"), existing] }); + expect(computeEmptyTurnNotice(input)).toBeNull(); + }); + + it("builds a generic status notice for a non-slash empty turn", () => { + const notice = computeEmptyTurnNotice( + baseInput({ messages: [userMessage("turn-1", "do the thing")] }), + ); + expect(notice).not.toBeNull(); + expect(notice?.id).toBe("empty-turn-turn-1"); + expect(notice?.type).toBe("status"); + expect(notice?.author_type).toBe("agent"); + expect(notice?.turn_id).toBe("turn-1"); + expect(notice?.metadata?.empty_turn).toBe(true); + expect(notice?.content).toBe("The agent finished without producing any output."); + }); + + it("builds an unknown-command notice", () => { + const notice = computeEmptyTurnNotice( + baseInput({ messages: [userMessage("turn-1", "/pr-fixup")], availableCommands: [] }), + ); + expect(notice?.content).toContain("isn't a command this agent recognizes"); + }); + + it("builds a known-but-empty notice when the command is advertised", () => { + const notice = computeEmptyTurnNotice( + baseInput({ + messages: [userMessage("turn-1", "/pr-fixup")], + availableCommands: [{ name: "pr-fixup" }], + }), + ); + expect(notice?.content).toContain("ran but produced no output"); + }); + + it("uses the most recent user message for the turn", () => { + const notice = computeEmptyTurnNotice( + baseInput({ + messages: [ + userMessage("turn-0", "/old"), + { ...userMessage("turn-1", "/deploy"), id: "u1" }, + ], + }), + ); + expect(notice?.content).toContain("`/deploy`"); + }); +}); diff --git a/apps/web/lib/ws/handlers/empty-turn-notice.ts b/apps/web/lib/ws/handlers/empty-turn-notice.ts new file mode 100644 index 0000000000..3d75958bb9 --- /dev/null +++ b/apps/web/lib/ws/handlers/empty-turn-notice.ts @@ -0,0 +1,132 @@ +import type { StoreApi } from "zustand"; +import type { AppState } from "@/lib/state/store"; +import type { AvailableCommand } from "@/lib/state/slices/session-runtime/types"; +import type { TurnEventPayload } from "@/lib/types/backend"; +import type { Message } from "@/lib/types/http"; +import { sessionId, taskId } from "@/lib/types/http"; + +/** + * Extracts a leading `/command` token from a user prompt. Returns the bare + * command name (no slash, original case) or null when the prompt does not start + * with a slash command. Agent-advertised command names carry no leading slash, + * so callers match the returned token case-insensitively against them. + */ +export function parseSlashCommand(content: string | undefined): string | null { + const trimmed = content?.trim() ?? ""; + if (!trimmed.startsWith("/")) return null; + const firstWord = trimmed.slice(1).split(/\s+/)[0] ?? ""; + return firstWord.length > 0 ? firstWord : null; +} + +/** Whether `command` matches one of the agent's advertised commands (case-insensitive). */ +export function isKnownCommand( + command: string, + available: AvailableCommand[] | undefined, +): boolean { + const lower = command.toLowerCase(); + return (available ?? []).some((c) => c.name.toLowerCase() === lower); +} + +const RESEND_HINT = "Try resending your request as a normal message, without the leading slash."; + +/** The notice text shown for an empty turn, tailored to the triggering prompt. */ +export function emptyTurnNoticeText(command: string | null, known: boolean): string { + if (!command) { + return "The agent finished without producing any output."; + } + if (!known) { + return `\`/${command}\` isn't a command this agent recognizes, so it returned no output. ${RESEND_HINT}`; + } + return `\`/${command}\` ran but produced no output. ${RESEND_HINT}`; +} + +export interface EmptyTurnNoticeInput { + sessionId: string; + taskId: string; + turnId: string; + hadOutput: boolean | undefined; + isEphemeralSurface: boolean; + messages: Message[] | undefined; + availableCommands: AvailableCommand[] | undefined; + now: string; +} + +function noticeId(turnId: string): string { + return `empty-turn-${turnId}`; +} + +function findLastUserMessage(messages: Message[], turnId: string): Message | undefined { + for (let i = messages.length - 1; i >= 0; i--) { + const m = messages[i]; + if (m.author_type === "user" && m.turn_id === turnId) return m; + } + return undefined; +} + +/** + * Decides whether an empty-turn notice should be shown and, if so, returns the + * synthetic status Message to inject. Pure: all store reads are passed in. + * + * Guards (all must pass): the turn explicitly had no output (orphan turns swept + * on resume report had_output=true from the backend, so only genuine live + * completions reach here); the surface is the main task chat (not quick-chat / + * config-chat); and no notice exists yet for the turn (keyed by a deterministic + * id, so it fires once). + */ +export function computeEmptyTurnNotice(input: EmptyTurnNoticeInput): Message | null { + // Only an explicit `false` triggers the notice. `true` means the turn had + // output; `undefined` means an older backend that doesn't send the field — + // both correctly fall through here and produce no notice. + if (input.hadOutput !== false) return null; + if (input.isEphemeralSurface) return null; + + const messages = input.messages ?? []; + const id = noticeId(input.turnId); + if (messages.some((m) => m.id === id)) return null; + + const userMessage = findLastUserMessage(messages, input.turnId); + const command = parseSlashCommand(userMessage?.content); + const known = command ? isKnownCommand(command, input.availableCommands) : false; + const text = emptyTurnNoticeText(command, known); + + return { + id, + session_id: sessionId(input.sessionId), + task_id: taskId(input.taskId), + turn_id: input.turnId, + author_type: "agent", + content: text, + type: "status", + metadata: { variant: "warning", message: text, empty_turn: true }, + created_at: input.now, + }; +} + +/** + * Reads the store, computes the empty-turn notice for a completed turn, and + * injects it into the chat when warranted. + */ +export function maybeEmitEmptyTurnNotice( + store: StoreApi, + payload: TurnEventPayload, + now: string = new Date().toISOString(), +): void { + const state = store.getState(); + const sid = payload.session_id; + const isEphemeralSurface = + state.quickChat.sessions.some((s) => s.sessionId === sid) || + state.configChat.sessions.some((s) => s.sessionId === sid); + + const notice = computeEmptyTurnNotice({ + sessionId: sid, + taskId: payload.task_id, + turnId: payload.id, + hadOutput: payload.had_output, + isEphemeralSurface, + messages: state.messages.bySession[sid], + availableCommands: state.availableCommands.bySessionId[sid], + now, + }); + + if (notice) store.getState().addMessage(notice); +} diff --git a/apps/web/lib/ws/handlers/turns.ts b/apps/web/lib/ws/handlers/turns.ts index 7d40373ca8..da94fec95c 100644 --- a/apps/web/lib/ws/handlers/turns.ts +++ b/apps/web/lib/ws/handlers/turns.ts @@ -3,6 +3,7 @@ import { createDebugLogger } from "@/lib/debug/log"; import type { AppState } from "@/lib/state/store"; import type { WsHandlers } from "@/lib/ws/handlers/types"; import { sessionId, taskId } from "@/lib/types/http"; +import { maybeEmitEmptyTurnNotice } from "@/lib/ws/handlers/empty-turn-notice"; const debug = createDebugLogger("session:turns"); @@ -49,6 +50,8 @@ export function registerTurnsHandlers(store: StoreApi): WsHandlers { payload.id, payload.completed_at || new Date().toISOString(), ); + // Surface a notice when the turn finished with no agent output. + maybeEmitEmptyTurnNotice(store, payload); // Clear the active turn when it completes store.getState().setActiveTurn(payload.session_id, null); diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index 848fab5b5e..870a93a12f 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -86,6 +86,7 @@ Subscription quota tracking and per-agent cheap-model profile routing. | Spec | Status | |---|---| | [comment-markdown](ui/comment-markdown.md) | shipped | +| [empty-turn-notice](ui/empty-turn-notice.md) | shipped | ## system-page/ — operational diagnostics & maintenance UI diff --git a/docs/specs/ui/empty-turn-notice.md b/docs/specs/ui/empty-turn-notice.md new file mode 100644 index 0000000000..7192c41279 --- /dev/null +++ b/docs/specs/ui/empty-turn-notice.md @@ -0,0 +1,49 @@ +--- +status: shipped +created: 2026-05-30 +owner: cfl +--- + +# Empty-Turn Notice and Slash-Command Hint + +## Why + +When a user sends a message to the agent chat and the turn completes with **no content and no tool calls** (a clean `end_turn`), kandev showed the user's message followed by dead air — no reply, no explanation. This is common when a user types a slash command (e.g. `/pr-fixup`) into the agent chat: the ACP agent treats it as a slash command and returns an empty turn, either because the command is unsupported or because it exists but no-ops. The user gets no feedback and assumes the app is broken. + +## What + +- When an agent turn completes having produced **no agent output**, the chat shows an unobtrusive inline status notice instead of nothing. +- "Output" means at least one agent message that is a tool call (`tool_call`/`tool_edit`/`tool_read`/`tool_execute`), a native plan/todo (`agent_plan`/`todo`), a permission or clarification prompt (`permission_request`/`clarification_request`), or a non-empty text response (`message`/`content`). Incidental per-turn messages — lifecycle `status`/`script_execution` notices, `log`, `progress`, and `thinking` — do **not** count as output. +- The backend is authoritative: the `session.turn.completed` event carries a transient `had_output` boolean computed from the turn's persisted messages at completion time (no DB column; the notice is live-only). +- The notice text adapts to the triggering user message: + - **No leading `/`** → "The agent finished without producing any output." + - **`/cmd` not in the agent's advertised commands** → "`/cmd` isn't a command this agent recognizes, so it returned no output. Try resending your request as a normal message, without the leading slash." + - **`/cmd` is advertised but the turn was empty** → "`/cmd` ran but produced no output. Try resending your request as a normal message, without the leading slash." +- The `/command` token is matched case-insensitively against the agent's advertised commands (`availableCommands.bySessionId`), whose names carry no leading slash; the leading `/` is stripped and the first whitespace-delimited word is taken. +- The notice renders as a non-alarming `type:"status"` chat message (amber warning row via the shared `StatusMessage` renderer), so it appears on both desktop and mobile chat with no layout change. +- It fires once per turn, keyed by a deterministic message id `empty-turn-${turn_id}` (the store merges messages by id). +- It is scoped to the main task agent chat; quick-chat and config-chat surfaces are excluded. +- Orphan turns swept on session resume report `had_output=true`, so they never trigger the notice. + +## Scenarios + +- **GIVEN** a user sends `/pr-fixup` and the agent returns a clean empty turn, **WHEN** the turn completes and `/pr-fixup` is not an advertised command, **THEN** an inline notice says it isn't a recognized command and to resend without the leading slash. +- **GIVEN** a user sends `/commit` (an advertised command) and the turn produces no output, **WHEN** the turn completes, **THEN** the notice says the command ran but produced no output and to resend without the leading slash. +- **GIVEN** a user sends a normal prompt with no leading slash and the turn is empty, **WHEN** the turn completes, **THEN** the notice says the agent finished without producing any output. +- **GIVEN** a turn that produces a text response or a tool call, **WHEN** the turn completes, **THEN** no notice appears. +- **GIVEN** the same empty turn's `turn.completed` is processed more than once, **WHEN** the handler runs again, **THEN** only one notice exists for that turn. +- **GIVEN** an orphan turn swept by resume cleanup, **WHEN** its `turn.completed` is published, **THEN** no notice appears. +- **GIVEN** an empty turn on a quick-chat or config-chat surface, **WHEN** it completes, **THEN** no notice appears. + +## Out of scope + +- Persisting the notice: it is synthesized client-side from a live `turn.completed` event and is not stored, so it does not reappear on reload or in history. +- Auto-retrying the prompt without the slash, or any one-click "resend" action — the hint is advisory text only. +- Deriving the slash-command match on the backend; command cross-referencing happens entirely on the frontend where `availableCommands` lives. +- Office dashboard / live-run surfaces, which render their own run errors. + +## Notes + +- Backend: `had_output` is computed in `Service.CompleteTurn` via `turnHadAgentOutput` over the turn's persisted messages (`apps/backend/internal/task/service/service_turns.go`). Messages are fetched via `ListMessagesByTurnID` (indexed by `turn_id`), so the read is O(turn_messages) rather than O(session_messages). The result is added to the `turn.completed` payload in `publishTurnEvent`. +- Frontend: pure decision logic in `apps/web/lib/ws/handlers/empty-turn-notice.ts` (`computeEmptyTurnNotice`), wired from the `session.turn.completed` handler in `turns.ts`. +- E2E: the mock agent's `empty-turn` scenario (`apps/backend/cmd/mock-agent/scenarios.go`) emits a multi-second empty `end_turn`; specs in `apps/web/e2e/tests/chat/empty-turn.spec.ts` (desktop) and `mobile-empty-turn.spec.ts` seed it as the auto-started turn so the live completion is observed.