From f8654880f3de8dcb011864eaa322edf9a4deaa20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Thu, 21 May 2026 22:58:14 +0100 Subject: [PATCH] fix(kanban): stop stuck spinner by writing task.state=REVIEW on engine on_turn_complete (#1001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): stop kanban spinner when primary session is terminal The kanban card spinner was gated only on `task.state` being IN_PROGRESS or SCHEDULING. Workflows leave a task in the IN_PROGRESS column after the agent's session reaches a terminal state (COMPLETED/FAILED/CANCELLED) so the user can review and move it manually, which made the loading icon spin indefinitely. The frontend already receives `primary_session_state` over SSR and WS; gate the spinner on that not being terminal. Fixes #985. Co-Authored-By: Claude Opus 4.7 * fix(orchestrator): write task.state=REVIEW after engine on_turn_complete transition The non-engine path (setSessionWaitingForInput → writeTaskReviewState) has always written tasks.state = REVIEW when the agent's session paused for input. The workflow-engine on_turn_complete path forgot the task-state write: it persisted the new workflow step + flipped the session to WAITING_FOR_INPUT but left tasks.state stale at IN_PROGRESS. That left the kanban card stuck in IN_PROGRESS — with a spinning loader — even after the workflow had moved it into the "Review" column. Mirror the non-engine path: call writeTaskReviewState inside the RUNNING/STARTING branch of applyEngineTransition. Add a regression test that asserts tasks.state = REVIEW after an engine-driven on_turn_complete transition. Also tighten the frontend gate added in the previous commit to suppress the spinner for any non-active primary-session state (WAITING_FOR_INPUT, IDLE in addition to COMPLETED/FAILED/CANCELLED). The backend fix is the real correction; the broader frontend gate stays as defense-in-depth against future state-sync drift. Refs #985. Co-Authored-By: Claude Opus 4.7 * test(orchestrator): drop redundant time.Sleep in engine on_turn_complete test writeTaskReviewState and ApplyTransition both run synchronously inside applyEngineTransition (before processOnEnter's goroutine), so the task-state and step assertions need no async wait. assertSessionState already has its own 500ms polling loop for the WAITING_FOR_INPUT flip. Co-Authored-By: Claude Opus 4.7 --------- Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 4.7 --- .../orchestrator/event_handlers_workflow.go | 11 ++++ .../orchestrator/workflow_e2e_test.go | 54 +++++++++++++++++++ apps/web/components/kanban-card-content.tsx | 10 ++-- apps/web/components/kanban-card.tsx | 6 +++ apps/web/lib/ui/state-icons.test.tsx | 47 +++++++++++++++- apps/web/lib/ui/state-icons.tsx | 30 +++++++++++ 6 files changed, 154 insertions(+), 4 deletions(-) diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index 6fad53016e..39d76de48c 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -1828,9 +1828,20 @@ func (s *Service) applyEngineTransition( // Flip to WAITING_FOR_INPUT so that autoStartStepPrompt in processOnEnter sends // the prompt directly instead of queueing it — the queue would never be drained // because handleAgentReady already returned. + // + // Mirror setSessionWaitingForInput's task-state side effect: write + // tasks.state = REVIEW so the kanban card drops out of IN_PROGRESS. Without + // this, an engine-driven on_turn_complete transition would persist the + // new workflow step + flip the session but leave tasks.state stale at + // IN_PROGRESS, leaving the spinner spinning in the new column even though + // the agent has paused. If the target step's on_enter starts another agent, + // setSessionRunning will flip tasks.state back to IN_PROGRESS — the + // REVIEW write is a safe intermediate that any active-running follow-up + // will overwrite. if session.State == models.TaskSessionStateRunning || session.State == models.TaskSessionStateStarting { s.updateTaskSessionState(ctx, taskID, session.ID, models.TaskSessionStateWaitingForInput, "", false, session) session.State = models.TaskSessionStateWaitingForInput + s.writeTaskReviewState(ctx, taskID) } // Launch processOnEnter asynchronously to avoid blocking the stream reader goroutine. diff --git a/apps/backend/internal/orchestrator/workflow_e2e_test.go b/apps/backend/internal/orchestrator/workflow_e2e_test.go index c4c187997b..2aee826cfe 100644 --- a/apps/backend/internal/orchestrator/workflow_e2e_test.go +++ b/apps/backend/internal/orchestrator/workflow_e2e_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + v1 "github.com/kandev/kandev/pkg/api/v1" + "github.com/kandev/kandev/internal/orchestrator/executor" "github.com/kandev/kandev/internal/orchestrator/messagequeue" "github.com/kandev/kandev/internal/task/models" @@ -408,3 +410,55 @@ func assertSessionState(t *testing.T, ctx context.Context, repo sessionExecutorS time.Sleep(10 * time.Millisecond) } } + +// Regression for #985: an on_turn_complete engine transition flips the +// session to WAITING_FOR_INPUT and moves the task to the next step, but the +// pre-fix code path forgot to write tasks.state = REVIEW. That left the +// kanban card stuck in IN_PROGRESS — with a spinning loader — even though +// the agent had paused and the card had already moved into the "Review" +// column. Mirrors what setSessionWaitingForInput (the non-engine path) +// has always done via writeTaskReviewState. +// +// The chosen transition is "New Step" → "Done": "Done" has no on_enter, so +// no follow-up action can mask the missing write (e.g. auto_start_agent +// would otherwise overwrite tasks.state back to IN_PROGRESS via +// setSessionRunning). +func TestProcessOnTurnCompleteViaEngine_WritesTaskStateReview(t *testing.T) { + ctx := context.Background() + + sg, nameToID := buildWorkflowFromJSON(t, developmentWorkflowJSON) + repo := setupTestRepo(t) + seedSession(t, repo, "t1", "s1", nameToID["New Step"]) + setSessionExecID(t, repo, "s1", "exec-1") + setSessionState(t, ctx, repo, "s1", models.TaskSessionStateRunning) + + agentMgr := &mockAgentManager{repoForExecutionLookup: repo} + taskRepo := newMockTaskRepo() + svc := createEngineService(t, repo, sg, agentMgr) + svc.taskRepo = taskRepo + + session, err := repo.GetTaskSession(ctx, "s1") + if err != nil { + t.Fatalf("failed to load session: %v", err) + } + + transitioned := svc.processOnTurnCompleteViaEngine(ctx, "t1", session) + if !transitioned { + t.Fatalf("expected a transition from New Step → Done, got none") + } + + // writeTaskReviewState and ApplyTransition both run synchronously inside + // applyEngineTransition (before processOnEnter's goroutine), so the + // task-state and step assertions need no async wait. assertSessionState + // has its own polling loop for the WAITING_FOR_INPUT flip. + assertStepByName(t, ctx, repo, "s1", "Done", nameToID) + assertSessionState(t, ctx, repo, "s1", models.TaskSessionStateWaitingForInput) + + got, ok := taskRepo.updatedStates["t1"] + if !ok { + t.Fatalf("expected tasks.state to be updated, but UpdateTaskState was never called") + } + if got != v1.TaskStateReview { + t.Errorf("tasks.state = %q, want %q (the kanban card would still show the spinner)", got, v1.TaskStateReview) + } +} diff --git a/apps/web/components/kanban-card-content.tsx b/apps/web/components/kanban-card-content.tsx index 9fd3a94fbd..847a434f73 100644 --- a/apps/web/components/kanban-card-content.tsx +++ b/apps/web/components/kanban-card-content.tsx @@ -17,7 +17,11 @@ import { import { useAppStore } from "@/components/state-provider"; import { RemoteCloudTooltip } from "@/components/task/remote-cloud-tooltip"; import { useTaskPendingClarification } from "@/hooks/use-task-pending-clarification"; -import { getTaskStateIcon, shouldUseQuestionTaskIcon } from "@/lib/ui/state-icons"; +import { + getTaskStateIcon, + shouldShowTaskRunningSpinner, + shouldUseQuestionTaskIcon, +} from "@/lib/ui/state-icons"; import { cn } from "@/lib/utils"; import { needsAction } from "@/lib/utils/needs-action"; import type { Task } from "@/components/kanban-card"; @@ -188,14 +192,14 @@ function KanbanCardActions({ const effectiveMenuOpen = menuOpen || Boolean(isDeleting) || Boolean(isArchiving); const hasPendingClarificationRequest = useTaskPendingClarification(task.primarySessionId); const showQuestionIcon = shouldUseQuestionTaskIcon(task.state, hasPendingClarificationRequest); + const showRunningSpinner = shouldShowTaskRunningSpinner(task.state, task.primarySessionState); const statusIcon = getTaskStateIcon(task.state, "h-4 w-4", hasPendingClarificationRequest); const hasKnownSession = Boolean(task.primarySessionId) || Boolean(task.sessionCount && task.sessionCount > 0); return (
- {(task.state === "IN_PROGRESS" || task.state === "SCHEDULING" || showQuestionIcon) && - statusIcon} + {(showRunningSpinner || showQuestionIcon) && statusIcon} {showMaximizeButton && onOpenFullPage && hasKnownSession && (