From 613b9a4eb97bc7f6926aee04adaab6bb33e33c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 3 Sep 2026 10:21:55 +0000 Subject: [PATCH] fix: deduplicate empty workflow prompts (#3299) --- ...vent_handlers_initial_prompt_dedup_test.go | 467 ++++++++++++++++++ .../orchestrator/event_handlers_workflow.go | 109 +++- apps/backend/internal/orchestrator/service.go | 7 + .../internal/orchestrator/task_operations.go | 118 ++++- .../task/handlers/process_handlers_test.go | 8 + .../task_handlers_start_agent_routing_test.go | 16 +- .../internal/task/repository/interface.go | 7 + .../repository/sqlite/message_prompt_index.go | 58 +++ .../repository/sqlite/prompt_history_test.go | 189 +++++++ .../task/repository/sqlite/session.go | 10 + .../internal/task/service/service_tasks.go | 13 +- .../service_tasks_step_routing_test.go | 7 +- apps/web/e2e/helpers/api-client.ts | 1 + ...bile-start-step-vs-auto-start-step.spec.ts | 71 +++ .../mobile-workflow-agent-switch.spec.ts | 1 + .../start-step-vs-auto-start-step.spec.ts | 217 +++++++- .../workflow/workflow-agent-switch.spec.ts | 1 + apps/web/hooks/use-message-handler.test.ts | 12 + apps/web/hooks/use-message-handler.ts | 4 +- .../plans/workflow-empty-prompt-dedup/plan.md | 156 ++++++ ...k-01-deduplicate-empty-workflow-prompts.md | 106 ++++ .../task-02-correct-plan-tool-names.md | 82 +++ .../task-03-prove-plan-mode-workflow-flow.md | 91 ++++ ...ask-04-route-immediate-plan-mode-starts.md | 109 ++++ docs/public/tasks-and-workflows.md | 4 +- docs/public/workflow-tips.md | 2 +- .../workflow-step-agent-start-ownership.md | 57 +++ .../workflow-step-agent-start-ownership.md | 109 ++++ 28 files changed, 1983 insertions(+), 49 deletions(-) create mode 100644 apps/backend/internal/orchestrator/event_handlers_initial_prompt_dedup_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/prompt_history_test.go create mode 100644 apps/web/e2e/tests/workflow/mobile-start-step-vs-auto-start-step.spec.ts create mode 100644 docs/plans/workflow-empty-prompt-dedup/plan.md create mode 100644 docs/plans/workflow-empty-prompt-dedup/task-01-deduplicate-empty-workflow-prompts.md create mode 100644 docs/plans/workflow-empty-prompt-dedup/task-02-correct-plan-tool-names.md create mode 100644 docs/plans/workflow-empty-prompt-dedup/task-03-prove-plan-mode-workflow-flow.md create mode 100644 docs/plans/workflow-empty-prompt-dedup/task-04-route-immediate-plan-mode-starts.md diff --git a/apps/backend/internal/orchestrator/event_handlers_initial_prompt_dedup_test.go b/apps/backend/internal/orchestrator/event_handlers_initial_prompt_dedup_test.go new file mode 100644 index 0000000000..c9d22fdb61 --- /dev/null +++ b/apps/backend/internal/orchestrator/event_handlers_initial_prompt_dedup_test.go @@ -0,0 +1,467 @@ +package orchestrator + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/kandev/kandev/internal/orchestrator/executor" + "github.com/kandev/kandev/internal/orchestrator/messagequeue" + "github.com/kandev/kandev/internal/orchestrator/queue" + "github.com/kandev/kandev/internal/orchestrator/scheduler" + "github.com/kandev/kandev/internal/sysprompt" + "github.com/kandev/kandev/internal/task/models" + sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite" + wfmodels "github.com/kandev/kandev/internal/workflow/models" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +func TestWorkflowAutoStartEmptyPrompt(t *testing.T) { + t.Run("prompted session suppresses task description", func(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, false) + beforeTurns := countSessionTurns(t, fixture.repo, fixture.sessionID) + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, false, true, false, + ) + + if got := len(fixture.agent.capturedPrompts); got != 0 { + t.Fatalf("captured prompts = %d, want 0", got) + } + if got := len(fixture.messages.userMessages); got != 0 { + t.Fatalf("recorded user messages = %d, want 0", got) + } + if got := countSessionTurns(t, fixture.repo, fixture.sessionID); got != beforeTurns { + t.Fatalf("session turns = %d, want unchanged count %d", got, beforeTurns) + } + }) + + t.Run("unprompted session uses task description once", func(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, false, false) + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, false, true, false, + ) + + if got := fixture.agent.capturedPrompts; len(got) != 1 || got[0] != fixture.taskDescription { + t.Fatalf("captured prompts = %#v, want [%q]", got, fixture.taskDescription) + } + if got := len(fixture.messages.userMessages); got != 1 { + t.Fatalf("recorded user messages = %d, want 1", got) + } + }) + + t.Run("passthrough session uses the same suppression decision", func(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, true) + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, false, true, false, + ) + + if got := len(fixture.agent.passthroughStdinCalls); got != 0 { + t.Fatalf("passthrough writes = %d, want 0", got) + } + }) + + t.Run("prompt-history errors stop automatic dispatch", func(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, false, false) + fixture.svc.repo = promptHistoryErrorRepo{ + repoStore: fixture.repo, + err: errors.New("history unavailable"), + } + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, false, true, false, + ) + + if got := len(fixture.agent.capturedPrompts); got != 0 { + t.Fatalf("captured prompts = %d, want 0", got) + } + updated, err := fixture.repo.GetTaskSession(context.Background(), fixture.sessionID) + if err != nil { + t.Fatalf("reload session: %v", err) + } + if updated.State != models.TaskSessionStateWaitingForInput { + t.Fatalf("session state = %q, want %q", updated.State, models.TaskSessionStateWaitingForInput) + } + }) + + t.Run("queued handoff survives task-description suppression", func(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, false) + if _, err := fixture.svc.messageQueue.QueueMessage( + context.Background(), fixture.sessionID, fixture.taskID, "Finish the handoff", "", "user", false, nil, + ); err != nil { + t.Fatalf("queue handoff: %v", err) + } + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, false, true, false, + ) + + if got := fixture.agent.capturedPrompts; len(got) != 1 || !strings.Contains(got[0], "Finish the handoff") || strings.Contains(got[0], fixture.taskDescription) { + t.Fatalf("captured prompts = %#v, want handoff only", got) + } + }) +} + +func TestWorkflowAutoStartNonEmptyPrompt(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, false) + fixture.step.Prompt = "Continue with validation." + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, false, true, false, + ) + + if got := fixture.agent.capturedPrompts; len(got) != 1 || got[0] != fixture.step.Prompt { + t.Fatalf("captured prompts = %#v, want [%q]", got, fixture.step.Prompt) + } + if got := len(fixture.messages.userMessages); got != 1 { + t.Fatalf("recorded user messages = %d, want 1", got) + } +} + +func TestWorkflowAutoStartPlanModeOnlyPrompt(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, false) + fixture.step.Events.OnEnter = []wfmodels.OnEnterAction{ + {Type: wfmodels.OnEnterEnablePlanMode}, + {Type: wfmodels.OnEnterAutoStartAgent}, + } + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, true, true, false, + ) + + if got := fixture.agent.capturedPrompts; len(got) != 1 || strings.Count(got[0], sysprompt.PlanMode()) != 1 { + t.Fatalf("captured prompts = %#v, want the plan-mode instructions", got) + } +} + +func TestWorkflowAutoStartPlanModeOnlyPromptForCreatedSession(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, false) + fixture.step.Events.OnEnter = []wfmodels.OnEnterAction{{Type: wfmodels.OnEnterEnablePlanMode}} + fixture.session.State = models.TaskSessionStateCreated + fixture.session.AgentProfileID = "profile-initial-prompt-dedup" + if err := fixture.repo.UpdateTaskSession(context.Background(), fixture.session); err != nil { + t.Fatalf("update created session: %v", err) + } + fixture.svc.scheduler = scheduler.NewScheduler( + queue.NewTaskQueue(10), fixture.svc.executor, fixture.svc.taskRepo, + testLogger(), scheduler.SchedulerConfig{}, + ) + fixture.svc.activeTurns.Store(fixture.sessionID, "turn-initial-prompt-dedup") + var launchPrompt string + fixture.agent.launchAgentFunc = func(_ context.Context, request *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) { + launchPrompt = request.TaskDescription + return &executor.LaunchAgentResponse{AgentExecutionID: "exec-created-plan-only"}, nil + } + + if err := fixture.svc.autoStartStepPrompt( + context.Background(), fixture.taskID, fixture.session, fixture.step, "", true, true, + ); err != nil { + t.Fatalf("autoStartStepPrompt returned error: %v", err) + } + + if launchPrompt == "" && len(fixture.agent.setExecutionDescriptionCalls) > 0 { + launchPrompt = fixture.agent.setExecutionDescriptionCalls[len(fixture.agent.setExecutionDescriptionCalls)-1].Prompt + } + if strings.Count(launchPrompt, sysprompt.PlanMode()) != 1 { + t.Fatalf("created launch prompt = %q, want one plan-mode instruction block", launchPrompt) + } +} + +func TestStartSessionForWorkflowStepPlanModeOnlyPrompt(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, false) + fixture.step.Events.OnEnter = []wfmodels.OnEnterAction{{Type: wfmodels.OnEnterEnablePlanMode}} + + if err := fixture.svc.StartSessionForWorkflowStep( + context.Background(), fixture.taskID, fixture.sessionID, fixture.step.ID, + ); err != nil { + t.Fatalf("StartSessionForWorkflowStep returned error: %v", err) + } + + if got := fixture.agent.capturedPrompts; len(got) != 1 || !strings.Contains(got[0], sysprompt.PlanMode()) { + t.Fatalf("captured prompts = %#v, want the plan-mode instructions", got) + } +} + +func TestWorkflowAutoStartAttachmentOnlyHandoffIsRecorded(t *testing.T) { + attachment := messagequeue.MessageAttachment{ + Type: "resource", + AttachmentID: "attachment-only", + MimeType: "text/plain", + Name: "handoff.txt", + Data: "aGVsbG8=", + } + for _, state := range []models.TaskSessionState{ + models.TaskSessionStateWaitingForInput, + models.TaskSessionStateCreated, + } { + t.Run(string(state), func(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, false, false) + fixture.svc.messageCreator = &repositoryBackedMessageCreator{ + mockMessageCreator: fixture.messages, + repo: fixture.repo, + } + fixture.session.State = state + fixture.session.AgentProfileID = "profile-initial-prompt-dedup" + if err := fixture.repo.UpdateTaskSession(context.Background(), fixture.session); err != nil { + t.Fatalf("update session: %v", err) + } + if state == models.TaskSessionStateCreated { + fixture.svc.scheduler = scheduler.NewScheduler( + queue.NewTaskQueue(10), fixture.svc.executor, fixture.svc.taskRepo, + testLogger(), scheduler.SchedulerConfig{}, + ) + } + // The fixture deliberately has no turn service. Reuse its seeded turn + // so the repository-backed creator can satisfy the message FK without + // introducing a second turn unrelated to this admission test. + fixture.svc.activeTurns.Store(fixture.sessionID, "turn-initial-prompt-dedup") + + if _, err := fixture.svc.messageQueue.QueueMessage( + context.Background(), fixture.sessionID, fixture.taskID, "", "", "user", false, []messagequeue.MessageAttachment{attachment}, + ); err != nil { + t.Fatalf("queue attachment-only handoff: %v", err) + } + + if err := fixture.svc.autoStartStepPrompt( + context.Background(), fixture.taskID, fixture.session, fixture.step, + "", false, true, + ); err != nil { + t.Fatalf("autoStartStepPrompt returned error: %v", err) + } + + if got := len(fixture.messages.userMessages); got != 1 { + t.Fatalf("recorded user messages = %d, want 1", got) + } + if state == models.TaskSessionStateCreated { + for _, call := range fixture.agent.setExecutionDescriptionCalls { + if strings.Contains(call.Prompt, fixture.taskDescription) { + t.Fatalf("created attachment-only handoff reintroduced task description: %#v", fixture.agent.setExecutionDescriptionCalls) + } + } + } + metadata, ok := fixture.messages.userMessages[0].metadata["attachments"].([]v1.MessageAttachment) + if !ok || len(metadata) != 1 || metadata[0].AttachmentID != attachment.AttachmentID { + t.Fatalf("recorded attachment metadata = %#v, want attachment-only handoff", fixture.messages.userMessages[0].metadata["attachments"]) + } + if hasHistory, err := fixture.repo.HasUserPromptHistory(context.Background(), fixture.sessionID); err != nil { + t.Fatalf("read attachment-only prompt history: %v", err) + } else if !hasHistory { + t.Fatal("attachment-only handoff did not advance durable prompt history") + } + persisted, err := fixture.repo.ListMessages(context.Background(), fixture.sessionID) + if err != nil { + t.Fatalf("list persisted attachment-only handoff: %v", err) + } + if len(persisted) != 1 || len(persisted[0].Metadata) == 0 { + t.Fatalf("persisted attachment-only messages = %#v, want one message with metadata", persisted) + } + persistedAttachments, ok := persisted[0].Metadata["attachments"].([]interface{}) + if !ok || len(persistedAttachments) != 1 { + t.Fatalf("persisted attachment metadata = %#v, want one attachment", persisted[0].Metadata["attachments"]) + } + }) + } +} + +func TestWorkflowAutoStartPassthroughDrainsSuppressedHandoff(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, true) + stdinDone := make(chan struct{}) + fixture.agent.passthroughStdinFunc = func(context.Context, string, string) error { + close(stdinDone) + return nil + } + if err := fixture.svc.messageQueue.SetAutoRun(context.Background(), fixture.sessionID, true); err != nil { + t.Fatalf("enable queue auto-run: %v", err) + } + if _, err := fixture.svc.messageQueue.QueueMessage( + context.Background(), fixture.sessionID, fixture.taskID, "Finish the handoff", "", "user", false, nil, + ); err != nil { + t.Fatalf("queue handoff: %v", err) + } + + fixture.svc.launchAfterOnEnterDispatch( + context.Background(), fixture.taskID, fixture.session, fixture.step, + fixture.taskDescription, false, true, false, + ) + + select { + case <-stdinDone: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for passthrough handoff drain") + } + if got := fixture.agent.passthroughStdinCalls; len(got) != 1 || !strings.Contains(got[0].Data, "Finish the handoff") { + t.Fatalf("passthrough writes = %#v, want the queued handoff", got) + } +} + +func TestStartSessionForWorkflowStepEmptyPrompt(t *testing.T) { + fixture := newInitialPromptDedupFixture(t, true, false) + beforeTurns := countSessionTurns(t, fixture.repo, fixture.sessionID) + + if err := fixture.svc.StartSessionForWorkflowStep( + context.Background(), fixture.taskID, fixture.sessionID, fixture.step.ID, + ); err != nil { + t.Fatalf("StartSessionForWorkflowStep returned error: %v", err) + } + + if got := len(fixture.agent.capturedPrompts); got != 0 { + t.Fatalf("captured prompts = %d, want 0", got) + } + if got := countSessionTurns(t, fixture.repo, fixture.sessionID); got != beforeTurns { + t.Fatalf("session turns = %d, want unchanged count %d", got, beforeTurns) + } +} + +type initialPromptDedupFixture struct { + repo *sqliterepo.Repository + svc *Service + agent *mockAgentManager + messages *mockMessageCreator + taskID string + sessionID string + taskDescription string + session *models.TaskSession + step *wfmodels.WorkflowStep +} + +type promptHistoryErrorRepo struct { + repoStore + err error +} + +type repositoryBackedMessageCreator struct { + *mockMessageCreator + repo *sqliterepo.Repository +} + +func (m *repositoryBackedMessageCreator) CreateUserMessage( + ctx context.Context, + taskID, content, sessionID, turnID string, + metadata map[string]interface{}, +) error { + if err := m.mockMessageCreator.CreateUserMessage(ctx, taskID, content, sessionID, turnID, metadata); err != nil { + return err + } + return m.repo.CreateMessage(ctx, &models.Message{ + TaskID: taskID, + TaskSessionID: sessionID, + TurnID: turnID, + AuthorType: models.MessageAuthorUser, + Content: content, + Metadata: metadata, + }) +} + +func (r promptHistoryErrorRepo) HasUserPromptHistory(context.Context, string) (bool, error) { + return false, r.err +} + +func (r promptHistoryErrorRepo) ClaimInitialPromptFallback(context.Context, string) (bool, error) { + return false, r.err +} + +func newInitialPromptDedupFixture(t *testing.T, prompted, passthrough bool) initialPromptDedupFixture { + t.Helper() + ctx := context.Background() + const ( + taskID = "task-initial-prompt-dedup" + sessionID = "session-initial-prompt-dedup" + stepID = "step-initial-prompt-dedup" + execID = "exec-initial-prompt-dedup" + ) + + repo := setupTestRepo(t) + seedSession(t, repo, taskID, sessionID, stepID) + session, err := repo.GetTaskSession(ctx, sessionID) + if err != nil { + t.Fatalf("get session: %v", err) + } + session.State = models.TaskSessionStateWaitingForInput + session.AgentExecutionID = execID + if err := repo.UpdateTaskSession(ctx, session); err != nil { + t.Fatalf("update session: %v", err) + } + seedExecutorRunning(t, repo, sessionID, taskID, execID) + + if err := repo.CreateTurn(ctx, &models.Turn{ + ID: "turn-initial-prompt-dedup", + TaskSessionID: sessionID, + TaskID: taskID, + CompletedAt: completedAt(), + }); err != nil { + t.Fatalf("create history turn: %v", err) + } + if prompted { + if err := repo.CreateMessage(ctx, &models.Message{ + ID: "message-initial-prompt-dedup", + TaskSessionID: sessionID, + TaskID: taskID, + TurnID: "turn-initial-prompt-dedup", + AuthorType: models.MessageAuthorUser, + Content: "already prompted", + }); err != nil { + t.Fatalf("create prompt history: %v", err) + } + } + + stepGetter := newMockStepGetter() + step := &wfmodels.WorkflowStep{ + ID: stepID, + WorkflowID: "wf1", + Name: "Initial prompt dedup", + Events: wfmodels.StepEvents{ + OnEnter: []wfmodels.OnEnterAction{{Type: wfmodels.OnEnterAutoStartAgent}}, + }, + } + stepGetter.steps[stepID] = step + taskRepo := newMockTaskRepo() + taskRepo.tasks[taskID] = &v1.Task{ + ID: taskID, WorkflowID: "wf1", State: v1.TaskStateInProgress, + } + agent := &mockAgentManager{ + isAgentRunning: true, + isPassthrough: passthrough, + repoForExecutionLookup: repo, + } + messages := &mockMessageCreator{} + svc := createTestServiceWithAgent(repo, stepGetter, taskRepo, agent) + svc.executor = executor.NewExecutor(agent, repo, testLogger(), executor.ExecutorConfig{}) + svc.messageCreator = messages + + return initialPromptDedupFixture{ + repo: repo, + svc: svc, + agent: agent, + messages: messages, + taskID: taskID, + sessionID: sessionID, + taskDescription: "Test", + session: session, + step: step, + } +} + +func completedAt() *time.Time { + completed := time.Now().UTC() + return &completed +} + +func countSessionTurns(t *testing.T, repo *sqliterepo.Repository, sessionID string) int { + t.Helper() + turns, err := repo.ListTurnsBySession(context.Background(), sessionID) + if err != nil { + t.Fatalf("list session turns: %v", err) + } + return len(turns) +} diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index 583aabf93f..ddd446611f 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -2884,13 +2884,31 @@ func (s *Service) launchAfterOnEnterDispatch( ) { sessionID := session.ID isPassthrough := s.agentManager.IsPassthroughSession(ctx, sessionID) + var effectivePrompt string + if hasAutoStart || (sessionSwitched && step.Prompt != "") { + var err error + effectivePrompt, err = s.buildWorkflowEntryPrompt( + ctx, taskDescription, step, taskID, sessionID, isPassthrough, + ) + if err != nil { + s.handleWorkflowEntryPromptError(ctx, taskID, session, step, err) + return + } + } switch { case hasAutoStart && isPassthrough && session.State != models.TaskSessionStateCreated: // Started passthrough path: write prompt directly to PTY stdin. // By the time processOnEnter runs (from an on_turn_complete transition), // the agent has finished its previous turn and the PTY is waiting for input. - effectivePrompt := s.buildWorkflowPrompt(ctx, taskDescription, step, taskID, sessionID, isPassthrough) + if strings.TrimSpace(effectivePrompt) == "" { + // The empty workflow prompt is suppressed for an already-prompted + // session, but a queued handoff may still be the next user input. + // Drain it through the normal executor path so passthrough attachments + // are materialized and the handoff is not stranded in the queue. + s.drainQueuedMessageForPromptableSession(ctx, sessionID) + return + } if err := s.autoStartPassthroughPrompt(ctx, taskID, session, step.Name, effectivePrompt); err != nil { s.logger.Error("failed to auto-start passthrough agent for step", zap.String("task_id", taskID), @@ -2906,7 +2924,6 @@ func (s *Service) launchAfterOnEnterDispatch( // When called from applyEngineTransition (on_turn_complete), processOnEnter // runs in a goroutine and the session is already WAITING_FOR_INPUT, so // autoStartStepPrompt sends the prompt directly via PromptTask. - effectivePrompt := s.buildWorkflowPrompt(ctx, taskDescription, step, taskID, sessionID, isPassthrough) if err := s.autoStartStepPrompt(ctx, taskID, session, step, effectivePrompt, hasPlanMode, true); err != nil { if errors.Is(err, errWorkflowAutoStartSessionTerminalized) { if workflowAutoStartWasCancelled(err) { @@ -2924,9 +2941,13 @@ func (s *Service) launchAfterOnEnterDispatch( zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.Error(replacementErr)) return } - replacementPrompt := s.buildWorkflowPrompt( + replacementPrompt, promptErr := s.buildWorkflowEntryPrompt( ctx, taskDescription, step, taskID, replacement.ID, isPassthrough, ) + if promptErr != nil { + s.handleWorkflowEntryPromptError(ctx, taskID, replacement, step, promptErr) + return + } if replacementErr = s.autoStartStepPrompt( ctx, taskID, replacement, step, replacementPrompt, hasPlanMode, true, ); replacementErr != nil { @@ -2951,7 +2972,6 @@ func (s *Service) launchAfterOnEnterDispatch( // has no auto_start_agent, launch the agent anyway — the profile override // implies the user wants this agent to run on this step. if sessionSwitched && step.Prompt != "" { - effectivePrompt := s.buildWorkflowPrompt(ctx, taskDescription, step, taskID, sessionID, isPassthrough) planMode := hasPlanMode stepID := step.ID s.logger.Info("auto-launching agent after profile switch (no explicit auto_start)", @@ -2994,9 +3014,13 @@ func (s *Service) launchAfterOnEnterDispatch( zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.Error(replacementErr)) return } - replacementPrompt := s.buildWorkflowPrompt( + replacementPrompt, promptErr := s.buildWorkflowEntryPrompt( asyncCtx, taskDescription, step, taskID, replacement.ID, isPassthrough, ) + if promptErr != nil { + s.handleWorkflowEntryPromptError(asyncCtx, taskID, replacement, step, promptErr) + return + } if replacementErr = s.autoStartStepPrompt( asyncCtx, taskID, replacement, step, replacementPrompt, planMode, true, ); replacementErr != nil { @@ -3028,9 +3052,13 @@ func (s *Service) launchAfterOnEnterDispatch( zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.Error(replacementErr)) return } - replacementPrompt := s.buildWorkflowPrompt( + replacementPrompt, promptErr := s.buildWorkflowEntryPrompt( asyncCtx, taskDescription, step, taskID, replacement.ID, isPassthrough, ) + if promptErr != nil { + s.handleWorkflowEntryPromptError(asyncCtx, taskID, replacement, step, promptErr) + return + } if replacementErr = s.autoStartStepPrompt( asyncCtx, taskID, replacement, step, replacementPrompt, planMode, true, ); replacementErr != nil { @@ -3067,6 +3095,22 @@ func (s *Service) launchAfterOnEnterDispatch( } +func (s *Service) handleWorkflowEntryPromptError( + ctx context.Context, + taskID string, + session *models.TaskSession, + step *wfmodels.WorkflowStep, + err error, +) { + s.logger.Error("failed to build workflow entry prompt", + zap.String("task_id", taskID), + zap.String("session_id", session.ID), + zap.String("step_name", step.Name), + zap.Error(err)) + s.setSessionWaitingForInput(ctx, taskID, session.ID, session) + s.publishSessionWaitingEvent(ctx, taskID, session.ID, step.ID, session) +} + // dispatchEngineOwnedOnEnterAction executes an engine-owned on_enter action // once for this step-entry. The marker CAS prevents duplicate execution. func (s *Service) dispatchEngineOwnedOnEnterAction( @@ -3825,6 +3869,13 @@ func (s *Service) autoStartStepPrompt( takenMsg, mergedPrompt, attachments, references := s.takeAndMergeHandoffMessage(ctx, sessionID, prompt) prompt = mergedPrompt agentPrompt := AppendEntityReferenceContext(prompt, references) + effectiveAgentPrompt := s.effectivePromptForSession(sessionID, agentPrompt, planMode, session) + if strings.TrimSpace(effectiveAgentPrompt) == "" && len(attachments) == 0 { + // The session is already running after ensureSessionRunning; no prompt + // means we leave it waiting for the first user message rather than + // auto-starting. Attachment-only handoffs are admitted below. + return nil + } // requeueTaken puts the original queued message back so a manual retry can // pick it up. Skip when shouldQueueIfBusy successfully re-queued the @@ -3858,7 +3909,14 @@ func (s *Service) autoStartStepPrompt( // pre-wrapped runtime context from current server state. // Passthrough sessions skip the wrap: the prompt is typed straight into // the agent CLI's TTY and the user sees it verbatim. - recordedPrompt := agentPrompt + // Persist the exact effective prompt so plan/config-only turns have a + // durable user row, but keep agentPrompt as the dispatch input for the + // running-session path. PromptTask applies the mode transforms once from + // that raw input; passing effectiveAgentPrompt there would double-wrap the + // plan/config instructions. The CREATED path uses the composed-prompt seam + // below because StartCreatedSession otherwise composes the prompt again. + recordedPrompt := effectiveAgentPrompt + dispatchPrompt := agentPrompt titleOwner := false isOfficeTask := false var taskForPrompt *models.Task @@ -3877,6 +3935,9 @@ func (s *Service) autoStartStepPrompt( recordedPrompt, pullRequestTargetContext = s.addTaskPullRequestTargetContext( ctx, taskID, recordedPrompt, session.IsPassthrough, ) + dispatchPrompt, _ = s.addTaskPullRequestTargetContext( + ctx, taskID, dispatchPrompt, session.IsPassthrough, + ) } if (session.State == models.TaskSessionStateCreated || step.HasOnEnterAction(wfmodels.OnEnterResetAgentContext)) && !session.IsPassthrough && (agentPrompt != "" || len(attachments) > 0) { configMode, _ := session.Metadata["config_mode"].(bool) @@ -3887,6 +3948,10 @@ func (s *Service) autoStartStepPrompt( taskID, sessionID, recordedPrompt, requiresSignal, referenceContext, pullRequestTargetContext, ) + dispatchPrompt = sysprompt.InjectOfficeContextWithOptions( + taskID, sessionID, dispatchPrompt, requiresSignal, + referenceContext, pullRequestTargetContext, + ) } else { recordedPrompt = sysprompt.InjectKandevContextWithOptions(taskID, sessionID, recordedPrompt, sysprompt.KandevContextOptions{ RequiresCompletionSignal: requiresSignal, @@ -3896,9 +3961,17 @@ func (s *Service) autoStartStepPrompt( IncludeUserQuestionTool: taskForPrompt == nil || !taskForPrompt.Autopilot, IncludeParentQuestionTool: taskForPrompt != nil && taskForPrompt.Autopilot && taskForPrompt.ParentID != "", }, referenceContext, pullRequestTargetContext) + dispatchPrompt = sysprompt.InjectKandevContextWithOptions(taskID, sessionID, dispatchPrompt, sysprompt.KandevContextOptions{ + RequiresCompletionSignal: requiresSignal, + IncludeCoordinatorTaskControls: !configMode, + IncludeTaskTitleTool: !configMode && titleOwner, + Autopilot: taskForPrompt != nil && taskForPrompt.Autopilot, + IncludeUserQuestionTool: taskForPrompt == nil || !taskForPrompt.Autopilot, + IncludeParentQuestionTool: taskForPrompt != nil && taskForPrompt.Autopilot && taskForPrompt.ParentID != "", + }, referenceContext, pullRequestTargetContext) } } - userMsgRecorded := s.recordAutoStartMessage(ctx, taskID, sessionID, recordedPrompt, planMode, origin, references) + userMsgRecorded := s.recordAutoStartMessage(ctx, taskID, sessionID, recordedPrompt, planMode, origin, references, attachments) // If the session is in CREATED state, the agent was never started (e.g. workspace-only // preparation from a blocked auto-start). PromptTask will reject CREATED sessions, @@ -3939,9 +4012,9 @@ func (s *Service) autoStartStepPrompt( zap.String("task_id", taskID), zap.String("session_id", sessionID), zap.String("step_name", stepName)) - _, err := s.StartCreatedSession( + _, err := s.startCreatedSessionWithComposedPrompt( ctx, taskID, sessionID, session.AgentProfileID, - recordedPrompt, true, planMode, true, attachments, references, + recordedPrompt, agentPrompt, true, planMode, true, attachments, references, ) // Release the guard as soon as StartCreatedSession has admitted the // launch (succeeded or failed definitively). The deferred release @@ -3959,7 +4032,7 @@ func (s *Service) autoStartStepPrompt( const maxRetryAttempts = 5 for attempt := 1; attempt <= maxRetryAttempts; attempt++ { - _, err := s.promptTask(ctx, taskID, sessionID, recordedPrompt, "", planMode, attachments, false, promptTaskOptions{ + _, err := s.promptTask(ctx, taskID, sessionID, dispatchPrompt, "", planMode, attachments, false, promptTaskOptions{ requireNonterminalSession: true, }) if err == nil { @@ -4170,18 +4243,19 @@ func (s *Service) takeAndMergeHandoffMessage(ctx context.Context, sessionID, bas // so it appears in the chat history. The prompt content includes system-injected // tags which are stripped when displayed to users via ToAPI(). // Returns true when the chat row was successfully inserted, false otherwise -// (messageCreator nil, empty prompt, or DB write failure). Callers that queue -// the prompt after this call must pass the return value to queueAutoStartPrompt -// as userMessageRecorded, so the drain side only skips CreateUserMessage when -// the write actually succeeded. +// (messageCreator nil, no prompt or attachment, or DB write failure). Callers +// that queue the prompt after this call must pass the return value to +// queueAutoStartPrompt as userMessageRecorded, so the drain side only skips +// CreateUserMessage when the write actually succeeded. func (s *Service) recordAutoStartMessage( ctx context.Context, taskID, sessionID, prompt string, planMode bool, origin workflowMessageOrigin, references []v1.EntityReference, + attachments []v1.MessageAttachment, ) bool { - if s.messageCreator == nil || prompt == "" { + if s.messageCreator == nil || (prompt == "" && len(attachments) == 0) { return false } turnID := s.getActiveTurnID(sessionID) @@ -4197,6 +4271,9 @@ func (s *Service) recordAutoStartMessage( // workflow_auto_start is the original tag this function set; preserved // for any consumer reading it directly. metaMap := workflowMessageMetadata(planMode, origin, references) + if len(attachments) > 0 { + metaMap["attachments"] = attachments + } if err := s.messageCreator.CreateUserMessage(ctx, taskID, prompt, sessionID, turnID, metaMap); err != nil { s.logger.Error("failed to create auto-start user message", zap.String("task_id", taskID), diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 2a9699042c..3a7671c62d 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -308,6 +308,13 @@ type repoStore interface { type sessionExecutorStore interface { // Session GetTaskSession(ctx context.Context, id string) (*models.TaskSession, error) + // HasUserPromptHistory reads the durable prompt sequence without scanning + // the session transcript. Empty workflow steps use it to decide whether the + // task description is still eligible as the initial prompt. + HasUserPromptHistory(ctx context.Context, sessionID string) (bool, error) + // ClaimInitialPromptFallback atomically reserves the first prompt slot for + // an empty workflow-step task-description fallback. + ClaimInitialPromptFallback(ctx context.Context, sessionID string) (bool, error) GetActiveTaskSessionByTaskID(ctx context.Context, taskID string) (*models.TaskSession, error) ListActiveTaskSessionsByTaskID(ctx context.Context, taskID string) ([]*models.TaskSession, error) SetSessionPrimary(ctx context.Context, sessionID string) error diff --git a/apps/backend/internal/orchestrator/task_operations.go b/apps/backend/internal/orchestrator/task_operations.go index b65c45d131..4002d4072d 100644 --- a/apps/backend/internal/orchestrator/task_operations.go +++ b/apps/backend/internal/orchestrator/task_operations.go @@ -431,7 +431,7 @@ func (s *Service) StartCreatedSession( ) (*executor.TaskExecution, error) { return s.startCreatedSession( ctx, taskID, sessionID, agentProfileID, prompt, - skipMessageRecord, planMode, autoStart, attachments, references, "", + skipMessageRecord, planMode, autoStart, attachments, references, "", startCreatedSessionOptions{}, ) } @@ -449,10 +449,41 @@ func (s *Service) StartCreatedSessionWithPromptContext( ) (*executor.TaskExecution, error) { return s.startCreatedSession( ctx, taskID, sessionID, agentProfileID, prompt, - skipMessageRecord, planMode, autoStart, attachments, references, promptReferenceContext, + skipMessageRecord, planMode, autoStart, attachments, references, promptReferenceContext, startCreatedSessionOptions{}, ) } +// startCreatedSessionWithComposedPrompt launches a prepared session from an +// auto-start path whose prompt was already composed and recorded by the +// orchestrator. The ordinary public entry point intentionally applies the +// workflow/config/plan transforms itself; reapplying them here would duplicate +// wrappers and would also fall back to the task description for an empty, +// already-prompted workflow step. Keep this seam private to the auto-start +// admission path so user-initiated starts retain their existing contract. +func (s *Service) startCreatedSessionWithComposedPrompt( + ctx context.Context, + taskID, sessionID, agentProfileID, prompt string, + retryPrompt string, + skipMessageRecord, planMode, autoStart bool, + attachments []v1.MessageAttachment, + references []v1.EntityReference, +) (*executor.TaskExecution, error) { + return s.startCreatedSession( + ctx, taskID, sessionID, agentProfileID, prompt, + skipMessageRecord, planMode, autoStart, attachments, references, "", startCreatedSessionOptions{ + skipTaskDescriptionFallback: true, + promptAlreadyComposed: true, + retryPrompt: retryPrompt, + }, + ) +} + +type startCreatedSessionOptions struct { + skipTaskDescriptionFallback bool + promptAlreadyComposed bool + retryPrompt string +} + //nolint:cyclop,funlen,gocognit // Existing complexity inherited from session-lifecycle handling. func (s *Service) startCreatedSession( ctx context.Context, @@ -461,6 +492,7 @@ func (s *Service) startCreatedSession( attachments []v1.MessageAttachment, references []v1.EntityReference, promptReferenceContext string, + options startCreatedSessionOptions, ) (*executor.TaskExecution, error) { releaseLifecycleLock := s.acquireSessionLifecycleLock(sessionID) defer releaseLifecycleLock() @@ -579,7 +611,7 @@ func (s *Service) startCreatedSession( } effectivePrompt := prompt - if effectivePrompt == "" { + if effectivePrompt == "" && !options.skipTaskDescriptionFallback { effectivePrompt = task.Description } @@ -633,20 +665,26 @@ func (s *Service) startCreatedSession( return nil, fmt.Errorf("failed to claim first-turn task title: %w", err) } } - effectivePrompt, planModeActive, generatedPromptReferenceContext := s.applyWorkflowAndPlanModeWithPromptContext( - ctx, effectivePrompt, taskID, sessionID, dbTask.WorkflowStepID, - planMode, task.IsEphemeral, session.IsPassthrough, promptReferenceContext, - ) - if generatedPromptReferenceContext != "" { - // The workflow helper preserves a direct message's acceptance-time - // context. Auto-started workflow prompts have no prior context, so their - // launch-time expansion becomes the trusted value here. - promptReferenceContext = generatedPromptReferenceContext + planModeActive := planMode + if !options.promptAlreadyComposed { + var generatedPromptReferenceContext string + effectivePrompt, planModeActive, generatedPromptReferenceContext = s.applyWorkflowAndPlanModeWithPromptContext( + ctx, effectivePrompt, taskID, sessionID, dbTask.WorkflowStepID, + planMode, task.IsEphemeral, session.IsPassthrough, promptReferenceContext, + ) + if generatedPromptReferenceContext != "" { + // The workflow helper preserves a direct message's acceptance-time + // context. Auto-started workflow prompts have no prior context, so their + // launch-time expansion becomes the trusted value here. + promptReferenceContext = generatedPromptReferenceContext + } } // Inject config context for config-mode sessions (dedicated settings chat) if configMode { - effectivePrompt = sysprompt.InjectConfigContext(sessionID, effectivePrompt) + if !options.promptAlreadyComposed { + effectivePrompt = sysprompt.InjectConfigContext(sessionID, effectivePrompt) + } } // Wrap the first prompt with the Kandev MCP system block. See the @@ -667,7 +705,11 @@ func (s *Service) startCreatedSession( // Cache the raw prompt so a transient-provider-error (529) retry can // re-drive this first turn — initial launches bypass PromptTask. - s.rememberTurnPrompt(sessionID, prompt, "", planMode, attachments) + retryPrompt := prompt + if options.promptAlreadyComposed { + retryPrompt = options.retryPrompt + } + s.rememberTurnPrompt(sessionID, retryPrompt, "", planMode, attachments) mcpMode := "" if isOfficeTask { @@ -1922,6 +1964,33 @@ func (s *Service) buildWorkflowPrompt(ctx context.Context, basePrompt string, st return prompt } +// buildWorkflowEntryPrompt applies the task-description fallback for one +// workflow entry. Empty steps use the description only when this call +// atomically claims the session's first prompt slot; non-empty step prompts +// keep their existing placeholder and replacement semantics. +func (s *Service) buildWorkflowEntryPrompt( + ctx context.Context, + taskDescription string, + step *wfmodels.WorkflowStep, + taskID, sessionID string, + isPassthrough bool, +) (string, error) { + basePrompt := taskDescription + if step.Prompt == "" && strings.TrimSpace(taskDescription) != "" { + claimed, err := s.repo.ClaimInitialPromptFallback(ctx, sessionID) + if err != nil { + return "", fmt.Errorf("failed to claim workflow prompt fallback: %w", err) + } + if !claimed { + basePrompt = "" + } + } + // A replacement session is intentionally a fresh prompt boundary. It has + // no prior claim, so the task description is eligible again after a reused + // session terminalizes during workflow entry. + return s.buildWorkflowPrompt(ctx, basePrompt, step, taskID, sessionID, isPassthrough), nil +} + // workflowInstructionsHeading/End are stable, agent-facing markers for the // optional workflow-level prompt block. Chat collapses everything between // them by default. Do not i18n (sent to the model, same as step prompt English). @@ -2371,7 +2440,12 @@ func (s *Service) StartSessionForWorkflowStep(ctx context.Context, taskID, sessi s.advanceTaskWorkflowStep(ctx, dbTask, workflowStepID, session) - effectivePrompt := s.buildWorkflowPrompt(ctx, dbTask.Description, step, taskID, sessionID, session.IsPassthrough) + effectivePrompt, err := s.buildWorkflowEntryPrompt( + ctx, dbTask.Description, step, taskID, sessionID, session.IsPassthrough, + ) + if err != nil { + return fmt.Errorf("failed to build workflow prompt: %w", err) + } if err := s.ensureSessionRunning(ctx, sessionID, session); err != nil { return err @@ -2381,8 +2455,20 @@ func (s *Service) StartSessionForWorkflowStep(ctx context.Context, taskID, sessi // step prompt. The helper reloads the session so a resume-created runtime // state cannot be overwritten by the stale request snapshot. s.applyWorkflowSessionConfigOnEnter(ctx, taskID, session, step) - stepPlanMode := step.HasOnEnterAction(wfmodels.OnEnterEnablePlanMode) + // The plan/config transforms can make an otherwise empty workflow prompt + // actionable. Decide whether to dispatch only after applying those same + // transforms that PromptTask applies downstream. + session = s.latestSession(ctx, session) + promptForEmptinessCheck := s.effectivePromptForSession(sessionID, effectivePrompt, stepPlanMode, session) + if strings.TrimSpace(promptForEmptinessCheck) == "" { + s.logger.Info("workflow step has no prompt after entry fallback", + zap.String("task_id", taskID), + zap.String("session_id", sessionID), + zap.String("workflow_step_id", workflowStepID)) + return nil + } + _, err = s.PromptTask(ctx, taskID, sessionID, effectivePrompt, "", stepPlanMode, nil, false) if err != nil { return fmt.Errorf("failed to prompt session: %w", err) diff --git a/apps/backend/internal/task/handlers/process_handlers_test.go b/apps/backend/internal/task/handlers/process_handlers_test.go index 96c236483f..35105ce349 100644 --- a/apps/backend/internal/task/handlers/process_handlers_test.go +++ b/apps/backend/internal/task/handlers/process_handlers_test.go @@ -33,6 +33,14 @@ type mockRepository struct { executors map[string]*models.Executor } +func (m *mockRepository) HasUserPromptHistory(context.Context, string) (bool, error) { + return false, nil +} + +func (m *mockRepository) ClaimInitialPromptFallback(context.Context, string) (bool, error) { + return true, nil +} + func (m *mockRepository) DeleteTurnIfUnreferenced(context.Context, string, string) (bool, error) { return false, nil } diff --git a/apps/backend/internal/task/handlers/task_handlers_start_agent_routing_test.go b/apps/backend/internal/task/handlers/task_handlers_start_agent_routing_test.go index d72fc3f849..83e51926ec 100644 --- a/apps/backend/internal/task/handlers/task_handlers_start_agent_routing_test.go +++ b/apps/backend/internal/task/handlers/task_handlers_start_agent_routing_test.go @@ -43,10 +43,15 @@ func TestHTTPCreateTask_StartAgentSelectsDestinationStep(t *testing.T) { for name, tc := range map[string]struct { startAgent bool + planMode bool wantStep string }{ "start_agent=true routes to the auto-start step": {startAgent: true, wantStep: "auto-start-step"}, "start_agent=false routes to the start step": {startAgent: false, wantStep: "start-step"}, + // @covers AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.1 + "start_agent=true with plan_mode routes to the auto-start step": { + startAgent: true, planMode: true, wantStep: "auto-start-step", + }, } { t.Run(name, func(t *testing.T) { log := newTestLogger(t) @@ -68,7 +73,8 @@ func TestHTTPCreateTask_StartAgentSelectsDestinationStep(t *testing.T) { "title": "Routed by intent", "description": "do the thing", "agent_profile_id": "profile-1", - "start_agent": ` + boolLiteral(tc.startAgent) + ` + "start_agent": ` + boolLiteral(tc.startAgent) + `, + "plan_mode": ` + boolLiteral(tc.planMode) + ` }` rec := httptest.NewRecorder() c, _ := gin.CreateTestContext(rec) @@ -103,6 +109,14 @@ func TestWSCreateTask_StartAgentSelectsDestinationStep(t *testing.T) { }, wantStep: "start-step", }, + // @covers AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.1 + "start_agent=true with plan_mode routes to the auto-start step": { + payload: map[string]any{ + "workspace_id": "ws-b", "workflow_id": "wf-b", "title": "Routed by intent", + "agent_profile_id": "profile-1", "start_agent": true, "plan_mode": true, + }, + wantStep: "auto-start-step", + }, "omitted start_agent routes to the start step": { payload: map[string]any{ "workspace_id": "ws-b", "workflow_id": "wf-b", "title": "Routed by intent", diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index a9a37fb336..4b2dc21af5 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -209,6 +209,13 @@ type WorkflowRepository interface { type MessageRepository interface { CreateMessage(ctx context.Context, message *models.Message) error GetMessage(ctx context.Context, id string) (*models.Message, error) + // HasUserPromptHistory reports whether the session has ever accepted a user + // prompt. The durable prompt sequence remains after message deletion. + HasUserPromptHistory(ctx context.Context, sessionID string) (bool, error) + // ClaimInitialPromptFallback atomically admits the task-description fallback + // for a never-prompted session. It returns false when another prompt or + // fallback has already claimed the session's first prompt slot. + ClaimInitialPromptFallback(ctx context.Context, sessionID string) (bool, error) // GetMessageWithPromptIndex retrieves a message by ID with its computed // prompt_index (1-based ordinal among the session's user messages). // Used by the idempotent WS replay/response path and user update-event diff --git a/apps/backend/internal/task/repository/sqlite/message_prompt_index.go b/apps/backend/internal/task/repository/sqlite/message_prompt_index.go index e972783048..fd4d8640d8 100644 --- a/apps/backend/internal/task/repository/sqlite/message_prompt_index.go +++ b/apps/backend/internal/task/repository/sqlite/message_prompt_index.go @@ -106,6 +106,64 @@ func (r *Repository) allocatePromptSeq(ctx context.Context, execer messageBounda return seq, nil } +// HasUserPromptHistory reports whether a session has accepted or reserved a +// user prompt. The durable sequence is not decremented when a message is +// deleted, so this remains true after the transcript row is removed. An empty +// sequence row is the reservation marker written by +// ClaimInitialPromptFallback; it is also history for the one-time fallback +// decision. The single-row lookup is bounded by the session primary key and +// does not scan message history. +func (r *Repository) HasUserPromptHistory(ctx context.Context, sessionID string) (bool, error) { + var marker int + err := r.ro.QueryRowContext(ctx, r.ro.Rebind( + `SELECT 1 FROM task_session_prompt_seq WHERE task_session_id = ?`, + ), sessionID).Scan(&marker) + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("read session prompt history: %w", err) + } + return marker == 1, nil +} + +// ClaimInitialPromptFallback atomically reserves the first prompt slot for an +// empty workflow-step task-description fallback. User-message creation uses +// the same per-session write boundary, so a direct prompt that commits first +// wins the slot and a concurrent fallback cannot also qualify. A successful +// claim writes a zero-valued reservation marker; the later visible fallback +// message then receives prompt ordinal 1 without making the reservation itself +// an empty transcript row. +func (r *Repository) ClaimInitialPromptFallback(ctx context.Context, sessionID string) (bool, error) { + if sessionID == "" { + return false, fmt.Errorf("session ID is required") + } + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return false, fmt.Errorf("begin initial prompt fallback claim: %w", err) + } + defer func() { _ = tx.Rollback() }() + if err := lockSessionTurnWrites(ctx, tx, r.db.DriverName(), sessionID); err != nil { + return false, err + } + result, err := tx.ExecContext(ctx, r.db.Rebind(` + INSERT INTO task_session_prompt_seq (task_session_id, last_seq) + VALUES (?, 0) + ON CONFLICT(task_session_id) DO NOTHING + `), sessionID) + if err != nil { + return false, fmt.Errorf("claim initial prompt fallback: %w", err) + } + if err := tx.Commit(); err != nil { + return false, fmt.Errorf("commit initial prompt fallback claim: %w", err) + } + claimed, err := result.RowsAffected() + if err != nil { + return false, fmt.Errorf("read initial prompt fallback claim result: %w", err) + } + return claimed > 0, nil +} + // readSessionMaxUserKey returns the session's newest user-message normalized // key in the exact key layout, and whether any user row exists. On SQLite the // key expression yields the text directly; on PostgreSQL the native TIMESTAMP diff --git a/apps/backend/internal/task/repository/sqlite/prompt_history_test.go b/apps/backend/internal/task/repository/sqlite/prompt_history_test.go new file mode 100644 index 0000000000..35dca0b30d --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/prompt_history_test.go @@ -0,0 +1,189 @@ +package sqlite + +import ( + "context" + "sync" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +type promptHistoryReader interface { + HasUserPromptHistory(context.Context, string) (bool, error) +} + +type promptHistoryClaimer interface { + ClaimInitialPromptFallback(context.Context, string) (bool, error) +} + +func TestHasUserPromptHistory(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedForMsgTest(t, repo, "task-history-a", "session-history-a", "turn-history-a") + seedForMsgTest(t, repo, "task-history-b", "session-history-b", "turn-history-b") + + reader, ok := any(repo).(promptHistoryReader) + if !ok { + t.Fatal("repository does not expose HasUserPromptHistory") + } + + hasHistory, err := reader.HasUserPromptHistory(ctx, "session-history-a") + if err != nil { + t.Fatalf("empty session history: %v", err) + } + if hasHistory { + t.Fatal("empty session reports user prompt history") + } + + if err := repo.CreateMessage(ctx, &models.Message{ + ID: "history-agent", + TaskSessionID: "session-history-a", + TurnID: "turn-history-a", + AuthorType: models.MessageAuthorAgent, + Content: "agent output", + }); err != nil { + t.Fatalf("create agent message: %v", err) + } + hasHistory, err = reader.HasUserPromptHistory(ctx, "session-history-a") + if err != nil { + t.Fatalf("agent-only session history: %v", err) + } + if hasHistory { + t.Fatal("agent-only session reports user prompt history") + } + + if err := repo.CreateMessage(ctx, &models.Message{ + ID: "history-user", + TaskSessionID: "session-history-a", + TurnID: "turn-history-a", + AuthorType: models.MessageAuthorUser, + Content: "first user prompt", + }); err != nil { + t.Fatalf("create user message: %v", err) + } + hasHistory, err = reader.HasUserPromptHistory(ctx, "session-history-a") + if err != nil { + t.Fatalf("session history after user message: %v", err) + } + if !hasHistory { + t.Fatal("session with a user message reports no history") + } + + if err := repo.DeleteMessage(ctx, "history-user"); err != nil { + t.Fatalf("delete user message: %v", err) + } + hasHistory, err = reader.HasUserPromptHistory(ctx, "session-history-a") + if err != nil { + t.Fatalf("session history after deletion: %v", err) + } + if !hasHistory { + t.Fatal("deleting a user message made prompt history eligible again") + } + + hasHistory, err = reader.HasUserPromptHistory(ctx, "session-history-b") + if err != nil { + t.Fatalf("other session history: %v", err) + } + if hasHistory { + t.Fatal("prompt history leaked between sessions") + } +} + +func TestClaimInitialPromptFallbackSerializesPromptAdmission(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedForMsgTest(t, repo, "task-claim-first", "session-claim-first", "turn-claim-first") + seedForMsgTest(t, repo, "task-direct-first", "session-direct-first", "turn-direct-first") + seedForMsgTest(t, repo, "task-claim-race", "session-claim-race", "turn-claim-race") + + claimer, ok := any(repo).(promptHistoryClaimer) + if !ok { + t.Fatal("repository does not expose ClaimInitialPromptFallback") + } + + claimed, err := claimer.ClaimInitialPromptFallback(ctx, "session-claim-first") + if err != nil || !claimed { + t.Fatalf("first fallback claim = %t, %v; want claimed", claimed, err) + } + if claimed, err := claimer.ClaimInitialPromptFallback(ctx, "session-claim-first"); err != nil || claimed { + t.Fatalf("second fallback claim = %t, %v; want rejected", claimed, err) + } + claimedPrompt := &models.Message{ + ID: "claimed-fallback-user", + TaskID: "task-claim-first", + TaskSessionID: "session-claim-first", + TurnID: "turn-claim-first", + AuthorType: models.MessageAuthorUser, + Content: "claimed fallback", + } + if err := repo.CreateMessage(ctx, claimedPrompt); err != nil { + t.Fatalf("create claimed fallback prompt: %v", err) + } + if claimedPrompt.PromptIndex != 1 { + t.Fatalf("claimed fallback prompt index = %d, want 1", claimedPrompt.PromptIndex) + } + + if err := repo.CreateMessage(ctx, &models.Message{ + ID: "direct-first-user", + TaskID: "task-direct-first", + TaskSessionID: "session-direct-first", + TurnID: "turn-direct-first", + AuthorType: models.MessageAuthorUser, + Content: "direct prompt", + }); err != nil { + t.Fatalf("create direct prompt: %v", err) + } + if claimed, err := claimer.ClaimInitialPromptFallback(ctx, "session-direct-first"); err != nil || claimed { + t.Fatalf("direct-first fallback claim = %t, %v; want rejected", claimed, err) + } + + var wg sync.WaitGroup + results := make(chan bool, 2) + for range 2 { + wg.Add(1) + go func() { + defer wg.Done() + claimed, claimErr := claimer.ClaimInitialPromptFallback(ctx, "session-claim-race") + if claimErr != nil { + t.Errorf("concurrent fallback claim: %v", claimErr) + return + } + results <- claimed + }() + } + wg.Wait() + close(results) + var claimedCount int + for claimed := range results { + if claimed { + claimedCount++ + } + } + if claimedCount != 1 { + t.Fatalf("concurrent fallback claims = %d, want exactly one winner", claimedCount) + } +} + +func TestDeleteTaskSessionRemovesPromptHistoryClaim(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedForMsgTest(t, repo, "task-session-reuse", "session-session-reuse", "turn-session-reuse") + + claimer := any(repo).(promptHistoryClaimer) + claimed, err := claimer.ClaimInitialPromptFallback(ctx, "session-session-reuse") + if err != nil || !claimed { + t.Fatalf("initial fallback claim = %t, %v; want claimed", claimed, err) + } + if err := repo.DeleteTaskSession(ctx, "session-session-reuse"); err != nil { + t.Fatalf("delete session: %v", err) + } + + seedForMsgTest(t, repo, "task-session-reuse", "session-session-reuse", "turn-session-reuse-new") + hasHistory, err := repo.HasUserPromptHistory(ctx, "session-session-reuse") + if err != nil { + t.Fatalf("read reused session history: %v", err) + } + if hasHistory { + t.Fatal("reused session inherited prompt history from deleted session") + } +} diff --git a/apps/backend/internal/task/repository/sqlite/session.go b/apps/backend/internal/task/repository/sqlite/session.go index a7d2d7b468..e5ebc88e40 100644 --- a/apps/backend/internal/task/repository/sqlite/session.go +++ b/apps/backend/internal/task/repository/sqlite/session.go @@ -2842,6 +2842,16 @@ func (r *Repository) DeleteTaskSession(ctx context.Context, id string) error { if rows == 0 { return fmt.Errorf("agent session not found: %s", id) } + // task_session_prompt_seq intentionally has no foreign key because it was + // added by a replay-safe migration. Remove its session-scoped admission + // state explicitly so a later session that reuses this ID starts fresh. + if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM task_session_prompt_seq WHERE task_session_id = ?`), id); err != nil { + // Isolated unit tests may omit the prompt-sequence schema. Production + // always has it; treat a missing table as already-cleaned. + if !db.IsMissingTableError(err) { + return fmt.Errorf("purge prompt history for session %s: %w", id, err) + } + } if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM queued_messages WHERE session_id = ?`), id); err != nil { // Isolated unit tests may omit the messagequeue schema. Production // always has queued_messages; treat a missing table as already-purged. diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index ada5184856..ad4dbdd56f 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -734,15 +734,14 @@ func (s *Service) resolveOfficeWorkflow(ctx context.Context, req *CreateTaskRequ // // Three destinations, picked by what the caller is asking for: // -// - plan mode → the first step by position. Planning happens before the work, -// so the task belongs at the head of the board even when a later step is -// marked as the start step. // - starting an agent now → the first step that runs agents // (on_enter: auto_start_agent). A task that is about to run does not belong -// in a parking column that was never configured to run anything. +// in a parking column that was never configured to run anything. Agent mode +// does not change this destination. +// - plan mode without an immediate agent start → the first step by position. // - everything else → the workflow's start step (is_start_step). // -// The middle case is the one that is easy to get wrong: `is_start_step` and +// The first case is the one that is easy to get wrong: `is_start_step` and // `auto_start_agent` are separate settings, and routing an agent start through // the start step silently made the two synonymous. It went unnoticed because // every built-in template puts both on the same step. @@ -752,10 +751,10 @@ func (s *Service) resolveWorkflowStep(ctx context.Context, req *CreateTaskReques var resolvedID string var err error switch { - case req.PlanMode: - resolvedID, err = s.startStepResolver.ResolveFirstStep(ctx, req.WorkflowID) case req.StartAgent: resolvedID, err = s.startStepResolver.ResolveAutoStartStep(ctx, req.WorkflowID) + case req.PlanMode: + resolvedID, err = s.startStepResolver.ResolveFirstStep(ctx, req.WorkflowID) default: resolvedID, err = s.startStepResolver.ResolveStartStep(ctx, req.WorkflowID) } diff --git a/apps/backend/internal/task/service/service_tasks_step_routing_test.go b/apps/backend/internal/task/service/service_tasks_step_routing_test.go index 346eac9f55..20ec2f2916 100644 --- a/apps/backend/internal/task/service/service_tasks_step_routing_test.go +++ b/apps/backend/internal/task/service/service_tasks_step_routing_test.go @@ -56,10 +56,11 @@ func TestResolveWorkflowStep_RoutesByCreateIntent(t *testing.T) { wantStepID: "first-step", }, { - name: "plan mode wins over an agent start", + // @covers AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.1 + name: "agent start wins over plan mode", req: &CreateTaskRequest{WorkflowID: "wf-1", PlanMode: true, StartAgent: true}, - wantResolver: "first", - wantStepID: "first-step", + wantResolver: "auto-start", + wantStepID: "auto-start-step", }, } diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 76974ef18b..f26f1353a8 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -2306,6 +2306,7 @@ export class ApiClient { state: string; is_primary: boolean; started_at: string; + updated_at: string; completed_at?: string | null; task_environment_id?: string; workspace_path?: string; diff --git a/apps/web/e2e/tests/workflow/mobile-start-step-vs-auto-start-step.spec.ts b/apps/web/e2e/tests/workflow/mobile-start-step-vs-auto-start-step.spec.ts new file mode 100644 index 0000000000..2cf735085d --- /dev/null +++ b/apps/web/e2e/tests/workflow/mobile-start-step-vs-auto-start-step.spec.ts @@ -0,0 +1,71 @@ +import { test, expect } from "../../fixtures/test-base"; +import { MobileKanbanPage } from "../../pages/mobile-kanban-page"; + +test.describe("mobile start step vs auto-start step", () => { + test("starts a plan-mode task on the first auto-start step", async ({ + testPage, + apiClient, + seedData, + }) => { + const workflow = await apiClient.createWorkflow( + seedData.workspaceId, + "Mobile Plan Mode Routing", + ); + const backlog = await apiClient.createWorkflowStep(workflow.id, "Backlog", 0, { + is_start_step: true, + }); + const inProgress = await apiClient.createWorkflowStep(workflow.id, "In Progress", 1); + await apiClient.updateWorkflowStep(backlog.id, { events: {} }); + await apiClient.updateWorkflowStep(inProgress.id, { + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: workflow.id, + task_create_last_used: { + repository_id: seedData.repositoryId, + branch: "main", + agent_profile_id: seedData.agentProfileId, + workflow_ids_by_workspace: { [seedData.workspaceId]: workflow.id }, + }, + enable_preview_on_click: false, + }); + + const mobile = new MobileKanbanPage(testPage); + await mobile.goto(); + await mobile.mobileFab.tap(); + + const dialog = testPage.getByTestId("create-task-dialog"); + await expect(dialog).toBeVisible(); + await dialog.getByTestId("task-title-input").fill("Mobile plan mode routing"); + await dialog.getByTestId("task-description-input").fill("/e2e:simple-message"); + + const planModeButton = dialog.getByTestId("mobile-plan-mode"); + await expect(planModeButton).toBeEnabled({ timeout: 30_000 }); + await planModeButton.tap(); + await expect(dialog).not.toBeVisible({ timeout: 15_000 }); + await expect(testPage).toHaveURL(/\/t\/.*layout=plan/, { timeout: 15_000 }); + + const taskId = testPage.url().match(/\/t\/([^/?#]+)/)?.[1]; + expect(taskId).toBeTruthy(); + if (!taskId) throw new Error("Mobile plan-mode task ID was missing from the session URL"); + + await expect + .poll(async () => (await apiClient.getTask(taskId)).workflow_step_id, { + timeout: 30_000, + message: "Waiting for the mobile plan-mode task to use the auto-start destination", + }) + .toBe(inProgress.id); + + await mobile.goto(); + await mobile.boardNavigator.tap(); + const inProgressTab = testPage + .getByTestId("mobile-board-navigator-drawer") + .getByTestId("column-tab-1"); + await expect(inProgressTab).toContainText("In Progress"); + await inProgressTab.tap(); + await expect(mobile.taskCardByTitle("Mobile plan mode routing")).toBeVisible({ + timeout: 15_000, + }); + }); +}); diff --git a/apps/web/e2e/tests/workflow/mobile-workflow-agent-switch.spec.ts b/apps/web/e2e/tests/workflow/mobile-workflow-agent-switch.spec.ts index 8cc0d4c6e1..0d5bbd08e9 100644 --- a/apps/web/e2e/tests/workflow/mobile-workflow-agent-switch.spec.ts +++ b/apps/web/e2e/tests/workflow/mobile-workflow-agent-switch.spec.ts @@ -50,6 +50,7 @@ test.describe("Workflow agent profile switching on mobile", () => { workflow_id: workflow.id, workflow_step_id: stepA.id, repository_ids: [seedData.repositoryId], + description: "Run the workflow session picker scenario", }, ); const originalASessionId = await waitForWorkflowProfileSession(apiClient, task.id, profileA.id); diff --git a/apps/web/e2e/tests/workflow/start-step-vs-auto-start-step.spec.ts b/apps/web/e2e/tests/workflow/start-step-vs-auto-start-step.spec.ts index c23f2bd450..71abb121cd 100644 --- a/apps/web/e2e/tests/workflow/start-step-vs-auto-start-step.spec.ts +++ b/apps/web/e2e/tests/workflow/start-step-vs-auto-start-step.spec.ts @@ -1,5 +1,7 @@ import { test, expect } from "../../fixtures/test-base"; +import { waitForSessionDone } from "../../helpers/session"; import { KanbanPage } from "../../pages/kanban-page"; +import { SessionPage } from "../../pages/session-page"; /** * `is_start_step` and `auto_start_agent` are independent settings: the first @@ -14,7 +16,7 @@ import { KanbanPage } from "../../pages/kanban-page"; * where the routing is visible at all. */ test.describe("start step vs auto-start step", () => { - test("parks a no-agent create and starts an agent create on the automated step", async ({ + test("routes immediate agent starts to the automated step in either agent mode", async ({ testPage, apiClient, seedData, @@ -70,6 +72,22 @@ test.describe("start step vs auto-start step", () => { }); // The parked card is untouched by the second create. await expect(kanban.taskCardInColumn("Parked for later", backlog.id)).toBeVisible(); + + // --- Start task in plan mode -> the same auto-start destination --- + await kanban.createTaskButton.click(); + await expect(dialog).toBeVisible(); + await testPage.getByTestId("task-title-input").fill("Plans right now"); + await testPage.getByTestId("task-description-input").fill("/e2e:simple-message"); + await expect(start).toBeEnabled({ timeout: 30_000 }); + await testPage.getByTestId("submit-start-agent-chevron").click(); + await testPage.getByTestId("submit-plan-mode").click(); + await expect(dialog).not.toBeVisible({ timeout: 15_000 }); + + await kanban.goto(); + await expect(kanban.taskCardInColumn("Plans right now", inProgress.id)).toBeVisible({ + timeout: 30_000, + }); + await expect(kanban.taskCardInColumn("Plans right now", backlog.id)).toHaveCount(0); }); // The dialog used to send its own `workflow_step_id`, taken from whatever the @@ -109,4 +127,201 @@ test.describe("start step vs auto-start step", () => { }); await expect(kanban.taskCardInColumn("Lands on the start step", backlog.id)).toHaveCount(0); }); + + test("does not repeat the plan-mode task description when entering an empty auto-start step", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const workflow = await apiClient.createWorkflow( + seedData.workspaceId, + "Plan Prompt Dedup Workflow", + ); + const planStep = await apiClient.createWorkflowStep(workflow.id, "Plan", 0, { + is_start_step: true, + }); + const autoStartStep = await apiClient.createWorkflowStep(workflow.id, "Auto Start", 1); + // The first step is also the first auto-start destination. This keeps the + // task on Plan under immediate-start routing, then exercises suppression + // when the existing session enters a second empty auto-start step. + await apiClient.updateWorkflowStep(planStep.id, { + prompt: "", + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + await apiClient.updateWorkflowStep(autoStartStep.id, { + prompt: "", + events: { on_enter: [{ type: "auto_start_agent" }] }, + }); + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: workflow.id, + task_create_last_used: { + repository_id: seedData.repositoryId, + branch: "main", + agent_profile_id: seedData.agentProfileId, + workflow_ids_by_workspace: { [seedData.workspaceId]: workflow.id }, + }, + enable_preview_on_click: false, + }); + + const description = "/e2e:simple-message"; + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + await kanban.createTaskButton.first().click(); + const dialog = testPage.getByTestId("create-task-dialog"); + await expect(dialog).toBeVisible(); + await testPage.getByTestId("task-title-input").fill("Plan prompt deduplication"); + await testPage.getByTestId("task-description-input").fill(description); + + const startButton = testPage.getByTestId("submit-start-agent"); + await expect(startButton).toBeEnabled({ timeout: 30_000 }); + await testPage.getByTestId("submit-start-agent-chevron").click(); + await expect(testPage.getByTestId("submit-plan-mode")).toBeVisible({ timeout: 5_000 }); + await testPage.getByTestId("submit-plan-mode").click(); + await expect(dialog).not.toBeVisible({ timeout: 10_000 }); + await expect(testPage).toHaveURL(/\/t\/.*layout=plan/, { timeout: 15_000 }); + + const taskId = testPage.url().match(/\/t\/([^/?#]+)/)?.[1]; + expect(taskId).toBeTruthy(); + if (!taskId) throw new Error("Plan-mode task ID was missing from the session URL"); + + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.planPanel).toBeVisible({ timeout: 10_000 }); + + let sessionId: string | undefined; + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(taskId); + sessionId = sessions[0]?.id; + return sessionId ?? null; + }, + { timeout: 30_000, message: "Waiting for the plan-mode session to be created" }, + ) + .not.toBeNull(); + if (!sessionId) throw new Error("Plan-mode session ID was not created"); + + await expect( + session.activeChat().getByText("simple mock response", { exact: false }), + ).toBeVisible({ + timeout: 30_000, + }); + await waitForSessionDone( + apiClient, + taskId, + sessionId, + "Waiting for the first plan-mode turn to finish", + ); + await session.waitForChatIdle({ timeout: 30_000 }); + + await expect + .poll( + async () => { + const { tasks } = await apiClient.listTasks(seedData.workspaceId); + return tasks.find((task) => task.id === taskId)?.workflow_step_id ?? null; + }, + { timeout: 15_000, message: "Waiting for the plan-mode task to remain on its first step" }, + ) + .toBe(planStep.id); + + const beforeMoveMessages = await apiClient.listSessionMessages(sessionId); + const beforeMoveTurns = await apiClient.listSessionTurns(sessionId); + const beforeMoveSession = (await apiClient.listTaskSessions(taskId)).sessions.find( + (candidate) => candidate.id === sessionId, + ); + expect(beforeMoveSession).toBeTruthy(); + const beforeMoveUpdatedAt = beforeMoveSession?.updated_at; + + await apiClient.moveTask(taskId, workflow.id, autoStartStep.id); + await expect + .poll( + async () => { + const { tasks } = await apiClient.listTasks(seedData.workspaceId); + return tasks.find((task) => task.id === taskId)?.workflow_step_id ?? null; + }, + { timeout: 15_000, message: "Waiting for the task to enter the empty auto-start step" }, + ) + .toBe(autoStartStep.id); + + // The move response only confirms task placement. The session review reset + // is the first durable signal from the asynchronous on_enter path, so wait + // for that post-move write before inspecting the transcript. + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(taskId); + const current = sessions.find((candidate) => candidate.id === sessionId); + return current?.updated_at && current.updated_at !== beforeMoveUpdatedAt + ? current.updated_at + : null; + }, + { + timeout: 30_000, + message: "Waiting for asynchronous workflow on_enter processing to write session state", + }, + ) + .not.toBeNull(); + + // Suppression is intentionally silent, so there is no new idle transition + // for waitForChatIdle to observe. Require a stable server-side snapshot + // after the causal write, which also catches a late duplicate turn. + let previousSnapshot = ""; + let stableReads = 0; + await expect + .poll( + async () => { + const [{ messages }, { turns }, { sessions }] = await Promise.all([ + apiClient.listSessionMessages(sessionId), + apiClient.listSessionTurns(sessionId), + apiClient.listTaskSessions(taskId), + ]); + const current = sessions.find((candidate) => candidate.id === sessionId); + const snapshot = JSON.stringify({ + state: current?.state, + messages: messages.map((message) => ({ + id: message.id, + author_type: message.author_type, + content: message.content, + })), + turns: turns.map((turn) => ({ id: turn.id, completed_at: turn.completed_at })), + }); + if (snapshot === previousSnapshot) { + stableReads += 1; + } else { + previousSnapshot = snapshot; + stableReads = 0; + } + return stableReads; + }, + { + timeout: 30_000, + intervals: [250, 500, 750, 1_000], + message: "Waiting for workflow on_enter prompt admission to settle", + }, + ) + .toBeGreaterThanOrEqual(3); + + const afterMoveMessages = await apiClient.listSessionMessages(sessionId); + const afterMoveTurns = await apiClient.listSessionTurns(sessionId); + const userMessages = afterMoveMessages.messages.filter( + (message) => message.author_type === "user", + ); + await session.showSessionContext(); + + const userDescriptionMessages = userMessages.filter((message) => + message.content.includes(description), + ); + const emptyUserMessages = userMessages.filter((message) => message.content.trim() === ""); + expect(userDescriptionMessages).toHaveLength(1); + expect(emptyUserMessages).toHaveLength(0); + expect(afterMoveTurns.turns).toHaveLength(beforeMoveTurns.turns.length); + expect( + beforeMoveMessages.messages.filter((message) => message.author_type === "user"), + ).toHaveLength(1); + await expect(session.activeChat().getByText(description, { exact: true })).toHaveCount(1); + }); }); diff --git a/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts b/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts index ae53af012e..b406574bd8 100644 --- a/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts @@ -233,6 +233,7 @@ async function runProfileSessionLifecycleScenario( workflow_step_id: inbox.id, agent_profile_id: profileA.id, repository_ids: [seedData.repositoryId], + description: "Run the workflow lifecycle scenario", }, ); await apiClient.moveTask(task.id, workflow.id, stepA.id); diff --git a/apps/web/hooks/use-message-handler.test.ts b/apps/web/hooks/use-message-handler.test.ts index b3c2490b35..a40b65d86d 100644 --- a/apps/web/hooks/use-message-handler.test.ts +++ b/apps/web/hooks/use-message-handler.test.ts @@ -1,6 +1,7 @@ import { act, renderHook } from "@testing-library/react"; import { beforeEach, describe, it, expect, vi } from "vitest"; import { + buildDocumentContext, buildContextFilesContext, buildTaskMentionsContext, sendMessageRequest, @@ -156,6 +157,17 @@ describe("buildTaskMentionsContext", () => { }); }); +describe("buildDocumentContext", () => { + it("uses the canonical plan tools in active-plan context", () => { + const out = buildDocumentContext({ type: "plan", taskId: TASK_ID }, true); + + expect(out).toContain("get_task_plan_kandev"); + expect(out).toContain("update_task_plan_kandev"); + expect(out).not.toContain("plan_get"); + expect(out).not.toContain("plan_update"); + }); +}); + describe("buildContextFilesContext", () => { it("describes attached files and directories while preserving their paths", () => { const out = buildContextFilesContext( diff --git a/apps/web/hooks/use-message-handler.ts b/apps/web/hooks/use-message-handler.ts index b3ce875235..9509563154 100644 --- a/apps/web/hooks/use-message-handler.ts +++ b/apps/web/hooks/use-message-handler.ts @@ -26,7 +26,7 @@ import { } from "./domains/session/session-input-mode"; import { t } from "@/lib/i18n"; -function buildDocumentContext( +export function buildDocumentContext( activeDocument: ActiveDocument | null, planModeEnabled: boolean, planComments?: PlanComment[], @@ -36,7 +36,7 @@ function buildDocumentContext( if (activeDocument.type === "plan") { if (!planModeEnabled) return ""; - let context = `\n\n\nACTIVE DOCUMENT: The user is editing the task plan side-by-side with this chat.\nRead the current plan using the plan_get MCP tool to understand the context before responding.\nAny plan modifications should use the plan_update MCP tool.`; + let context = `\n\n\nACTIVE DOCUMENT: The user is editing the task plan side-by-side with this chat.\nRead the current plan using the get_task_plan_kandev MCP tool to understand the context before responding.\nAny plan modifications should use the update_task_plan_kandev MCP tool.`; if (planComments && planComments.length > 0) { context += `\n\nUser comments on the plan:\n`; diff --git a/docs/plans/workflow-empty-prompt-dedup/plan.md b/docs/plans/workflow-empty-prompt-dedup/plan.md new file mode 100644 index 0000000000..57b13dd4fe --- /dev/null +++ b/docs/plans/workflow-empty-prompt-dedup/plan.md @@ -0,0 +1,156 @@ +--- +created: 2026-09-02 +status: done +requirements: + - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003 + - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004 + - REQ-TASKS-MCP-TOOL-NAMES-001 +system_design: + - ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md + - ../../specs/tasks/system-design/mcp-tool-name-stability.md +legacy_specs: + - ../../specs/workflow-on-enter-action-dispatch/spec.md +--- + +# Implementation Plan: Deduplicate Empty Workflow Prompts + +## Overview + +The backend uses an atomically claimed session prompt boundary before it applies +the task-description fallback. The frontend uses the canonical plan-tool names. + +The follow-up work makes immediate agent-start placement independent of agent +mode. This keeps the task in the workflow step that owns automatic starts. + +## Scope + +### In scope + +- Stop repeated task-description prompts on empty automatic-start steps. +- Apply one fallback rule to automatic step entry and explicit workflow-step launch. +- Preserve non-empty step prompts for prompted sessions. +- Preserve the first task-description prompt for unprompted sessions. +- Correct `plan_get` and `plan_update` in the active-plan context. +- Add focused Go, TypeScript, and Playwright regressions. +- Route an immediate plan-mode agent start to the first automatic-start step. +- Prove the routing through desktop and mobile task creation. +- Update the public workflow guidance. + +### Out of scope + +- Do not change the first-step placement of plan-only prepared sessions when + `start_agent=false`. +- Do not change the configured start-step behavior for no-agent task creation. +- Do not change workflow prompts that contain text. +- Do not add a database column or migration. +- Do not change MCP tool registration or transport names. + +## Technical approach + +### Durable prompt history + +Add a bounded repository query for `task_session_prompt_seq.last_seq` and an atomic initial-fallback claim. The claim and direct user-message creation use the same per-session persistence boundary, so concurrent automatic and explicit admissions cannot both qualify as the first prompt. The claim uses a zero-valued counter row as its reservation marker, so the first visible message still receives prompt ordinal 1. + +Expose this query and claim through the repository contract that the orchestrator uses. Keep message deletion behavior unchanged because the counter never decreases. + +### Shared workflow prompt decision + +Add an orchestrator helper that composes a workflow-entry prompt for one session. For an empty `WorkflowStep.Prompt` only, use the task description only when the atomic initial-fallback claim succeeds. Non-empty step prompts, including `{{task_prompt}}` expansion, retain their existing semantics. + +Call the helper from `launchAfterOnEnterDispatch` before its ACP or passthrough split. Call the same helper from `StartSessionForWorkflowStep`. + +Keep `buildWorkflowPromptWithTrustedContext` as the string composer. Keep non-empty `WorkflowStep.Prompt` behavior unchanged. + +Apply plan-mode and session-config transforms before deciding whether the composed prompt is empty. If the composed ACP prompt is empty, let `autoStartStepPrompt` inspect any queued handoff first. Return without message creation or dispatch only when the merged prompt and attachments are empty. Attachment-only handoffs are admitted and persisted with their metadata. + +### Plan-tool names + +Change the active-plan context in `apps/web/hooks/use-message-handler.ts`. Use `get_task_plan_kandev` and `update_task_plan_kandev`. + +Export the pure context helper for focused unit coverage. Do not localize this model-facing instruction. + +### Immediate-launch placement + +Change `task.Service.resolveWorkflowStep` so `StartAgent` is evaluated before +`PlanMode`. A request with both values uses `ResolveAutoStartStep`. A request +with only `PlanMode` continues to use `ResolveFirstStep`. + +Keep explicit `workflow_step_id` precedence and the existing automatic-start +fallback chain. No transport, API, persistence, or frontend production change +is required. + +Update the existing desktop workflow E2E scenario. Add a mobile scenario that +uses the visible plan-mode action. Update the public task-creation reference +and troubleshooting text. + +## Tests + +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.1`: add a Go test for an unprompted session on an empty automatic-start step. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.2`: add a Go test for a prompted plan-mode session that later enters an empty automatic-start step. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.3`: add a Go test that a non-empty step prompt still dispatches after earlier prompts. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.4`: assert that an empty result creates no user row and no runtime prompt, while an attachment-only handoff remains durable input. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.5`: add a focused `StartSessionForWorkflowStep` regression. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.6`: cover the shared decision before the transport split. +- `AC-TASKS-MCP-TOOL-NAMES-001.3`: add a TypeScript test for canonical plan-tool names. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.1`: update service and + transport routing tests for `start_agent=true` with `plan_mode=true`. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.2`: retain the existing + automatic-start fallback test. +- `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.3`: retain the explicit-step + precedence test. + +## E2E tests + +Update `apps/web/e2e/tests/workflow/start-step-vs-auto-start-step.spec.ts`. +Start a plan-mode agent through the desktop task dialog. Assert that the task +lands in the first automatic-start step. + +Keep the duplicate-prompt scenario by making Plan the first automatic-start +destination. Move the idle task into a second empty automatic-start step. +Assert that the description appears once and that no extra turn exists. + +Add `apps/web/e2e/tests/workflow/mobile-start-step-vs-auto-start-step.spec.ts`. +Use the mobile plan-mode button and assert the same destination. These tests +change behavior only. The current task dialog layout and mobile composition +remain unchanged. + +The current mobile task-create footer is the nearest mobile exemplar. It keeps +the plan-mode action visible as a touch-sized button. The desktop split menu +and the mobile button continue to use one submission handler. + +## Work orders + +- [x] [Task 01: Deduplicate empty workflow prompts](task-01-deduplicate-empty-workflow-prompts.md) +- [x] [Task 02: Correct plan-tool names](task-02-correct-plan-tool-names.md) +- [x] [Task 03: Prove the plan-mode workflow flow](task-03-prove-plan-mode-workflow-flow.md) +- [x] [Task 04: Route immediate plan-mode starts](task-04-route-immediate-plan-mode-starts.md) + +## Verification results + +- Backend focused race suite: 18 tests passed after the PR fixup, including prompt-admission races, plan-only and attachment-only inputs, passthrough draining, and session-ID reuse cleanup. +- Backend full suite: all packages passed with the task-session internal + configuration handoff variables cleared. The plain command was blocked by + the inherited launcher-selected `/root/.kandev/config.yaml`, which made + isolated config-discovery tests select the operator config. +- Backend `make lint`: 0 issues. +- Backend new-from-`origin/main` `golangci-lint`: no issues. +- Frontend focused hook suite: 27 tests passed. +- Frontend typecheck, targeted ESLint, i18n check, and i18n ratchet: passed. +- Frontend production build and targeted Playwright regression: passed. +- Task 04 focused backend suite: 14 tests passed in 2 packages. +- Task 04 desktop Playwright suite: 3 tests passed, including the retained + duplicate-prompt regression. +- Task 04 mobile Chrome Playwright suite: 1 test passed through the phone-only + Plan mode action and phone board navigation. +- Public documentation validation: 61 tests passed and 41 published pages + validated. + +## Risks + +- A transcript scan can grow with session size. The implementation must use the bounded prompt counter. +- Suppression before queued-message merge can lose a handoff. The ACP path must merge first. +- A broad guard can suppress non-empty step prompts. The claim applies only to the empty `WorkflowStep.Prompt` fallback; non-empty prompts, including `{{task_prompt}}`, retain their existing semantics. +- A session-ID reuse can leak a prompt claim if the counter is not removed with the session. Delete the counter explicitly because the replay-safe table has no foreign key. +- The explicit workflow-step path also manages resume state. Prompt suppression must not change its existing lifecycle behavior. +- The current desktop E2E scenario encodes the old plan-mode destination. Task + 04 must replace that assertion without removing duplicate-prompt coverage. diff --git a/docs/plans/workflow-empty-prompt-dedup/task-01-deduplicate-empty-workflow-prompts.md b/docs/plans/workflow-empty-prompt-dedup/task-01-deduplicate-empty-workflow-prompts.md new file mode 100644 index 0000000000..733c8d3a7f --- /dev/null +++ b/docs/plans/workflow-empty-prompt-dedup/task-01-deduplicate-empty-workflow-prompts.md @@ -0,0 +1,106 @@ +--- +id: "01-deduplicate-empty-workflow-prompts" +title: "Deduplicate empty workflow prompts" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003 +acceptance_criteria: + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.1 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.2 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.3 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.4 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.5 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.6 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.7 +system_design: + - ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md +--- + +# Task 01: Deduplicate Empty Workflow Prompts + +## Summary + +Add a durable prompt-history counter and atomic first-prompt claim for workflow prompt composition. Only an empty step prompt can use the task description as the one-time fallback. + +## In scope + +- Add a bounded prompt-history query to the task repository. +- Apply the query in both workflow-step prompt paths. +- Keep ACP and passthrough behavior aligned. +- Preserve non-empty step prompts and ACP handoffs. +- Add deterministic repository and orchestrator tests. + +## Out of scope + +- Plan-mode step placement. +- Start-step resolution. +- New persisted fields. +- Frontend tool-name text. + +## Acceptance + +- An empty automatic-start step does not repeat a task description for a prompted session. +- An unprompted session still receives the task description once. +- A non-empty step prompt still dispatches, and an empty result creates no turn or message. +- Attachment-only handoffs are persisted and dispatched with their attachment metadata. +- Concurrent direct and automatic first-prompt admissions allow at most one initial fallback. + +## Verification + +```bash +(cd apps/backend && go test -race -tags fts5 ./internal/task/repository/sqlite ./internal/orchestrator -run '^(TestHasUserPromptHistory|TestClaimInitialPromptFallbackSerializesPromptAdmission|TestDeleteTaskSessionRemovesPromptHistoryClaim|TestWorkflowAutoStartEmptyPrompt|TestWorkflowAutoStartNonEmptyPrompt|TestStartSessionForWorkflowStepEmptyPrompt|TestWorkflowAutoStartPlanModeOnlyPrompt|TestWorkflowAutoStartPlanModeOnlyPromptForCreatedSession|TestStartSessionForWorkflowStepPlanModeOnlyPrompt|TestWorkflowAutoStartAttachmentOnlyHandoffIsRecorded|TestWorkflowAutoStartPassthroughDrainsSuppressedHandoff)' -count=1) +make -C apps/backend test +make -C apps/backend lint +(cd apps/backend && golangci-lint run ./... --new-from-rev="$(git merge-base HEAD origin/main)" --timeout=5m) +``` + +## Files likely touched + +- `apps/backend/internal/task/repository/interface.go` +- `apps/backend/internal/task/repository/sqlite/message_prompt_index.go` +- `apps/backend/internal/task/repository/sqlite/prompt_history_test.go` +- `apps/backend/internal/orchestrator/service.go` +- `apps/backend/internal/orchestrator/task_operations.go` +- `apps/backend/internal/orchestrator/event_handlers_workflow.go` +- `apps/backend/internal/orchestrator/event_handlers_initial_prompt_dedup_test.go` +- `apps/backend/internal/task/handlers/process_handlers_test.go` + +## Dependencies + +None. + +## Risks + +- Repository interface changes can require test-double updates. +- The no-content branch can bypass a queued handoff if it runs too early. +- Prompt-history errors can leave the session waiting until a user retries. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003` +- Prompt fallback ownership and workflow-entry prompt flow in the system design. +- Existing prompt-counter allocation in `message_prompt_index.go`. +- Existing auto-start recovery tests in `event_handlers_duplicate_autostart_test.go`. + +## Results + +- Added the durable `HasUserPromptHistory` query plus the atomic + `ClaimInitialPromptFallback` admission boundary and applied one entry-prompt + decision to automatic and explicit workflow-step launches. +- Added coverage for prompted and unprompted empty steps, non-empty step + prompts, passthrough suppression, prompt-history errors, plan-mode-only + instructions, attachment-only handoffs, prompt-admission races, session-ID + reuse cleanup, and the no-turn/no-message empty result. +- The focused race suite passed with 18 tests, including both prompt-admission orderings, concurrent fallback claims, plan-only prompts, attachment-only handoffs, passthrough draining, and session-ID reuse cleanup. +- `env KANDEV_INTERNAL_CONFIG_FILE= KANDEV_INTERNAL_CONFIG_HOME_FILE= make -C apps/backend test` passed for every backend package. The unmodified + `make -C apps/backend test` form was also attempted, but this task session's + injected launcher config caused unrelated home-config discovery failures. +- `make -C apps/backend lint` passed with 0 issues. +- `golangci-lint run ./... --new-from-rev=0064b9fb5a903feeb6cffc2ce6b7db1c16218e6e --timeout=5m` passed with no issues. diff --git a/docs/plans/workflow-empty-prompt-dedup/task-02-correct-plan-tool-names.md b/docs/plans/workflow-empty-prompt-dedup/task-02-correct-plan-tool-names.md new file mode 100644 index 0000000000..87fbeae9b3 --- /dev/null +++ b/docs/plans/workflow-empty-prompt-dedup/task-02-correct-plan-tool-names.md @@ -0,0 +1,82 @@ +--- +id: "02-correct-plan-tool-names" +title: "Correct plan-tool names" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-MCP-TOOL-NAMES-001 +acceptance_criteria: + - AC-TASKS-MCP-TOOL-NAMES-001.3 +system_design: + - ../../specs/tasks/system-design/mcp-tool-name-stability.md +--- + +# Task 02: Correct Plan-Tool Names + +## Summary + +Correct the two canonical MCP names in active-plan context. Add focused coverage for the model-facing instruction. + +## In scope + +- Replace `plan_get` with `get_task_plan_kandev`. +- Replace `plan_update` with `update_task_plan_kandev`. +- Add a pure-helper unit test for both names. + +## Out of scope + +- MCP registration and transport behavior. +- User-interface copy or localization catalogs. +- Backend prompt composition. + +## Acceptance + +- Active-plan context names only registered canonical plan tools. +- The focused test rejects the two obsolete names. + +## Verification + +```bash +(cd apps && pnpm --filter @kandev/web test -- --run hooks/use-message-handler.test.ts) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm exec eslint hooks/use-message-handler.ts hooks/use-message-handler.test.ts) +(cd apps/web && pnpm run i18n:check) +(cd apps/web && pnpm run i18n:ratchet) +``` + +## Files likely touched + +- `apps/web/hooks/use-message-handler.ts` +- `apps/web/hooks/use-message-handler.test.ts` + +## Dependencies + +None. + +## Risks + +- Exporting the helper increases the module surface. Keep the export limited to the existing pure function. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-TASKS-MCP-TOOL-NAMES-001` +- Prompt consistency in the MCP tool-name system design. +- Canonical names in `apps/backend/config/prompts/plan-mode.md`. + +## Results + +- Corrected the active-plan context to name `get_task_plan_kandev` and + `update_task_plan_kandev`. +- Exported the existing pure context builder only for focused unit coverage; + no user-facing copy or localization changed. +- `cd apps && pnpm --filter @kandev/web test -- --run hooks/use-message-handler.test.ts` passed with 27 tests. +- `cd apps/web && pnpm run typecheck` passed. +- `cd apps/web && pnpm exec eslint hooks/use-message-handler.ts hooks/use-message-handler.test.ts` passed. +- `cd apps/web && pnpm run i18n:check` passed. +- `cd apps/web && pnpm run i18n:ratchet` passed with 0 added violations. diff --git a/docs/plans/workflow-empty-prompt-dedup/task-03-prove-plan-mode-workflow-flow.md b/docs/plans/workflow-empty-prompt-dedup/task-03-prove-plan-mode-workflow-flow.md new file mode 100644 index 0000000000..eea178f72e --- /dev/null +++ b/docs/plans/workflow-empty-prompt-dedup/task-03-prove-plan-mode-workflow-flow.md @@ -0,0 +1,91 @@ +--- +id: "03-prove-plan-mode-workflow-flow" +title: "Prove the plan-mode workflow flow" +status: done +wave: 2 +depends_on: + - "01-deduplicate-empty-workflow-prompts" +plan: "plan.md" +requirements: + - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003 +acceptance_criteria: + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.1 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.2 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.4 +system_design: + - ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md +--- + +# Task 03: Prove the Plan-Mode Workflow Flow + +## Summary + +Add a Playwright regression for the reported task flow. The test will prove the visible transcript contains one task-description message after the later step entry, with no empty user row or extra turn. + +## In scope + +- Create a workflow whose first step differs from its automatic-start step. +- Create and start a task in plan mode through the task dialog. +- Wait for the first agent turn to finish. +- Move the idle task into the empty automatic-start step. +- Assert one visible task-description message, no empty user row, and no additional turn. + +## Out of scope + +- A mobile-only duplicate of the transport-neutral behavior. +- A change to the create-task or workflow UI. +- Assertions that plan-mode placement must use the automatic-start step. + +## Acceptance + +- The task starts at the first step in plan mode. +- Entry into the empty automatic-start step does not add another description message. +- The final assertion uses the visible transcript after backend session readiness. + +## Verification + +```bash +(cd apps/web && pnpm e2e:run tests/workflow/start-step-vs-auto-start-step.spec.ts -- --grep "does not repeat the plan-mode task description") +``` + +## Files likely touched + +- `apps/web/e2e/tests/workflow/start-step-vs-auto-start-step.spec.ts` + +## Dependencies + +- Task 01 supplies the behavior. + +## Risks + +- The first mock-agent turn can finish before browser subscriptions start. Poll the backend session state before the move. +- Remembered workflow selection can override the seeded workflow. Set the workspace selection explicitly before task creation. + +## Parallelism + +`sequential` + +## Inputs + +- The reported Backlog to In Progress reproduction. +- `apps/web/e2e/helpers/api-client.ts` workflow and session helpers. +- Existing task-dialog plan-mode coverage in `apps/web/e2e/tests/task/create-task.spec.ts`. +- Existing step-routing coverage in `start-step-vs-auto-start-step.spec.ts`. + +## Results + +- Added the requested browser regression using a dedicated two-step workflow: + plan-mode creation lands on the first step, then an idle task enters an empty + automatic-start step. +- The test waits for the first turn, backend task placement, the asynchronous + on-enter session write, and a stable transcript/turn snapshot. It then checks + the final API transcript and visible chat for exactly one task-description + message, no empty user row, and no additional turn. +- The production Vite build passed, including the pseudo-locale build used by + the E2E runner. +- `(cd apps/web && pnpm e2e:run tests/workflow/start-step-vs-auto-start-step.spec.ts -- --grep "does not repeat the plan-mode task description")` passed: 1 test. +- No mobile-only test was added because this is a transport-neutral prompt + decision with no mobile layout or interaction change; the existing shared + session surface is exercised by the browser regression. +- Task 04 supersedes the initial placement assertion. It will retain this + prompt regression by making Plan the first automatic-start destination. diff --git a/docs/plans/workflow-empty-prompt-dedup/task-04-route-immediate-plan-mode-starts.md b/docs/plans/workflow-empty-prompt-dedup/task-04-route-immediate-plan-mode-starts.md new file mode 100644 index 0000000000..5a0a7d01e6 --- /dev/null +++ b/docs/plans/workflow-empty-prompt-dedup/task-04-route-immediate-plan-mode-starts.md @@ -0,0 +1,109 @@ +--- +id: "04-route-immediate-plan-mode-starts" +title: "Route immediate plan-mode starts" +status: done +wave: 3 +depends_on: + - "03-prove-plan-mode-workflow-flow" +plan: "plan.md" +requirements: + - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004 +acceptance_criteria: + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.1 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.2 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.3 + - AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.4 +system_design: + - ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md +--- + +# Task 04: Route Immediate Plan-Mode Starts + +## Summary + +Route every immediate agent start to the workflow's first automatic-start +step. Plan mode changes agent execution but does not override this destination. + +## In scope + +- Give `StartAgent` precedence over `PlanMode` during initial step resolution. +- Retain explicit-step precedence and all destination fallbacks. +- Update backend routing coverage for combined start and plan-mode intent. +- Update desktop and mobile Playwright coverage. +- Update the public task-creation and workflow guidance. + +## Out of scope + +- Plan-only prepared sessions with `start_agent=false`. +- Task-dialog layout, labels, navigation, and touch behavior. +- Workflow editor behavior and stored workflow definitions. +- Automatic entry actions after later task moves. + +## Acceptance + +- A request with `start_agent=true` and `plan_mode=true` resolves through + `ResolveAutoStartStep` unless it supplies an explicit step. +- Desktop and mobile plan-mode starts land in the first automatic-start step. +- The duplicate-prompt E2E retains its original message and turn assertions. + +## Verification + +```bash +(cd apps/backend && go test ./internal/task/service ./internal/task/handlers -run 'TestResolveWorkflowStep_RoutesByCreateIntent|TestHTTPCreateTask_StartAgentSelectsDestinationStep|TestWSCreateTask_StartAgentSelectsDestinationStep' -count=1) +(cd apps/web && pnpm e2e:run tests/workflow/start-step-vs-auto-start-step.spec.ts) +(cd apps/web && pnpm e2e:run --project mobile-chrome tests/workflow/mobile-start-step-vs-auto-start-step.spec.ts) +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +``` + +## Files likely touched + +- `apps/backend/internal/task/service/service_tasks.go` +- `apps/backend/internal/task/service/service_tasks_step_routing_test.go` +- `apps/backend/internal/task/handlers/task_handlers_start_agent_routing_test.go` +- `apps/web/e2e/tests/workflow/start-step-vs-auto-start-step.spec.ts` +- `apps/web/e2e/tests/workflow/mobile-start-step-vs-auto-start-step.spec.ts` +- `docs/public/tasks-and-workflows.md` +- `docs/public/workflow-tips.md` + +## Dependencies + +- Task 03 provides the existing duplicate-prompt browser scenario. + +## Risks + +- Replacing the old placement assertion can remove the only browser proof for + the duplicate-prompt repair. +- A transport test can pass without exercising the combined request flags. +- The mobile test can select a remembered workflow unless it pins that choice. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004`. +- The creation destination section in the owning system design. +- Existing service, transport, and Playwright routing tests. +- The shared desktop and mobile task-create submission handler. + +## Results + +- `StartAgent` now takes precedence over `PlanMode` in initial workflow-step + resolution. Explicit steps, plan-only prepared sessions, and the existing + automatic-start fallback chain are unchanged. +- HTTP and WebSocket contract tests preserve the combined `start_agent=true` + and `plan_mode=true` intent through the task service. +- The desktop browser scenario proves normal and plan-mode starts use the same + automatic-start destination. Its duplicate-prompt regression now makes Plan + the first automatic-start destination before moving into the second empty + automatic-start step. +- The mobile browser scenario uses the phone-only Plan mode button. It checks + the persisted destination and shows the task in the In Progress phone board. +- Public task-creation and workflow guidance now document the common routing + rule. +- Focused backend verification passed: 14 tests in 2 packages. +- Desktop Playwright verification passed: 3 tests. +- Mobile Chrome Playwright verification passed: 1 test. +- Public documentation tests passed: 61 tests and 41 published pages validated. diff --git a/docs/public/tasks-and-workflows.md b/docs/public/tasks-and-workflows.md index 0537bf5e72..e9d137a1fb 100644 --- a/docs/public/tasks-and-workflows.md +++ b/docs/public/tasks-and-workflows.md @@ -74,7 +74,7 @@ Use **New Task** in the sidebar. In an open task, the **Task** split button also 6. Choose the applicable action: - **Start Plan Mode** is the primary empty-description action and creates the task through the plan-mode path. - **Start task** requires a nonempty description, creates the task, and starts its agent. This path starts in the first positional step whose entry actions include **Auto-start agent**, falling back to **Start step** when the workflow automates no step. - - **Start task in plan mode** requires a nonempty description and starts the agent with plan mode enabled. This path starts in the first positional workflow step, even if another step is marked **Start step**. + - **Start task in plan mode** requires a nonempty description and starts the agent with plan mode enabled. Like **Start task**, this path starts in the first positional step whose entry actions include **Auto-start agent**, then falls back to **Start step**. - **Create without starting agent** requires a nonempty description and starts in **Start step**. A structured ACP profile prepares the session/workspace without starting an agent turn. Passthrough/TUI is an exception: the backend launches it immediately so its native PTY exists. On mobile, the two non-primary actions are separate buttons labeled **Plan mode** and **Create only**; they have the same plan-mode and create-without-agent behavior. @@ -633,7 +633,7 @@ settled task in the still-working state. - **No workflow is available:** open the workspace's **Workflows** page. Newly added workspaces have none by default. - **No agent starts:** the empty-description **Start Plan Mode** path does not use the normal start-agent submission. To begin an agent immediately, enter a description and use **Start task** or **Start task in plan mode**; also confirm the selected profiles are healthy and compatible. -- **Task starts in the wrong step:** the destination depends on the action. **Create without starting agent** uses **Start step** with first-step fallback; **Start task** uses the first **Auto-start agent** step and falls back to **Start step**; **Start task in plan mode** deliberately uses the first positional step. An explicit `workflow_step_id` from the creator outranks all three. +- **Task starts in the wrong step:** the destination depends on whether an agent starts immediately. **Create without starting agent** uses **Start step** with first-step fallback. **Start task** and **Start task in plan mode** use the first **Auto-start agent** step, then fall back to **Start step**. An explicit `workflow_step_id` from the creator outranks these defaults. - **A task moves unexpectedly:** inspect **On Turn Start**, **On Turn Complete**, child completion, entry actions, and the destination step's entry actions. - **A task stays after a cancel:** check for a pending clarification, the cancelled-turn completion policy, an absent or blocked transition, a queued WIP card, or an invalid target left by an older definition. - **Move rejected:** check the target WIP limit and whether the task is already counted there. diff --git a/docs/public/workflow-tips.md b/docs/public/workflow-tips.md index b442f33042..ec3adc83cd 100644 --- a/docs/public/workflow-tips.md +++ b/docs/public/workflow-tips.md @@ -126,7 +126,7 @@ Workflow-level settings include the name and default agent profile. Each step ca |---------|----------| | Name and color | Board label and presentation. Color is stored as a CSS utility class. | | Prompt | Step-specific agent prompt. `{{task_prompt}}` inserts the task description. Type `@` to reference a saved prompt by name. | -| Start step | Where a task is created when no agent starts with it. The editor keeps at most one. If none is set, task creation falls back to the first step by position. Creating a task that starts an agent immediately uses the first Auto-start agent step instead, so a Start step with no entry actions is a genuine parking column. | +| Start step | Where a task is created when no agent starts with it. The editor keeps at most one. If none is set, task creation falls back to the first step by position. Creating a task that starts an agent immediately uses the first Auto-start agent step instead, including in plan mode. Thus, a Start step with no entry actions is a genuine parking column. | | Auto-start agent | Adds `auto_start_agent` to `on_enter`. It still needs a valid agent and executor configuration. | | Plan mode | Adds `enable_plan_mode` on entry. Add the matching disable behavior on completion or exit when later steps should edit files. | | Reset agent context | Starts the step with fresh conversation context. It is disabled when the step changes agent profile because the destination step's session start setting controls whether that switch reuses or creates a conversation. | diff --git a/docs/specs/tasks/requirements/workflow-step-agent-start-ownership.md b/docs/specs/tasks/requirements/workflow-step-agent-start-ownership.md index 6f763e2a05..314bdde706 100644 --- a/docs/specs/tasks/requirements/workflow-step-agent-start-ownership.md +++ b/docs/specs/tasks/requirements/workflow-step-agent-start-ownership.md @@ -2,6 +2,7 @@ status: draft system: tasks created: 2026-08-05 +updated: 2026-09-03 owners: - Kandev --- @@ -34,6 +35,59 @@ This document is the migrated task-system source for the capability. The source - **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.5:** When a prompt waits for an unresolved dispatch-only completion, the wait shall end within a bounded period and release session admission. - **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6:** A delayed completion from the replaced turn shall not complete or release a later prompt generation. +### REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003: Single-use task-description fallback + +**Intent:** Start an unprompted workflow session from its task description without +repeating that description during later workflow entries. + +#### Acceptance criteria + +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.1:** When an unprompted +session enters an automatic-start step with no step prompt, the system shall send +the task description once. Admission of this fallback shall be atomic with the +session's first-prompt boundary. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.2:** When a prompted session +enters an automatic-start step with no step prompt, the system shall not send the +task description again. A concurrent direct user prompt and automatic fallback +shall not both qualify as the first prompt. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.3:** When an +automatic-start step has a step prompt, the system shall send the evaluated step +prompt regardless of earlier session prompts. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.4:** When fallback +suppression removes all textual prompt content and no queued attachment remains, +the system shall not create a user message or dispatch an empty agent turn. A +queued attachment-only handoff shall remain durable user input and shall be +dispatched with its attachment metadata. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.5:** The automatic +`on_enter` path and the explicit workflow-step launch path shall use the same +task-description fallback rule. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.6:** ACP and passthrough +sessions shall use the same task-description fallback rule. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003.7:** If prompt-history +inspection fails, the system shall stop the automatic prompt and expose the +existing workflow-start error behavior. + +### REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004: Immediate-launch placement + +**Intent:** Place a new task in the workflow step that owns its requested agent +start, regardless of the selected agent mode. + +#### Acceptance criteria + +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.1:** When task creation +starts an agent immediately, the system shall place the task in the first +positional step with an `auto_start_agent` entry action. Plan mode shall not +change this destination. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.2:** When no workflow step +has an `auto_start_agent` entry action, an immediate agent start shall use the +configured start step. If no start step exists, it shall use the first +positional step. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.3:** When a creator supplies +an explicit workflow step, the system shall use that step instead of an +intent-derived destination. +- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004.4:** Desktop and mobile task +creation shall apply the same immediate-launch placement rule. + ## Migrated source detail ## Why @@ -152,3 +206,6 @@ read, which is a design change beyond this repair. force-kill rather than an independent defect. - Changing task `FAILED` semantics, the reconciliation path, or the lazy-recovery boot that follows a backend restart. +- Changing placement for a plan-only prepared session that does not request an + immediate agent start. This path continues to use the first workflow step by + position. diff --git a/docs/specs/tasks/system-design/workflow-step-agent-start-ownership.md b/docs/specs/tasks/system-design/workflow-step-agent-start-ownership.md index c947f1ad93..127d5c1c51 100644 --- a/docs/specs/tasks/system-design/workflow-step-agent-start-ownership.md +++ b/docs/specs/tasks/system-design/workflow-step-agent-start-ownership.md @@ -4,6 +4,8 @@ system: tasks requirements: - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-001 - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002 + - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003 + - REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004 --- # Workflow Step Agent Start Ownership System Design @@ -22,6 +24,8 @@ The design preserves runtime configuration through the existing reset contract. | --- | --- | | `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-001` | [Session states](#session-states) | | `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002` | [Active-turn reset flow](#active-turn-reset-flow), [Bounded predecessor wait](#bounded-predecessor-wait) | +| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003` | [Prompt fallback ownership](#prompt-fallback-ownership), [Prompt-history contract](#prompt-history-contract), [Workflow-entry prompt flow](#workflow-entry-prompt-flow) | +| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004` | [Creation destination routing](#creation-destination-routing) | ## Components and responsibilities @@ -33,6 +37,40 @@ The cancellation coordinator owns active-turn quiescence. It uses the internal c `lifecycle.SessionManager` owns prompt serialization and the dispatch-only completion barrier. Prompt generations continue to identify completion ownership. +The orchestrator owns the task-description fallback decision. The task +repository owns the durable prompt counter and its atomic initial-fallback +claim. Direct user-message persistence and the fallback claim use the same +per-session write boundary. + +The task service owns the initial workflow-step destination. It derives the +destination from explicit step selection and agent-start intent. Agent mode is +an execution setting and cannot override an immediate start. + +## Creation destination routing + +`task.Service.resolveWorkflowStep` applies this precedence: + +1. Use an explicit `workflow_step_id` without further resolution. +2. If `StartAgent` is true, call `ResolveAutoStartStep` regardless of + `PlanMode`. +3. If only `PlanMode` is true, call `ResolveFirstStep` for the existing + plan-only prepared-session path. +4. Otherwise, call `ResolveStartStep`. + +`ResolveAutoStartStep` selects the first positional step whose `on_enter` +actions include `auto_start_agent`. If no such step exists, it calls +`ResolveStartStep`. That resolver uses the configured start step and then the +first positional step as its fallback. + +The HTTP and WebSocket create transports preserve both `start_agent` and +`plan_mode` in `CreateTaskRequest`. The service uses `start_agent` as the launch +intent before session preparation or launch begins. + +Desktop and mobile use the same task-create payload builder and submission +handler. The implementation does not change their layout, labels, touch +targets, or navigation. Separate Playwright scenarios exercise the desktop +split-menu action and the mobile plan-mode action. + ## Session states A `CREATED` session has no conversation to reset. The workflow reset skips provider work and leaves the first start to `auto_start_agent`. @@ -77,6 +115,69 @@ An unnumbered completion cannot release a pending numbered dispatch-only prompt. Internal cancellation finishes or escalates the old generation before provider replacement. The reset does not drain a completion signal from an active generation. +## Prompt fallback ownership + +A non-empty `WorkflowStep.Prompt` is work for each applicable step entry. The +orchestrator evaluates and dispatches this prompt with the current placeholder +rules. + +An empty `WorkflowStep.Prompt` does not define new step work. For an unprompted +session, the task description supplies the first prompt. After that first user +prompt, the task description is no longer a workflow-entry prompt. + +This rule changes only the empty-step fallback. It preserves workflow-level +instructions, prompt reference expansion, plan-mode context, and queued handoff +behavior. + +## Prompt-history contract + +`task_session_prompt_seq.last_seq` is the durable session prompt counter. A +positive value is an accepted user-prompt ordinal. The zero value is reserved +for an admitted empty-step fallback whose visible message has not been written +yet. The repository also exposes an atomic insert-if-absent claim for the +empty-step task-description fallback. A direct user-message write and this +claim take the same per-session write boundary, so the first committed +admission wins. + +The counter does not decrease after message deletion. This property prevents a +deleted transcript row from making the task description eligible again. + +The task repository exposes a bounded existence query for this state and the +atomic fallback claim. The existence of the counter row, including a +zero-valued reservation marker, is the history signal. The orchestrator does not +load the complete session transcript to make the fallback decision. The +replay-safe counter table has no foreign key, so session deletion explicitly +removes the counter before commit. This change needs no schema migration. + +## Workflow-entry prompt flow + +`launchAfterOnEnterDispatch` and `StartSessionForWorkflowStep` use one prompt +composition helper. The helper applies these rules: + +1. If `WorkflowStep.Prompt` is non-empty, retain the task description for + placeholder evaluation. Non-empty prompts, including `{{task_prompt}}`, keep + their existing semantics. +2. If the step prompt is empty, atomically claim the initial fallback slot. +3. If the claim succeeds, use the task description as the base prompt. +4. If the claim is already taken, use an empty base prompt. +5. Build the workflow prompt with the existing workflow instructions and + reference expansion. + +The `on_enter` caller applies this result before it selects ACP or passthrough +delivery. Thus, both transports use the same fallback rule. + +Before emptiness is decided, the explicit and automatic paths apply the same +plan-mode and session-configuration transforms that prompt dispatch applies. +The ACP path still lets `autoStartStepPrompt` merge a queued handoff. If the +merged result has no content or attachments, it returns without a message or +agent dispatch. An attachment-only handoff is admitted, persisted with its +attachment metadata, and dispatched even when its text is empty. A started +passthrough session drains the queued handoff before returning from a suppressed +empty-step decision. + +The explicit workflow-step launch keeps its existing resume and session-setting +behavior. It does not call `PromptTask` when the composed prompt is empty. + ## Failure and recovery If internal cancellation fails, the provider session remains unchanged. The workflow entry records the reset error and does not send the automatic prompt. @@ -85,8 +186,16 @@ If provider reset or configuration restoration fails, the existing reset reconci If a stale predecessor barrier times out, the successor prompt fails before dispatch. The session guard becomes available for cancellation and recovery. +If the prompt-history read or atomic claim fails, prompt composition returns an +error. The automatic entry uses the existing waiting-state recovery. An +explicit workflow-step launch returns the error to its caller. + This repair does not reconcile sessions that became stuck before the new boundary existed. Users can replace such a session with a new session. +The prompt counter and fallback claim are durable across backend restarts. A +restart cannot make an earlier task description eligible for another fallback +dispatch, and deleting/recreating a session ID starts a new prompt boundary. + ## Observability The workflow reset logs the task, session, workflow step, and execution identifiers. Internal cancellation uses the existing cancellation and escalation logs.