diff --git a/.agents/skills/verify/SKILL.md b/.agents/skills/verify/SKILL.md index 6bf8b4536b..005af6dc41 100644 --- a/.agents/skills/verify/SKILL.md +++ b/.agents/skills/verify/SKILL.md @@ -9,15 +9,34 @@ Run the full verification pipeline for the Kandev monorepo, then fix any issues ## Steps -1. **Format first** (prevents formatter-induced lint failures): +1. **Rebase on main** (ensures CI parity — CI merges main into the PR branch): + ```bash + git fetch origin main --quiet + ``` + - If the current branch is `main`, skip this step. + - If the branch is stacked on another feature branch (not main), skip this step. + Detect: `git log --oneline --first-parent origin/main..HEAD | tail -1` — if the first commit's parent is not on `origin/main`, it's stacked. + Simpler heuristic: check `git merge-base --is-ancestor origin/main HEAD` — if false, the branch diverged from a non-main base; skip rebase. + - Otherwise, rebase: + ```bash + git rebase origin/main + ``` + - If the rebase has conflicts, resolve them: + 1. For each conflicted file, read the file and understand both sides of the conflict + 2. Resolve by keeping the correct combination of both changes (don't just pick one side) + 3. `git add ` then `git rebase --continue` + 4. Repeat for each conflicting commit until the rebase completes + 5. If a conflict is too ambiguous to resolve confidently, abort (`git rebase --abort`) and ask the user for guidance + +2. **Format** (prevents formatter-induced lint failures): - `make -C apps/backend fmt` - `cd apps && pnpm format` -2. **Verify in parallel** where possible: +3. **Verify in parallel** where possible: - `make -C apps/backend test lint` - `cd apps && pnpm --filter @kandev/web typecheck && pnpm --filter @kandev/web lint` -3. **Fix issues** — do NOT just report them: +4. **Fix issues** — do NOT just report them: - Read each failing file at the reported line number - Fix the root cause (don't suppress warnings or add ignores) - Common fixes: @@ -30,6 +49,6 @@ Run the full verification pipeline for the Kandev monorepo, then fix any issues - **Test failures**: read the test, understand the assertion, fix the code (not the test) unless the test is outdated - After fixing, re-run only the failed command to confirm the fix -4. **Repeat** steps 2-3 until all commands pass. If a fix introduces new issues, address those too. +5. **Repeat** steps 3-4 until all commands pass. If a fix introduces new issues, address those too. -5. **Done** when all four pass cleanly: fmt, typecheck, test, lint. +6. **Done** when all pass cleanly: rebase, fmt, typecheck, test, lint. diff --git a/apps/backend/cmd/kandev/adapters.go b/apps/backend/cmd/kandev/adapters.go index f149866c48..eced497c3a 100644 --- a/apps/backend/cmd/kandev/adapters.go +++ b/apps/backend/cmd/kandev/adapters.go @@ -688,3 +688,18 @@ func (a *turnServiceAdapter) GetActiveTurn(ctx context.Context, sessionID string func newTurnServiceAdapter(svc *taskservice.Service) *turnServiceAdapter { return &turnServiceAdapter{svc: svc} } + +// taskSessionCheckerAdapter adapts the task repository for github.TaskSessionChecker. +type taskSessionCheckerAdapter struct { + repo interface { + ListTaskSessions(ctx context.Context, taskID string) ([]*models.TaskSession, error) + } +} + +func (a *taskSessionCheckerAdapter) HasTaskSessions(ctx context.Context, taskID string) (bool, error) { + sessions, err := a.repo.ListTaskSessions(ctx, taskID) + if err != nil { + return false, err + } + return len(sessions) > 0, nil +} diff --git a/apps/backend/cmd/kandev/gateway.go b/apps/backend/cmd/kandev/gateway.go index a27255f0c6..66c9b2ee70 100644 --- a/apps/backend/cmd/kandev/gateway.go +++ b/apps/backend/cmd/kandev/gateway.go @@ -121,6 +121,7 @@ func provideGateway( }) } gitHandlers.SetOnGitOperationFailed(func(ctx context.Context, sessionID, taskID, operation, errorOutput string) { + fixPrompt := fmt.Sprintf("The git %s command failed with the following error:\n\n```\n%s\n```\n\nPlease fix the issues reported above.", operation, errorOutput) if _, err := taskSvc.CreateMessage(ctx, &taskservice.CreateMessageRequest{ TaskSessionID: sessionID, TaskID: taskID, @@ -134,6 +135,15 @@ func provideGateway( "session_id": sessionID, "task_id": taskID, "variant": "error", + "actions": []map[string]interface{}{{ + "type": "ws_request", "label": "Fix", "icon": "sparkles", + "tooltip": "Ask the agent to fix the git error", + "test_id": "git-fix-button", + "params": map[string]interface{}{ + "method": "message.add", + "payload": map[string]interface{}{"task_id": taskID, "session_id": sessionID, "content": fixPrompt}, + }, + }}, }, }); err != nil { log.Error("failed to create git operation error message", diff --git a/apps/backend/cmd/kandev/main.go b/apps/backend/cmd/kandev/main.go index 9f34e11973..2d853062f5 100644 --- a/apps/backend/cmd/kandev/main.go +++ b/apps/backend/cmd/kandev/main.go @@ -313,6 +313,8 @@ func startAgentInfrastructure( // Wire GitHub service into orchestrator for PR auto-detection on push if services.GitHub != nil { orchestratorSvc.SetGitHubService(services.GitHub) + services.GitHub.SetTaskDeleter(services.Task) + services.GitHub.SetTaskSessionChecker(&taskSessionCheckerAdapter{repo: repos.Task}) log.Info("GitHub service configured for orchestrator (PR auto-detection enabled)") // Start GitHub background poller diff --git a/apps/backend/internal/github/controller.go b/apps/backend/internal/github/controller.go index 0e85ef7aa0..c66b580e5d 100644 --- a/apps/backend/internal/github/controller.go +++ b/apps/backend/internal/github/controller.go @@ -251,7 +251,9 @@ func (c *Controller) httpTriggerReviewWatch(ctx *gin.Context) { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - ctx.JSON(http.StatusOK, gin.H{"new_prs": len(newPRs), "prs": newPRs}) + // Clean up tasks for merged/closed PRs that haven't been started. + cleaned, _ := c.service.CleanupMergedReviewTasks(ctx.Request.Context(), watch) + ctx.JSON(http.StatusOK, gin.H{"new_prs": len(newPRs), "prs": newPRs, "cleaned": cleaned}) } func (c *Controller) httpTriggerAllReviewChecks(ctx *gin.Context) { diff --git a/apps/backend/internal/github/poller.go b/apps/backend/internal/github/poller.go index 2cbd4a6939..5b18fe59c2 100644 --- a/apps/backend/internal/github/poller.go +++ b/apps/backend/internal/github/poller.go @@ -264,5 +264,13 @@ func (p *Poller) checkReviewWatches(ctx context.Context) { zap.String("title", pr.Title)) p.service.publishNewReviewPREvent(ctx, watch, pr) } + // Clean up tasks for merged/closed PRs that the user hasn't opened. + if cleaned, err := p.service.CleanupMergedReviewTasks(ctx, watch); err != nil { + p.logger.Warn("failed to cleanup merged review tasks", + zap.String("watch_id", watch.ID), zap.Error(err)) + } else if cleaned > 0 { + p.logger.Info("cleaned up merged review tasks", + zap.String("watch_id", watch.ID), zap.Int("deleted", cleaned)) + } } } diff --git a/apps/backend/internal/github/service.go b/apps/backend/internal/github/service.go index a332246740..8e54bf8a2a 100644 --- a/apps/backend/internal/github/service.go +++ b/apps/backend/internal/github/service.go @@ -21,15 +21,27 @@ const ( AuthMethodPAT = "pat" ) +// TaskDeleter deletes tasks by ID. Used for cleaning up merged PR tasks. +type TaskDeleter interface { + DeleteTask(ctx context.Context, taskID string) error +} + +// TaskSessionChecker checks if a task has any sessions (user interacted with it). +type TaskSessionChecker interface { + HasTaskSessions(ctx context.Context, taskID string) (bool, error) +} + // Service coordinates GitHub integration operations. type Service struct { - mu sync.Mutex - client Client - authMethod string - secrets SecretProvider - store *Store - eventBus bus.EventBus - logger *logger.Logger + mu sync.Mutex + client Client + authMethod string + secrets SecretProvider + store *Store + eventBus bus.EventBus + logger *logger.Logger + taskDeleter TaskDeleter + taskSessionChecker TaskSessionChecker } // NewService creates a new GitHub service. @@ -44,6 +56,12 @@ func NewService(client Client, authMethod string, secrets SecretProvider, store } } +// SetTaskDeleter sets the task deletion dependency for cleanup operations. +func (s *Service) SetTaskDeleter(d TaskDeleter) { s.taskDeleter = d } + +// SetTaskSessionChecker sets the session checker for cleanup operations. +func (s *Service) SetTaskSessionChecker(c TaskSessionChecker) { s.taskSessionChecker = c } + // Client returns the underlying GitHub client (may be nil if not authenticated). func (s *Service) Client() Client { return s.client @@ -777,6 +795,84 @@ func (s *Service) RecordReviewPRTask(ctx context.Context, watchID, repoOwner, re return s.store.CreateReviewPRTask(ctx, rpt) } +// CleanupMergedReviewTasks checks PRs tracked by a review watch and deletes +// tasks whose PRs are merged/closed. Returns the number of tasks deleted. +func (s *Service) CleanupMergedReviewTasks(ctx context.Context, watch *ReviewWatch) (int, error) { + if s.client == nil || s.taskDeleter == nil { + return 0, nil + } + prTasks, err := s.store.ListReviewPRTasksByWatch(ctx, watch.ID) + if err != nil { + return 0, fmt.Errorf("list review PR tasks: %w", err) + } + deleted := 0 + for _, rpt := range prTasks { + shouldDelete, reason := s.shouldDeleteReviewTask(ctx, rpt) + if !shouldDelete { + continue + } + if err := s.taskDeleter.DeleteTask(ctx, rpt.TaskID); err != nil { + s.logger.Warn("failed to delete review PR task", + zap.String("task_id", rpt.TaskID), zap.Error(err)) + continue + } + _ = s.store.DeleteReviewPRTask(ctx, rpt.ID) + s.logger.Info("deleted review task", + zap.String("task_id", rpt.TaskID), + zap.String("reason", reason), + zap.Int("pr_number", rpt.PRNumber), + zap.String("repo", rpt.RepoOwner+"/"+rpt.RepoName)) + deleted++ + } + return deleted, nil +} + +// shouldDeleteReviewTask checks if a review PR task should be cleaned up. +// Returns true + reason if the PR is done (merged/closed/approved) AND the user +// hasn't interacted with the task yet (no sessions). Tasks with sessions are +// preserved so the user can see the PR merged banner and their work history. +func (s *Service) shouldDeleteReviewTask(ctx context.Context, rpt *ReviewPRTask) (bool, string) { + feedback, err := s.client.GetPRFeedback(ctx, rpt.RepoOwner, rpt.RepoName, rpt.PRNumber) + if err != nil { + s.logger.Debug("failed to fetch PR feedback for cleanup", + zap.Int("pr_number", rpt.PRNumber), zap.Error(err)) + return false, "" + } + if feedback.PR == nil { + return false, "" + } + var reason string + if feedback.PR.State == prStateMerged || feedback.PR.State == prStateClosed { + reason = "pr_merged_or_closed" + } else { + // Check if the authenticated user already approved the PR on GitHub. + user, _ := s.client.GetAuthenticatedUser(ctx) + for _, review := range feedback.Reviews { + if review.State == "APPROVED" && review.Author == user { + reason = "pr_approved_by_user" + break + } + } + } + if reason == "" { + return false, "" + } + // Don't delete tasks the user has interacted with (has sessions). + // Those show the PR merged banner instead. + if s.taskSessionChecker != nil { + hasSessions, err := s.taskSessionChecker.HasTaskSessions(ctx, rpt.TaskID) + if err != nil { + s.logger.Debug("failed to check task sessions", + zap.String("task_id", rpt.TaskID), zap.Error(err)) + return false, "" + } + if hasSessions { + return false, "" + } + } + return true, reason +} + // TriggerAllReviewChecks triggers all review watches for a workspace. func (s *Service) TriggerAllReviewChecks(ctx context.Context, workspaceID string) (int, error) { watches, err := s.store.ListReviewWatches(ctx, workspaceID) diff --git a/apps/backend/internal/github/store.go b/apps/backend/internal/github/store.go index 5d5a0c070e..c52abf9c36 100644 --- a/apps/backend/internal/github/store.go +++ b/apps/backend/internal/github/store.go @@ -363,6 +363,21 @@ func (s *Store) HasReviewPRTask(ctx context.Context, reviewWatchID, repoOwner, r return count > 0, err } +// ListReviewPRTasksByWatch lists all dedup records for a given review watch. +func (s *Store) ListReviewPRTasksByWatch(ctx context.Context, watchID string) ([]*ReviewPRTask, error) { + var tasks []*ReviewPRTask + err := s.ro.SelectContext(ctx, &tasks, + `SELECT id, review_watch_id, repo_owner, repo_name, pr_number, pr_url, task_id, created_at + FROM github_review_pr_tasks WHERE review_watch_id = ?`, watchID) + return tasks, err +} + +// DeleteReviewPRTask deletes a dedup record by ID. +func (s *Store) DeleteReviewPRTask(ctx context.Context, id string) error { + _, err := s.db.ExecContext(ctx, `DELETE FROM github_review_pr_tasks WHERE id = ?`, id) + return err +} + // --- Stats queries --- // prStatsQuery builds parameterised SELECT queries against the github_task_prs table. diff --git a/apps/backend/internal/orchestrator/event_handlers_agent.go b/apps/backend/internal/orchestrator/event_handlers_agent.go index f8826e137f..1c2e2a12db 100644 --- a/apps/backend/internal/orchestrator/event_handlers_agent.go +++ b/apps/backend/internal/orchestrator/event_handlers_agent.go @@ -467,12 +467,13 @@ func (s *Service) handleRecoverableFailure(ctx context.Context, data watcher.Age statusMsg := fmt.Sprintf("Agent encountered an error: %s", displayMsg) + hasResumeToken := s.wasResumeAttempt(ctx, data.SessionID) meta := map[string]interface{}{ "variant": "error", "recovery_actions": true, "session_id": data.SessionID, "task_id": data.TaskID, - "has_resume_token": s.wasResumeAttempt(ctx, data.SessionID), + "has_resume_token": hasResumeToken, "is_auth_error": authErr, } @@ -483,6 +484,9 @@ func (s *Service) handleRecoverableFailure(ctx context.Context, data watcher.Age } } + // Build generic actions array for the frontend ActionMessage component. + meta["actions"] = buildRecoveryActions(data.TaskID, data.SessionID, hasResumeToken, authErr) + if err := s.messageCreator.CreateSessionMessage( ctx, data.TaskID, @@ -533,6 +537,38 @@ func (s *Service) handleAgentStartFailed(ctx context.Context, taskID, sessionID, return true } +// buildRecoveryActions creates the generic actions array for agent error recovery. +func buildRecoveryActions(taskID, sessionID string, hasResumeToken, isAuthError bool) []map[string]interface{} { + recoverPayload := func(action string) map[string]interface{} { + return map[string]interface{}{ + "method": "session.recover", + "payload": map[string]interface{}{"task_id": taskID, "session_id": sessionID, "action": action}, + } + } + actions := []map[string]interface{}{} + if hasResumeToken { + actions = append(actions, map[string]interface{}{ + "type": "ws_request", "label": "Resume session", "icon": "refresh", + "tooltip": "Re-launch with resume flag — keeps all previous messages and context", + "test_id": "recovery-resume-button", "params": recoverPayload("resume"), + }) + } + label := "Start fresh session" + if isAuthError { + label = "Restart session" + } + testID := "recovery-fresh-button" + if isAuthError { + testID = "recovery-restart-button" + } + actions = append(actions, map[string]interface{}{ + "type": "ws_request", "label": label, "icon": "player-play", + "tooltip": "New agent process on the same workspace — no previous conversation context", + "test_id": testID, "params": recoverPayload("fresh_start"), + }) + return actions +} + // handleAgentStopped handles agent stopped events (manual stop or cancellation) func (s *Service) handleAgentStopped(ctx context.Context, data watcher.AgentEventData) { s.logger.Info("handling agent stopped", diff --git a/apps/backend/internal/orchestrator/event_handlers_streaming.go b/apps/backend/internal/orchestrator/event_handlers_streaming.go index 26645d0f4e..f1ac8e8093 100644 --- a/apps/backend/internal/orchestrator/event_handlers_streaming.go +++ b/apps/backend/internal/orchestrator/event_handlers_streaming.go @@ -401,6 +401,10 @@ func (s *Service) updateTaskSessionState(ctx context.Context, taskID, sessionID if len(session.Metadata) > 0 { eventData["session_metadata"] = session.Metadata } + // Propagate toast suppression flag set by failure handlers (e.g. missing branch). + if suppressed, ok := s.suppressToast.LoadAndDelete(sessionID); ok && suppressed.(bool) { + eventData["suppress_toast"] = true + } _ = s.eventBus.Publish(ctx, events.TaskSessionStateChanged, bus.NewEvent(events.TaskSessionStateChanged, "task-session", eventData)) } } diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 92cb27220d..2afdcf231b 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -320,6 +320,10 @@ type SessionStateChangeFunc func(ctx context.Context, taskID, sessionID string, // and the executor should skip its default FAILED state updates. type AgentStartFailedFunc func(ctx context.Context, taskID, sessionID, agentExecutionID string, err error) (handled bool) +// LaunchFailedFunc is called when session launch fails before the agent starts. +// Useful for creating user-facing status messages tied to launch errors. +type LaunchFailedFunc func(ctx context.Context, taskID, sessionID string, err error) + // ExecutorTypeCapabilities provides behavioral queries about executor types. // Implemented by the lifecycle manager using its backend registry. type ExecutorTypeCapabilities interface { @@ -354,6 +358,10 @@ type Executor struct { // to detect auth errors and treat them as recoverable. onAgentStartFailed AgentStartFailedFunc + // Callback for session launch failures (pre-start). Allows orchestrator + // to emit user-friendly guidance for known failure patterns. + onLaunchFailed LaunchFailedFunc + // Per-session locks to prevent concurrent resume/launch operations on the same session. // This prevents race conditions when the backend restarts and multiple resume requests // arrive simultaneously (e.g., from frontend auto-resume). @@ -427,6 +435,12 @@ func (e *Executor) SetOnAgentStartFailed(fn AgentStartFailedFunc) { e.onAgentStartFailed = fn } +// SetOnLaunchFailed sets a callback for launch failures that happen before +// the agent process has started. +func (e *Executor) SetOnLaunchFailed(fn LaunchFailedFunc) { + e.onLaunchFailed = fn +} + // SetCapabilities sets the executor type capabilities provider. func (e *Executor) SetCapabilities(c ExecutorTypeCapabilities) { e.capabilities = c diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index f2ed4441e8..cc6b6d9a1f 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -375,15 +375,23 @@ func (e *Executor) LaunchPreparedSession(ctx context.Context, task *v1.Task, ses // handleLaunchFailure marks the session and task as FAILED and returns the original error. func (e *Executor) handleLaunchFailure(ctx context.Context, taskID, sessionID string, launchErr error) error { + // Detach from caller context so failure bookkeeping completes even if the + // original request context was cancelled. + failCtx := context.WithoutCancel(ctx) e.logger.Error("failed to launch agent", zap.String("task_id", taskID), zap.Error(launchErr)) - if updateErr := e.updateSessionState(ctx, taskID, sessionID, models.TaskSessionStateFailed, launchErr.Error()); updateErr != nil { + // Call onLaunchFailed before state updates so it can set the suppressToast + // flag that updateSessionState will propagate to the frontend. + if e.onLaunchFailed != nil { + e.onLaunchFailed(failCtx, taskID, sessionID, launchErr) + } + if updateErr := e.updateSessionState(failCtx, taskID, sessionID, models.TaskSessionStateFailed, launchErr.Error()); updateErr != nil { e.logger.Warn("failed to mark session as failed after launch error", zap.String("session_id", sessionID), zap.Error(updateErr)) } - if updateErr := e.updateTaskState(ctx, taskID, v1.TaskStateFailed); updateErr != nil { + if updateErr := e.updateTaskState(failCtx, taskID, v1.TaskStateFailed); updateErr != nil { e.logger.Warn("failed to mark task as failed after launch error", zap.String("task_id", taskID), zap.Error(updateErr)) diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 392214448c..4deff1e2d9 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -13,6 +13,8 @@ package orchestrator import ( "context" "errors" + "regexp" + "strings" "sync" "time" @@ -200,6 +202,11 @@ type Service struct { // Used to suppress stale ready events and avoid draining queued prompts mid-reset. resetInProgressSessions sync.Map + // suppressToast: sessionID -> true. Set by failure handlers that create + // guidance messages with actions. Cleared by updateTaskSessionState which + // adds suppress_toast to the WS event so the frontend skips the error toast. + suppressToast sync.Map + // clarificationWatchdogs tracks active clarification primary-path resume watchdogs. // key: "::", value: *clarificationWatchdogEntry clarificationWatchdogs sync.Map @@ -279,6 +286,7 @@ func NewService( s.updateTaskSessionState(ctx, taskID, sessionID, state, errorMessage, true) return nil }) + exec.SetOnLaunchFailed(s.handleSessionLaunchFailed) exec.SetOnAgentStartFailed(s.handleAgentStartFailed) if caps, ok := agentManager.(executor.ExecutorTypeCapabilities); ok { exec.SetCapabilities(caps) @@ -730,6 +738,91 @@ func canResumeRunning(running *models.ExecutorRunning) bool { return models.IsAlwaysResumableRuntime(running.Runtime) } +func isMissingMergedPRBranchError(err error) bool { + if err == nil { + return false + } + msg := strings.ToLower(err.Error()) + return strings.Contains(msg, "couldn't find remote ref") || + (strings.Contains(msg, "not found locally or on remote") && strings.Contains(msg, "branch")) || + (strings.Contains(msg, "pathspec") && strings.Contains(msg, "did not match")) +} + +var ( + quotedBranchPattern = regexp.MustCompile(`branch "([^"]+)"`) + remoteRefPattern = regexp.MustCompile(`remote ref ([^\s]+)`) + pathspecBranchPattern = regexp.MustCompile(`pathspec '([^']+)'`) +) + +func extractMissingBranchName(err error) string { + if err == nil { + return "" + } + msg := err.Error() + if match := quotedBranchPattern.FindStringSubmatch(msg); len(match) == 2 { + return strings.TrimSpace(match[1]) + } + if match := remoteRefPattern.FindStringSubmatch(msg); len(match) == 2 { + return strings.TrimSpace(match[1]) + } + if match := pathspecBranchPattern.FindStringSubmatch(msg); len(match) == 2 { + return strings.TrimSpace(match[1]) + } + return "" +} + +func (s *Service) handleSessionLaunchFailed(ctx context.Context, taskID, sessionID string, launchErr error) { + if s.messageCreator == nil || !isMissingMergedPRBranchError(launchErr) { + return + } + + branch := extractMissingBranchName(launchErr) + content := "This task references a PR branch that no longer exists on remote (likely merged and deleted)." + if branch != "" { + content = "The remote PR branch \"" + branch + "\" no longer exists (likely merged and deleted)." + } + metadata := map[string]interface{}{ + "variant": "warning", + "failure_kind": "missing_pr_branch", + "missing_branch": branch, + "actions": []map[string]interface{}{ + { + "type": "archive_task", + "label": "Archive task", + "tooltip": "Keep task history and hide it from active work", + "icon": "archive", + "test_id": "missing-branch-archive-button", + }, + { + "type": "delete_task", + "label": "Delete task", + "tooltip": "Permanently remove this task", + "variant": "destructive", + "icon": "trash", + "test_id": "missing-branch-delete-button", + }, + }, + } + s.suppressToast.Store(sessionID, true) + msgCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + if err := s.messageCreator.CreateSessionMessage( + msgCtx, + taskID, + content, + sessionID, + string(v1.MessageTypeStatus), + "", // No turn — agent never started, avoid lazily creating a synthetic turn. + metadata, + false, + ); err != nil { + s.logger.Warn("failed to create missing PR branch launch failure message", + zap.String("task_id", taskID), + zap.String("session_id", sessionID), + zap.Error(err)) + } +} + // IsRunning returns true if the service is running func (s *Service) IsRunning() bool { s.mu.RLock() diff --git a/apps/backend/internal/orchestrator/task_launch_failure_test.go b/apps/backend/internal/orchestrator/task_launch_failure_test.go new file mode 100644 index 0000000000..ae75adb2e8 --- /dev/null +++ b/apps/backend/internal/orchestrator/task_launch_failure_test.go @@ -0,0 +1,152 @@ +package orchestrator + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +func TestIsMissingMergedPRBranchError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "remote ref missing", + err: errors.New("environment preparation failed: fatal: couldn't find remote ref feature/foo"), + want: true, + }, + { + name: "branch not found locally or remote", + err: errors.New("environment preparation failed: branch \"feature/foo\" not found locally or on remote"), + want: true, + }, + { + name: "pathspec did not match (local executor)", + err: errors.New("environment preparation failed: checkout branch: git command failed: error: pathspec 'feature/foo' did not match any file(s) known to git"), + want: true, + }, + { + name: "unrelated launch error", + err: errors.New("failed to launch container"), + want: false, + }, + { + name: "nil error", + err: nil, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isMissingMergedPRBranchError(tt.err); got != tt.want { + t.Fatalf("isMissingMergedPRBranchError()=%v, want %v", got, tt.want) + } + }) + } +} + +func TestExtractMissingBranchName(t *testing.T) { + tests := []struct { + name string + err error + want string + }{ + { + name: "quoted branch form", + err: errors.New("environment preparation failed: branch \"feature/foo\" not found locally or on remote"), + want: "feature/foo", + }, + { + name: "remote ref form", + err: errors.New("fatal: couldn't find remote ref hotfix/bar"), + want: "hotfix/bar", + }, + { + name: "pathspec form (local executor)", + err: errors.New("checkout branch: git command failed: error: pathspec 'feature/baz' did not match any file(s) known to git"), + want: "feature/baz", + }, + { + name: "no branch available", + err: errors.New("failed to launch container"), + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := extractMissingBranchName(tt.err); got != tt.want { + t.Fatalf("extractMissingBranchName()=%q, want %q", got, tt.want) + } + }) + } +} + +func TestHandleSessionLaunchFailed_CreatesGuidanceMessageForMissingPRBranch(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting) + + mc := &mockMessageCreator{} + svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo()) + svc.messageCreator = mc + + err := errors.New("environment preparation failed: branch \"feature/foo\" not found locally or on remote: fatal: couldn't find remote ref feature/foo") + svc.handleSessionLaunchFailed(ctx, "task1", "session1", err) + + if len(mc.sessionMessages) != 1 { + t.Fatalf("expected 1 session message, got %d", len(mc.sessionMessages)) + } + msg := mc.sessionMessages[0] + if msg.taskID != "task1" || msg.sessionID != "session1" { + t.Fatalf("unexpected message target: task=%s session=%s", msg.taskID, msg.sessionID) + } + if msg.messageType != string(v1.MessageTypeStatus) { + t.Fatalf("expected status message type, got %q", msg.messageType) + } + if msg.turnID != "" { + t.Fatalf("expected empty turn ID for pre-start failure, got %q", msg.turnID) + } + if !strings.Contains(msg.content, "feature/foo") || !strings.Contains(strings.ToLower(msg.content), "merged") { + t.Fatalf("expected content with branch name and merged hint, got %q", msg.content) + } + if kind, ok := msg.metadata["failure_kind"].(string); !ok || kind != "missing_pr_branch" { + t.Fatalf("expected failure_kind metadata, got %#v", msg.metadata["failure_kind"]) + } + if branch, ok := msg.metadata["missing_branch"].(string); !ok || branch != "feature/foo" { + t.Fatalf("expected missing_branch metadata, got %#v", msg.metadata["missing_branch"]) + } + // Verify actions array is present with archive and delete actions + actions, ok := msg.metadata["actions"].([]map[string]interface{}) + if !ok || len(actions) != 2 { + t.Fatalf("expected 2 actions, got %#v", msg.metadata["actions"]) + } + if actions[0]["type"] != "archive_task" { + t.Fatalf("expected first action type archive_task, got %v", actions[0]["type"]) + } + if actions[1]["type"] != "delete_task" { + t.Fatalf("expected second action type delete_task, got %v", actions[1]["type"]) + } +} + +func TestHandleSessionLaunchFailed_IgnoresUnrelatedErrors(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateStarting) + + mc := &mockMessageCreator{} + svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo()) + svc.messageCreator = mc + + svc.handleSessionLaunchFailed(ctx, "task1", "session1", errors.New("failed to launch container")) + + if len(mc.sessionMessages) != 0 { + t.Fatalf("expected no session messages for unrelated error, got %d", len(mc.sessionMessages)) + } +} diff --git a/apps/backend/internal/orchestrator/task_operations_test.go b/apps/backend/internal/orchestrator/task_operations_test.go index c5e8b02d6c..b8784849d0 100644 --- a/apps/backend/internal/orchestrator/task_operations_test.go +++ b/apps/backend/internal/orchestrator/task_operations_test.go @@ -166,7 +166,8 @@ func TestStartCreatedSession_NotInCreatedState(t *testing.T) { // mockMessageCreator implements MessageCreator for testing. // Only CreateUserMessage is tracked; all other methods are no-op stubs. type mockMessageCreator struct { - userMessages []mockUserMessage + userMessages []mockUserMessage + sessionMessages []mockSessionMessage } type mockUserMessage struct { @@ -174,6 +175,12 @@ type mockUserMessage struct { metadata map[string]interface{} } +type mockSessionMessage struct { + taskID, content, sessionID, messageType, turnID string + metadata map[string]interface{} + requestsInput bool +} + func (m *mockMessageCreator) CreateUserMessage(_ context.Context, taskID, content, sessionID, turnID string, metadata map[string]interface{}) error { m.userMessages = append(m.userMessages, mockUserMessage{taskID, content, sessionID, turnID, metadata}) return nil @@ -191,7 +198,16 @@ func (m *mockMessageCreator) UpdateToolCallMessage(context.Context, string, stri return nil } -func (m *mockMessageCreator) CreateSessionMessage(context.Context, string, string, string, string, string, map[string]interface{}, bool) error { +func (m *mockMessageCreator) CreateSessionMessage(_ context.Context, taskID, content, sessionID, messageType, turnID string, metadata map[string]interface{}, requestsInput bool) error { + m.sessionMessages = append(m.sessionMessages, mockSessionMessage{ + taskID: taskID, + content: content, + sessionID: sessionID, + messageType: messageType, + turnID: turnID, + metadata: metadata, + requestsInput: requestsInput, + }) return nil } diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index 2cd77cabbb..814a8e452e 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -344,24 +344,25 @@ func (m *Manager) fetchBranchToLocal(ctx context.Context, repoPath, branch strin fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", "origin", branch+":"+branch) if output, err := fetchCmd.CombinedOutput(); err != nil { + outputStr := string(output) m.logger.Warn("fetch from origin failed, checking local branch", zap.String("branch", branch), - zap.String("output", string(output)), + zap.String("output", outputStr), zap.Error(err)) // Fall back to local branch if it exists. if !m.branchExists(repoPath, branch) { - return nil, fmt.Errorf("branch %q not found locally or on remote: %s", branch, string(output)) + return nil, fmt.Errorf("branch %q not found locally or on remote: %s", branch, outputStr) } - reason := classifyGitFallbackReason(err, string(output), fetchCtx.Err()) + reason := classifyGitFallbackReason(err, outputStr, fetchCtx.Err()) warning := fmt.Sprintf("Could not fetch latest from origin (%s). Using local version of branch %q which may be outdated.", reason, branch) m.logger.Info("using local branch (fetch failed)", zap.String("branch", branch), zap.String("warning", warning)) return &FetchBranchResult{ Warning: warning, - WarningDetail: strings.TrimSpace(string(output)), + WarningDetail: strings.TrimSpace(outputStr), }, nil } diff --git a/apps/backend/internal/worktree/manager_test.go b/apps/backend/internal/worktree/manager_test.go index 6c083113d9..1c1c7fde07 100644 --- a/apps/backend/internal/worktree/manager_test.go +++ b/apps/backend/internal/worktree/manager_test.go @@ -895,6 +895,48 @@ esac } } +func TestFetchBranchToLocal_MissingRemoteRefReturnsError(t *testing.T) { + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + fetch) + echo "fatal: couldn't find remote ref feature/pr-branch" >&2 + exit 128 + ;; + rev-parse) + # Simulate branch does NOT exist locally + if [ "${2:-}" = "--verify" ]; then + exit 1 + fi + exit 0 + ;; + *) + exit 0 + ;; +esac +`) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + repoPath := t.TempDir() + _, err = mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch") + if err == nil { + t.Fatal("fetchBranchToLocal() should fail when remote ref is missing and no local branch") + } + if !strings.Contains(err.Error(), "not found locally or on remote") { + t.Fatalf("unexpected error message: %v", err) + } + if !strings.Contains(err.Error(), "couldn't find remote ref") { + t.Fatalf("expected error to contain git output, got: %v", err) + } +} + func TestClassifyGitFallbackReason_AuthPrompt(t *testing.T) { reason := classifyGitFallbackReason(nil, "fatal: could not read Username for 'https://github.com'", nil) if reason != "non_interactive_auth_failed" { diff --git a/apps/web/components/task/chat/message-list-native.tsx b/apps/web/components/task/chat/message-list-native.tsx index a9e9b7b979..54d001c178 100644 --- a/apps/web/components/task/chat/message-list-native.tsx +++ b/apps/web/components/task/chat/message-list-native.tsx @@ -6,6 +6,7 @@ import { useDockviewStore } from "@/lib/state/dockview-store"; import type { Message } from "@/lib/types/http"; import { AgentStatus } from "@/components/task/chat/messages/agent-status"; import { PrepareProgress } from "@/components/session/prepare-progress"; +import { MessageRenderer } from "@/components/task/chat/message-renderer"; import { useLazyLoadMessages } from "@/hooks/use-lazy-load-messages"; import { type MessageListProps, @@ -173,6 +174,7 @@ function useScrollToMessage() { export const NativeMessageList = memo(function NativeMessageList({ items, messages, + footerActionMessages, permissionsByToolCallId, childrenByParentToolCallId, taskId, @@ -254,6 +256,14 @@ export const NativeMessageList = memo(function NativeMessageList({ {sessionId && } + {(footerActionMessages ?? []).map((msg: Message) => ( + + ))} {/* Bottom anchor — browser keeps scroll pinned here when new content appends */}
diff --git a/apps/web/components/task/chat/message-list-shared.tsx b/apps/web/components/task/chat/message-list-shared.tsx index b145cda08e..fb590c8de3 100644 --- a/apps/web/components/task/chat/message-list-shared.tsx +++ b/apps/web/components/task/chat/message-list-shared.tsx @@ -9,6 +9,8 @@ import { TurnGroupMessage } from "@/components/task/chat/messages/turn-group-mes export type MessageListProps = { items: RenderItem[]; messages: Message[]; + /** Action messages rendered after the env prep error status in the footer. */ + footerActionMessages?: Message[]; permissionsByToolCallId: Map; childrenByParentToolCallId: Map; taskId?: string; diff --git a/apps/web/components/task/chat/message-list-virtuoso.tsx b/apps/web/components/task/chat/message-list-virtuoso.tsx index a223f0cdb7..402657035e 100644 --- a/apps/web/components/task/chat/message-list-virtuoso.tsx +++ b/apps/web/components/task/chat/message-list-virtuoso.tsx @@ -7,6 +7,7 @@ import { SessionPanelContent } from "@kandev/ui/pannel-session"; import type { RenderItem } from "@/hooks/use-processed-messages"; import { AgentStatus } from "@/components/task/chat/messages/agent-status"; import { PrepareProgress } from "@/components/session/prepare-progress"; +import { MessageRenderer } from "@/components/task/chat/message-renderer"; import { useLazyLoadMessages } from "@/hooks/use-lazy-load-messages"; import { type MessageListProps, @@ -218,7 +219,15 @@ function useVisibleScrollParent() { } export const VirtuosoMessageList = memo(function VirtuosoMessageList(props: MessageListProps) { - const { items, messages, sessionId, messagesLoading, isWorking, sessionState } = props; + const { + items, + messages, + footerActionMessages, + sessionId, + messagesLoading, + isWorking, + sessionState, + } = props; const { scrollParent, setScrollRef } = useVisibleScrollParent(); const isInitialLoading = messagesLoading && messages.length === 0; const isNonLoadableSession = @@ -243,14 +252,24 @@ export const VirtuosoMessageList = memo(function VirtuosoMessageList(props: Mess [isLoadingMore, hasMore, showLoadingState, messagesLoading, isInitialLoading, messages.length], ); + const footerActions = useMemo(() => footerActionMessages ?? [], [footerActionMessages]); + const Footer = useCallback( () => ( <> {sessionId && } + {footerActions.map((msg) => ( + + ))} ), - [sessionId, sessionState, messages], + [sessionId, sessionState, messages, footerActions], ); if (isInitialLoading || items.length === 0) { @@ -266,6 +285,14 @@ export const VirtuosoMessageList = memo(function VirtuosoMessageList(props: Mess /> {sessionId && } + {footerActions.map((msg) => ( + + ))} ); } diff --git a/apps/web/components/task/chat/message-renderer.tsx b/apps/web/components/task/chat/message-renderer.tsx index 42777ce391..8963c4b64a 100644 --- a/apps/web/components/task/chat/message-renderer.tsx +++ b/apps/web/components/task/chat/message-renderer.tsx @@ -21,8 +21,7 @@ import { ScriptExecutionMessage } from "@/components/task/chat/messages/script-e import { ClarificationRequestMessage } from "@/components/task/chat/messages/clarification-request-message"; import { ToolSubagentMessage } from "@/components/task/chat/messages/tool-subagent-message"; import { AgentPlanMessage } from "@/components/task/chat/messages/agent-plan-message"; -import { AgentErrorRecoveryMessage } from "@/components/task/chat/messages/agent-error-recovery-message"; -import { GitOperationErrorMessage } from "@/components/task/chat/messages/git-operation-error-message"; +import { ActionMessage } from "@/components/task/chat/messages/action-message"; type AdapterContext = { isTaskDescription: boolean; @@ -174,18 +173,11 @@ const adapters: MessageAdapter[] = [ }, }, { - matches: (comment) => - comment.type === "error" && - !!(comment.metadata as Record | undefined)?.git_operation_error, - render: (comment) => , - }, - { - matches: (comment) => - (comment.type === "status" || comment.type === "error") && - !!(comment.metadata as Record | undefined)?.recovery_actions, - render: (comment, ctx) => ( - - ), + matches: (comment) => { + const meta = comment.metadata as Record | undefined; + return Array.isArray(meta?.actions) && (meta.actions as unknown[]).length > 0; + }, + render: (comment, ctx) => , }, { matches: (comment) => diff --git a/apps/web/components/task/chat/messages/action-message.tsx b/apps/web/components/task/chat/messages/action-message.tsx new file mode 100644 index 0000000000..a291fdc491 --- /dev/null +++ b/apps/web/components/task/chat/messages/action-message.tsx @@ -0,0 +1,207 @@ +"use client"; + +import { useState, useCallback, memo, type ReactElement } from "react"; +import { + IconAlertTriangle, + IconArchive, + IconTrash, + IconRefresh, + IconPlayerPlay, + IconSparkles, + IconGitCommit, +} from "@tabler/icons-react"; +import { cn } from "@/lib/utils"; +import { Button } from "@kandev/ui/button"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { getWebSocketClient } from "@/lib/ws/connection"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; +import { useArchiveAndSwitchTask } from "@/hooks/use-task-actions"; +import { useTaskRemoval } from "@/hooks/use-task-removal"; +import { deleteTask } from "@/lib/api/domains/kanban-api"; +import { AuthMethodsPanel, GenericAuthPanel } from "./auth-methods-panel"; +import type { Message, TaskSessionState } from "@/lib/types/http"; +import type { MessageAction, RecoveryAuthMethod } from "@/components/task/chat/types"; + +const ICON_MAP: Record = { + archive: IconArchive, + trash: IconTrash, + refresh: IconRefresh, + "player-play": IconPlayerPlay, + sparkles: IconSparkles, + "git-commit": IconGitCommit, + "alert-triangle": IconAlertTriangle, +}; + +type ActionMeta = { + actions?: MessageAction[]; + variant?: string; + is_auth_error?: boolean; + auth_methods?: RecoveryAuthMethod[]; + error_output?: string; +}; + +function isSessionActive(state?: TaskSessionState) { + return state === "RUNNING" || state === "STARTING" || state === "COMPLETED"; +} + +export const ActionMessage = memo(function ActionMessage({ + comment, + sessionState, +}: { + comment: Message; + sessionState?: TaskSessionState; +}) { + const metadata = comment.metadata as ActionMeta | undefined; + const isWarning = metadata?.variant === "warning"; + const message = comment.content || "An error occurred"; + + // Hide once session is active again (recovery succeeded) + if (isSessionActive(sessionState)) return null; + + const iconClass = isWarning ? "text-amber-500" : "text-red-500"; + const textClass = isWarning + ? "text-amber-600 dark:text-amber-400" + : "text-red-600 dark:text-red-400"; + + return ( +
+
+
+ +
+
+
{message}
+ + {metadata?.actions && metadata.actions.length > 0 && ( + + )} +
+
+
+ ); +}); + +function ActionMessageDetails({ metadata }: { metadata: ActionMeta | undefined }) { + const store = useAppStoreApi(); + const openTerminalWithCommand = useCallback( + (command: string) => store.getState().openBottomTerminalWithCommand(command), + [store], + ); + const openBottomTerminal = useCallback(() => { + if (!store.getState().bottomTerminal.isOpen) store.getState().toggleBottomTerminal(); + }, [store]); + + if (!metadata) return null; + return ( + <> + {metadata.error_output && ( + 
+          {metadata.error_output}
+
+ )} + {metadata.is_auth_error && metadata.auth_methods && metadata.auth_methods.length > 0 && ( + + )} + {metadata.is_auth_error && (!metadata.auth_methods || metadata.auth_methods.length === 0) && ( + + )} + + ); +} + +function ActionButtons({ actions, taskId }: { actions: MessageAction[]; taskId?: string }) { + return ( +
+ {actions.map((action, i) => ( + + ))} +
+ ); +} + +function ActionButton({ + action, + messageTaskId, +}: { + action: MessageAction; + messageTaskId?: string; +}): ReactElement { + const [state, setState] = useState<"idle" | "busy" | "done" | "error">("idle"); + const activeTaskId = useAppStore((s) => s.tasks.activeTaskId); + const taskId = messageTaskId || activeTaskId; + const store = useAppStoreApi(); + const archiveAndSwitch = useArchiveAndSwitchTask(); + const { removeTaskFromBoard } = useTaskRemoval({ store }); + + const execute = useCallback(async () => { + if (state === "busy") return; + setState("busy"); + try { + switch (action.type) { + case "archive_task": { + if (taskId) await archiveAndSwitch(taskId); + break; + } + case "delete_task": { + if (taskId) { + const { activeTaskId, activeSessionId } = store.getState().tasks; + await deleteTask(taskId); + await removeTaskFromBoard(taskId, { + wasActiveTaskId: activeTaskId, + wasActiveSessionId: activeSessionId, + }); + } + break; + } + case "ws_request": { + const client = getWebSocketClient(); + const params = action.params as + | { method: string; payload: Record } + | undefined; + if (client && params) await client.request(params.method, params.payload); + break; + } + } + setState("done"); + } catch { + setState("error"); + setTimeout(() => setState("idle"), 3000); + } + }, [action, state, taskId, store, archiveAndSwitch, removeTaskFromBoard]); + + const Icon = action.icon ? ICON_MAP[action.icon] : null; + const disabled = state === "busy" || state === "done"; + const isDestructive = action.variant === "destructive"; + const label = + state === "done" && action.type === "ws_request" ? `${action.label} requested` : action.label; + + const button = ( + + ); + + if (action.tooltip) { + return ( + + {button} + {action.tooltip} + + ); + } + return button; +} diff --git a/apps/web/components/task/chat/messages/agent-error-recovery-message.tsx b/apps/web/components/task/chat/messages/agent-error-recovery-message.tsx deleted file mode 100644 index cf123a744c..0000000000 --- a/apps/web/components/task/chat/messages/agent-error-recovery-message.tsx +++ /dev/null @@ -1,357 +0,0 @@ -"use client"; - -import { useState, useCallback, useEffect, useRef, memo } from "react"; -import { - IconAlertTriangle, - IconRefresh, - IconPlayerPlay, - IconTerminal2, - IconCopy, - IconCheck, -} from "@tabler/icons-react"; -import { cn } from "@/lib/utils"; -import { Button } from "@kandev/ui/button"; -import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; -import { getWebSocketClient } from "@/lib/ws/connection"; -import { useAppStoreApi } from "@/components/state-provider"; -import type { Message, TaskSessionState } from "@/lib/types/http"; -import type { RecoveryAuthMethod, RecoveryMetadata } from "@/components/task/chat/types"; - -type RecoveryState = "pending" | "recovering" | "recovered" | "error"; - -export const AgentErrorRecoveryMessage = memo(function AgentErrorRecoveryMessage({ - comment, - sessionState, -}: { - comment: Message; - sessionState?: TaskSessionState; -}) { - const [state, setState] = useState("pending"); - const resetTimerRef = useRef | null>(null); - const metadata = comment.metadata as RecoveryMetadata | undefined; - const canRecover = Boolean(metadata?.task_id && metadata?.session_id); - const storeApi = useAppStoreApi(); - - useEffect(() => { - return () => { - if (resetTimerRef.current) clearTimeout(resetTimerRef.current); - }; - }, []); - - const openBottomTerminal = useCallback(() => { - const store = storeApi.getState(); - if (!store.bottomTerminal.isOpen) { - store.toggleBottomTerminal(); - } - }, [storeApi]); - - const openTerminalWithCommand = useCallback( - (command: string) => { - storeApi.getState().openBottomTerminalWithCommand(command); - }, - [storeApi], - ); - - const handleRecover = useCallback( - async (action: "resume" | "fresh_start") => { - if (!canRecover || !metadata) return; - if (resetTimerRef.current) { - clearTimeout(resetTimerRef.current); - resetTimerRef.current = null; - } - const client = getWebSocketClient(); - if (!client) return; - setState("recovering"); - try { - await client.request("session.recover", { - task_id: metadata.task_id, - session_id: metadata.session_id, - action, - }); - setState("recovered"); - } catch (error) { - console.error("Failed to recover session:", error); - setState("error"); - resetTimerRef.current = setTimeout(() => { - setState("pending"); - resetTimerRef.current = null; - }, 3000); - } - }, - [canRecover, metadata], - ); - - // Hide once recovery succeeded or session is active again (handles page refresh) - const isSessionActive = - sessionState === "RUNNING" || sessionState === "STARTING" || sessionState === "COMPLETED"; - if (state === "recovered" || isSessionActive) { - return null; - } - - const isAuthError = metadata?.is_auth_error === true; - const authMethods = metadata?.auth_methods; - const message = comment.content || "Agent encountered an error"; - - return ( -
-
-
- -
-
-
{message}
- -
-
-
- ); -}); - -function ErrorActions({ - isAuthError, - authMethods, - openTerminalWithCommand, - openBottomTerminal, - state, - canRecover, - hasResumeToken, - onRecover, -}: { - isAuthError: boolean; - authMethods: RecoveryAuthMethod[] | undefined; - openTerminalWithCommand: (command: string) => void; - openBottomTerminal: () => void; - state: RecoveryState; - canRecover: boolean; - hasResumeToken?: boolean; - onRecover: (action: "resume" | "fresh_start") => void; -}) { - if (isAuthError) { - const disabled = state === "recovering" || !canRecover; - return ( - <> - {authMethods && authMethods.length > 0 ? ( - - ) : ( - - )} -
- - - - - Restart the agent session after logging in - - {state === "error" && Failed — try again} -
- - ); - } - return ( - - ); -} - -function AuthMethodsPanel({ - methods, - onOpenTerminal, -}: { - methods: RecoveryAuthMethod[]; - onOpenTerminal: (command: string) => void; -}) { - return ( -
-
- Authentication required, log in before resuming -
- {methods.map((method) => ( - - ))} -
- ); -} - -function RecoveryActions({ - state, - canRecover, - hasResumeToken, - onRecover, -}: { - state: RecoveryState; - canRecover: boolean; - hasResumeToken?: boolean; - onRecover: (action: "resume" | "fresh_start") => void; -}) { - const disabled = state === "recovering" || !canRecover; - - return ( -
- {hasResumeToken && ( - - - - - - Re-launch with resume flag — keeps all previous messages and context - - - )} - - - - - - New agent process on the same workspace — no previous conversation context - - - {state === "error" && Failed — try again} -
- ); -} - -function GenericAuthPanel({ onOpenTerminal }: { onOpenTerminal: () => void }) { - return ( -
-
- Authentication required — please log in via the terminal -
- -
- ); -} - -function buildFullCommand(termAuth: RecoveryAuthMethod["terminal_auth"]): string | null { - if (!termAuth) return null; - if (termAuth.args && termAuth.args.length > 0) { - return `${termAuth.command} ${termAuth.args.join(" ")}`; - } - return termAuth.command; -} - -function AuthMethodRow({ - method, - onOpenTerminal, -}: { - method: RecoveryAuthMethod; - onOpenTerminal: (command: string) => void; -}) { - const [copied, setCopied] = useState(false); - const termAuth = method.terminal_auth; - - const fullCommand = buildFullCommand(termAuth); - - const handleCopy = useCallback(async () => { - if (!fullCommand) return; - try { - await navigator.clipboard.writeText(fullCommand); - setCopied(true); - setTimeout(() => setCopied(false), 2000); - } catch { - // clipboard API unavailable - } - }, [fullCommand]); - - return ( -
-
{method.name}
- {termAuth && fullCommand ? ( -
-
- - {fullCommand} -
- - - - - Copy command - - - - - - - Open the bottom terminal and paste this command - - -
- ) : ( -
- {method.description || "Login required"} -
- )} -
- ); -} diff --git a/apps/web/components/task/chat/messages/auth-methods-panel.tsx b/apps/web/components/task/chat/messages/auth-methods-panel.tsx new file mode 100644 index 0000000000..970cc3964b --- /dev/null +++ b/apps/web/components/task/chat/messages/auth-methods-panel.tsx @@ -0,0 +1,128 @@ +"use client"; + +import { useState, useCallback } from "react"; +import { IconTerminal2, IconCopy, IconCheck } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import type { RecoveryAuthMethod } from "@/components/task/chat/types"; + +function buildFullCommand(termAuth: RecoveryAuthMethod["terminal_auth"]): string | null { + if (!termAuth) return null; + if (termAuth.args && termAuth.args.length > 0) { + return `${termAuth.command} ${termAuth.args.join(" ")}`; + } + return termAuth.command; +} + +export function AuthMethodsPanel({ + methods, + onOpenTerminal, +}: { + methods: RecoveryAuthMethod[]; + onOpenTerminal: (command: string) => void; +}) { + return ( +
+
+ Authentication required, log in before resuming +
+ {methods.map((method) => ( + + ))} +
+ ); +} + +export function GenericAuthPanel({ onOpenTerminal }: { onOpenTerminal: () => void }) { + return ( +
+
+ Authentication required — please log in via the terminal +
+ +
+ ); +} + +function AuthMethodRow({ + method, + onOpenTerminal, +}: { + method: RecoveryAuthMethod; + onOpenTerminal: (command: string) => void; +}) { + const [copied, setCopied] = useState(false); + const termAuth = method.terminal_auth; + const fullCommand = buildFullCommand(termAuth); + + const handleCopy = useCallback(async () => { + if (!fullCommand) return; + try { + await navigator.clipboard.writeText(fullCommand); + setCopied(true); + setTimeout(() => setCopied(false), 2000); + } catch { + // clipboard API unavailable + } + }, [fullCommand]); + + return ( +
+
{method.name}
+ {termAuth && fullCommand ? ( +
+
+ + {fullCommand} +
+ + + + + Copy command + + + + + + + Open the bottom terminal and paste this command + + +
+ ) : ( +
+ {method.description || "Login required"} +
+ )} +
+ ); +} diff --git a/apps/web/components/task/chat/messages/git-operation-error-message.tsx b/apps/web/components/task/chat/messages/git-operation-error-message.tsx deleted file mode 100644 index 7e2f5331f9..0000000000 --- a/apps/web/components/task/chat/messages/git-operation-error-message.tsx +++ /dev/null @@ -1,80 +0,0 @@ -"use client"; - -import { useState, useCallback, memo } from "react"; -import { IconGitCommit, IconSparkles } from "@tabler/icons-react"; -import { Button } from "@kandev/ui/button"; -import { getWebSocketClient } from "@/lib/ws/connection"; -import type { Message } from "@/lib/types/http"; -import type { GitOperationErrorMetadata } from "@/components/task/chat/types"; - -export const GitOperationErrorMessage = memo(function GitOperationErrorMessage({ - comment, -}: { - comment: Message; -}) { - const [fixSent, setFixSent] = useState(false); - const metadata = comment.metadata as GitOperationErrorMetadata | undefined; - - const handleFix = useCallback(async () => { - const client = getWebSocketClient(); - if (!client || !metadata) return; - - setFixSent(true); - const prompt = [ - `The git ${metadata.operation} command failed with the following error:`, - "", - "```", - metadata.error_output, - "```", - "", - "Please fix the issues reported above.", - ].join("\n"); - - try { - await client.request("message.add", { - task_id: metadata.task_id, - session_id: metadata.session_id, - content: prompt, - }); - } catch { - setFixSent(false); - } - }, [metadata]); - - const operation = metadata?.operation || "operation"; - const title = comment.content || `Git ${operation} failed`; - - return ( -
-
-
- -
- -
-
{title}
- - {metadata?.error_output && ( - 
-              {metadata.error_output}
-
- )} - -
- -
-
-
-
- ); -}); diff --git a/apps/web/components/task/chat/types.ts b/apps/web/components/task/chat/types.ts index cde63fcb08..fd4e091f21 100644 --- a/apps/web/components/task/chat/types.ts +++ b/apps/web/components/task/chat/types.ts @@ -123,12 +123,14 @@ export type RecoveryMetadata = StatusMetadata & { auth_methods?: RecoveryAuthMethod[]; }; -export type GitOperationErrorMetadata = StatusMetadata & { - git_operation_error: true; - operation: string; - error_output: string; - session_id: string; - task_id: string; +export type MessageAction = { + type: "archive_task" | "delete_task" | "ws_request"; + label: string; + tooltip?: string; + variant?: "default" | "destructive"; + icon?: string; + params?: Record; + test_id?: string; }; export type TodoMetadata = { text: string; done?: boolean } | string; diff --git a/apps/web/components/task/task-chat-panel.tsx b/apps/web/components/task/task-chat-panel.tsx index e222b692d9..183b3fbe59 100644 --- a/apps/web/components/task/task-chat-panel.tsx +++ b/apps/web/components/task/task-chat-panel.tsx @@ -84,6 +84,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({ messagesLoading, groupedItems, allMessages, + footerActionMessages, permissionsByToolCallId, childrenByParentToolCallId, agentMessageCount, @@ -111,6 +112,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({ , key: string, value: unknown) { + if (value !== undefined && value !== null) body[key] = value; +} + +type CreateTaskOpts = { + description?: string; + workflow_id?: string; + workflow_step_id?: string; + agent_profile_id?: string; + repository_ids?: string[]; + repositories?: Array<{ repository_id: string; base_branch?: string; checkout_branch?: string }>; + plan_mode?: boolean; + metadata?: Record; +}; + +function buildTaskMetadata(opts: CreateTaskOpts): Record | undefined { + const meta: Record = { ...(opts.metadata ?? {}) }; + if (opts.agent_profile_id && meta.agent_profile_id == null) { + meta.agent_profile_id = opts.agent_profile_id; + } + return Object.keys(meta).length > 0 ? meta : undefined; +} + +function buildCreateTaskBody( + workspaceId: string, + title: string, + opts?: CreateTaskOpts, +): Record { + const body: Record = { + workspace_id: workspaceId, + title, + description: opts?.description ?? "", + }; + setIf(body, "workflow_id", opts?.workflow_id); + setIf(body, "workflow_step_id", opts?.workflow_step_id); + setIf(body, "metadata", opts ? buildTaskMetadata(opts) : undefined); + setIf( + body, + "repositories", + opts?.repositories ?? opts?.repository_ids?.map((id) => ({ repository_id: id })), + ); + if (opts?.plan_mode) body.plan_mode = true; + return body; +} + /** * HTTP API client for seeding test data via the backend REST API. */ @@ -112,22 +157,19 @@ export class ApiClient { agent_profile_id?: string; /** Repository IDs to associate with the task (required for agent execution). */ repository_ids?: string[]; + /** Full repository entries with optional checkout_branch / base_branch. */ + repositories?: Array<{ + repository_id: string; + base_branch?: string; + checkout_branch?: string; + }>; /** When true, task is placed at position 0 regardless of is_start_step. */ plan_mode?: boolean; + /** Extra metadata to store on the task. */ + metadata?: Record; }, ): Promise { - return this.request("POST", "/api/v1/tasks", { - workspace_id: workspaceId, - title, - description: opts?.description ?? "", - ...(opts?.workflow_id ? { workflow_id: opts.workflow_id } : {}), - ...(opts?.workflow_step_id ? { workflow_step_id: opts.workflow_step_id } : {}), - ...(opts?.agent_profile_id ? { metadata: { agent_profile_id: opts.agent_profile_id } } : {}), - ...(opts?.repository_ids - ? { repositories: opts.repository_ids.map((id) => ({ repository_id: id })) } - : {}), - ...(opts?.plan_mode ? { plan_mode: true } : {}), - }); + return this.request("POST", "/api/v1/tasks", buildCreateTaskBody(workspaceId, title, opts)); } async listAgents(): Promise<{ agents: Agent[]; total: number }> { @@ -496,6 +538,34 @@ export class ApiClient { await this.request("POST", "/api/v1/github/mock/task-prs", data); } + async mockGitHubGetStatus(): Promise<{ + authenticated: boolean; + username: string; + auth_method: string; + }> { + return this.request("GET", "/api/v1/github/status"); + } + + // --- Session --- + + async listSessionMessages( + sessionId: string, + ): Promise<{ messages: Array<{ id: string; content: string; author_type: string }> }> { + return this.request("GET", `/api/v1/task-sessions/${sessionId}/messages`); + } + + async listTasks( + workspaceId: string, + ): Promise<{ tasks: Array<{ id: string; title: string; workflow_step_id?: string }> }> { + return this.request("GET", `/api/v1/workspaces/${workspaceId}/tasks`); + } + + async listTaskSessions( + taskId: string, + ): Promise<{ sessions: Array<{ id: string; state: string }> }> { + return this.request("GET", `/api/v1/tasks/${taskId}/sessions`); + } + // --- GitHub Review Watch --- async createReviewWatch( @@ -522,29 +592,7 @@ export class ApiClient { }); } - async triggerReviewWatch(watchId: string): Promise<{ new_prs: number }> { - return this.request("POST", `/api/v1/github/watches/review/${watchId}/trigger`); - } - - async mockGitHubGetStatus(): Promise<{ - authenticated: boolean; - username: string; - auth_method: string; - }> { - return this.request("GET", "/api/v1/github/status"); - } - - // --- Session --- - - async listSessionMessages( - sessionId: string, - ): Promise<{ messages: Array<{ id: string; content: string; author_type: string }> }> { - return this.request("GET", `/api/v1/task-sessions/${sessionId}/messages`); - } - - async listTasks( - workspaceId: string, - ): Promise<{ tasks: Array<{ id: string; title: string; workflow_step_id?: string }> }> { - return this.request("GET", `/api/v1/workspaces/${workspaceId}/tasks`); + async triggerReviewWatch(watchId: string): Promise<{ new_prs: number; cleaned?: number }> { + return this.request("POST", `/api/v1/github/watches/review/${watchId}/trigger`, undefined); } } diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index 6d7c3fa01c..a2bd18bf32 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -383,7 +383,7 @@ export class SessionPage { /** Git operation error message in chat (shown when a git operation fails). */ gitOperationErrorMessage(): Locator { - return this.chat.getByTestId("git-operation-error-message"); + return this.chat.locator("div:has([data-testid='git-fix-button'])").first(); } /** Fix button on a git operation error message. */ diff --git a/apps/web/e2e/tests/pr-watcher-merged-cleanup.spec.ts b/apps/web/e2e/tests/pr-watcher-merged-cleanup.spec.ts new file mode 100644 index 0000000000..4cf283cafd --- /dev/null +++ b/apps/web/e2e/tests/pr-watcher-merged-cleanup.spec.ts @@ -0,0 +1,296 @@ +import { test, expect } from "../fixtures/test-base"; +import { KanbanPage } from "../pages/kanban-page"; + +test.describe("PR watcher merged cleanup", () => { + /** + * When a PR watcher creates a task for a PR, and the PR is subsequently + * merged (branch deleted), triggering the watch again should auto-delete + * the task if the user hasn't opened it yet (no sessions). + * + * Setup: + * - Create review watch on a workflow step without auto-start + * - Mock a PR (open), trigger watch → task created + * - Change PR to merged, trigger watch again → task auto-deleted + */ + test("auto-deletes unstarted task when PR is merged", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + + // --- Setup mock GitHub --- + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("test-user"); + await apiClient.mockGitHubAddPRs([ + { + number: 101, + title: "Feature to review", + state: "open", + head_branch: "feature/review-me", + base_branch: "main", + author_login: "contributor", + repo_owner: "testorg", + repo_name: "testrepo", + requested_reviewers: [{ login: "test-user", type: "User" }], + }, + ]); + + // Create a workflow step without auto-start for the review watch. + // The seed start step may have auto_start_agent, which would create a session + // and prevent cleanup (we only delete unstarted tasks). + const inboxStep = await apiClient.createWorkflowStep(seedData.workflowId, "PR Inbox", 0); + + // --- Create review watch on the inbox step (no auto-start) --- + const watch = await apiClient.createReviewWatch( + seedData.workspaceId, + seedData.workflowId, + inboxStep.id, + seedData.agentProfileId, + { repos: [{ owner: "testorg", name: "testrepo" }] }, + ); + + // --- Trigger watch → should create a task for PR #101 --- + const triggerResult = await apiClient.triggerReviewWatch(watch.id); + expect(triggerResult.new_prs).toBeGreaterThanOrEqual(1); + + // Task creation is async (goroutine), poll until it appears + let prTask: { id: string; title: string } | undefined; + await expect + .poll( + async () => { + const { tasks } = await apiClient.listTasks(seedData.workspaceId); + prTask = tasks.find((t) => t.title.includes("PR #101")); + return prTask; + }, + { timeout: 15_000 }, + ) + .toBeTruthy(); + + // Navigate to kanban and verify task is visible + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await expect(kanban.taskCardByTitle("PR #101: Feature to review")).toBeVisible({ + timeout: 15_000, + }); + + // --- Simulate PR merged: update mock PR state to closed --- + await apiClient.mockGitHubAddPRs([ + { + number: 101, + title: "Feature to review", + state: "closed", + head_branch: "feature/review-me", + base_branch: "main", + author_login: "contributor", + repo_owner: "testorg", + repo_name: "testrepo", + }, + ]); + + // --- Trigger watch again → should detect merged PR and delete the task --- + await apiClient.triggerReviewWatch(watch.id); + + // Verify task was deleted + await expect(kanban.taskCardByTitle("PR #101: Feature to review")).not.toBeVisible({ + timeout: 15_000, + }); + + // Confirm via API + const { tasks: tasksAfterCleanup } = await apiClient.listTasks(seedData.workspaceId); + const deletedTask = tasksAfterCleanup.find((t) => t.title.includes("PR #101")); + expect(deletedTask).toBeUndefined(); + }); + + /** + * When the user already opened a PR task and the agent completed, + * triggering the watch after the PR is merged should NOT delete the task + * — the user's work history is preserved, and the PR merged banner shows. + */ + test("preserves started task when PR is merged (shows banner instead)", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + // --- Setup mock GitHub --- + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("test-user"); + await apiClient.mockGitHubAddPRs([ + { + number: 202, + title: "Reviewed feature", + state: "open", + head_branch: "feature/reviewed", + base_branch: "main", + author_login: "contributor", + repo_owner: "testorg", + repo_name: "testrepo", + requested_reviewers: [{ login: "test-user", type: "User" }], + }, + ]); + + // Create workflow: Inbox → Working (auto-start + move to Done) → Done + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Review Workflow"); + const inboxStep = await apiClient.createWorkflowStep(workflow.id, "Inbox", 0); + const workingStep = await apiClient.createWorkflowStep(workflow.id, "Working", 1); + const doneStep = await apiClient.createWorkflowStep(workflow.id, "Done", 2); + await apiClient.updateWorkflowStep(workingStep.id, { + prompt: 'e2e:message("review complete")\n{{task_prompt}}', + events: { + on_enter: [{ type: "auto_start_agent" }], + on_turn_complete: [{ type: "move_to_step", config: { step_id: doneStep.id } }], + }, + }); + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: workflow.id, + }); + + // Create review watch on the inbox step + const watch = await apiClient.createReviewWatch( + seedData.workspaceId, + workflow.id, + inboxStep.id, + seedData.agentProfileId, + { repos: [{ owner: "testorg", name: "testrepo" }] }, + ); + + // Trigger → task created in Inbox + await apiClient.triggerReviewWatch(watch.id); + + let prTask: { id: string; title: string } | undefined; + await expect + .poll( + async () => { + const { tasks } = await apiClient.listTasks(seedData.workspaceId); + prTask = tasks.find((t) => t.title.includes("PR #202")); + return prTask; + }, + { timeout: 15_000 }, + ) + .toBeTruthy(); + + // Navigate to kanban + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await expect(kanban.taskCardInColumn("PR #202: Reviewed feature", inboxStep.id)).toBeVisible({ + timeout: 15_000, + }); + + // Move task to Working → auto-start → completes → Done (task now has sessions) + await apiClient.moveTask(prTask!.id, workflow.id, workingStep.id); + await expect(kanban.taskCardInColumn("PR #202: Reviewed feature", doneStep.id)).toBeVisible({ + timeout: 45_000, + }); + + // Simulate PR merged + await apiClient.mockGitHubAddPRs([ + { + number: 202, + title: "Reviewed feature", + state: "closed", + head_branch: "feature/reviewed", + base_branch: "main", + author_login: "contributor", + repo_owner: "testorg", + repo_name: "testrepo", + }, + ]); + + // Trigger watch → should NOT delete task (user already worked on it) + await apiClient.triggerReviewWatch(watch.id); + + // Task should still be visible + await expect(kanban.taskCardByTitle("PR #202: Reviewed feature")).toBeVisible({ + timeout: 5_000, + }); + + // Confirm via API + const { tasks: tasksAfter } = await apiClient.listTasks(seedData.workspaceId); + expect(tasksAfter.find((t) => t.title.includes("PR #202"))).toBeTruthy(); + }); + + /** + * When the user goes to GitHub directly and approves the PR (without using + * the Kandev task), the task should be auto-deleted since the review is done. + */ + test("auto-deletes task when PR is approved on GitHub", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + + // --- Setup mock GitHub --- + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("test-user"); + await apiClient.mockGitHubAddPRs([ + { + number: 303, + title: "Quick fix to approve", + state: "open", + head_branch: "fix/quick", + base_branch: "main", + author_login: "contributor", + repo_owner: "testorg", + repo_name: "testrepo", + requested_reviewers: [{ login: "test-user", type: "User" }], + }, + ]); + + // Create a step without auto-start + const inboxStep = await apiClient.createWorkflowStep(seedData.workflowId, "Review Inbox", 0); + + const watch = await apiClient.createReviewWatch( + seedData.workspaceId, + seedData.workflowId, + inboxStep.id, + seedData.agentProfileId, + { repos: [{ owner: "testorg", name: "testrepo" }] }, + ); + + // Trigger → task created + await apiClient.triggerReviewWatch(watch.id); + + await expect + .poll( + async () => { + const { tasks } = await apiClient.listTasks(seedData.workspaceId); + return tasks.find((t) => t.title.includes("PR #303")); + }, + { timeout: 15_000 }, + ) + .toBeTruthy(); + + // Navigate to kanban and verify task visible + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await expect(kanban.taskCardByTitle("PR #303: Quick fix to approve")).toBeVisible({ + timeout: 15_000, + }); + + // --- Simulate: user approved the PR on GitHub directly --- + await apiClient.mockGitHubAddReviews("testorg", "testrepo", 303, [ + { + id: 1, + author: "test-user", + state: "APPROVED", + body: "LGTM", + created_at: new Date().toISOString(), + }, + ]); + + // Trigger watch → should detect approved PR and delete the task + await apiClient.triggerReviewWatch(watch.id); + + // Verify task was deleted + await expect(kanban.taskCardByTitle("PR #303: Quick fix to approve")).not.toBeVisible({ + timeout: 15_000, + }); + + const { tasks: tasksAfterCleanup } = await apiClient.listTasks(seedData.workspaceId); + expect(tasksAfterCleanup.find((t) => t.title.includes("PR #303"))).toBeUndefined(); + }); +}); diff --git a/apps/web/e2e/tests/pr-watcher-missing-branch.spec.ts b/apps/web/e2e/tests/pr-watcher-missing-branch.spec.ts new file mode 100644 index 0000000000..f1633a69bd --- /dev/null +++ b/apps/web/e2e/tests/pr-watcher-missing-branch.spec.ts @@ -0,0 +1,126 @@ +import { test, expect } from "../fixtures/test-base"; +import { KanbanPage } from "../pages/kanban-page"; +import { SessionPage } from "../pages/session-page"; + +test.describe("PR watcher missing branch", () => { + /** + * When a task is created from a PR watcher and the PR's head branch has + * already been deleted (merged PR), clicking the task should show a + * user-friendly error in the chat with options to archive or delete. + * + * Setup: + * - Create task with checkout_branch pointing to a non-existent branch + * (simulates what the PR watcher creates when a PR is found) + * - Associate a mock PR with the task + * - Navigate to the task — frontend auto-prepares the session + * - Environment preparation fails because the branch doesn't exist + * - Assert the guidance message appears in chat + */ + test("shows guidance message when PR branch is deleted", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + + // --- Setup mock GitHub --- + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("test-user"); + + const prBranch = "feature/already-merged-and-deleted"; + + // Mock the PR that was already merged (branch deleted on remote) + await apiClient.mockGitHubAddPRs([ + { + number: 999, + title: "Already merged feature", + state: "closed", + head_branch: prBranch, + base_branch: "main", + author_login: "test-user", + repo_owner: "testorg", + repo_name: "testrepo", + }, + ]); + + // Create a task as the PR watcher would: with a checkout_branch that + // no longer exists on remote (PR was merged, branch deleted). + const task = await apiClient.createTask( + seedData.workspaceId, + "PR #999: Already merged feature", + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repositories: [ + { + repository_id: seedData.repositoryId, + base_branch: "main", + checkout_branch: prBranch, + }, + ], + metadata: { + agent_profile_id: seedData.agentProfileId, + pr_number: 999, + pr_branch: prBranch, + pr_repo: "testorg/testrepo", + pr_author: "test-user", + }, + }, + ); + + // Associate the PR with the task (as the watcher would) + await apiClient.mockGitHubAssociateTaskPR({ + task_id: task.id, + owner: "testorg", + repo: "testrepo", + pr_number: 999, + pr_url: "https://github.com/testorg/testrepo/pull/999", + pr_title: "Already merged feature", + head_branch: prBranch, + base_branch: "main", + author_login: "test-user", + state: "closed", + }); + + // --- Navigate to the kanban and click the task --- + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + const card = kanban.taskCardByTitle("PR #999: Already merged feature"); + await expect(card).toBeVisible({ timeout: 15_000 }); + await card.click(); + + // Wait for navigation to session view + await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 }); + + const session = new SessionPage(testPage); + + // --- Assert the guidance message appears in chat --- + // The MissingBranchMessage component renders with archive/delete action buttons. + await expect(session.chat.getByText("no longer exists", { exact: false })).toBeVisible({ + timeout: 30_000, + }); + + // Verify the action buttons are present + await expect(session.chat.getByTestId("missing-branch-archive-button")).toBeVisible({ + timeout: 5_000, + }); + await expect(session.chat.getByTestId("missing-branch-delete-button")).toBeVisible({ + timeout: 5_000, + }); + + // Verify the session state via API — should be FAILED + const { sessions } = await apiClient.listTaskSessions(task.id); + const failedSession = sessions.find((s) => s.state === "FAILED"); + expect(failedSession).toBeTruthy(); + + // Verify the guidance message metadata via API + const { messages } = await apiClient.listSessionMessages(failedSession!.id); + const guidanceMsg = messages.find( + (m: Record) => + (m.metadata as Record)?.failure_kind === "missing_pr_branch", + ); + expect(guidanceMsg).toBeTruthy(); + expect((guidanceMsg as Record).content).toContain(prBranch); + }); +}); diff --git a/apps/web/hooks/use-processed-messages.ts b/apps/web/hooks/use-processed-messages.ts index eeae4d7a64..0bff4526b3 100644 --- a/apps/web/hooks/use-processed-messages.ts +++ b/apps/web/hooks/use-processed-messages.ts @@ -259,15 +259,32 @@ export function useProcessedMessages( return visibleMessages.filter((c) => c.author_type !== "user").length; }, [visibleMessages]); + // Separate footer action messages (e.g. missing branch guidance with archive/delete + // buttons) from regular messages so they render after the env prep error status. + const { regularMessages, footerActionMessages } = useMemo(() => { + const regular: Message[] = []; + const footer: Message[] = []; + for (const msg of allMessages) { + const meta = msg.metadata as Record | undefined; + if (Array.isArray(meta?.actions) && !meta?.recovery_actions) { + footer.push(msg); + } else { + regular.push(msg); + } + } + return { regularMessages: regular, footerActionMessages: footer }; + }, [allMessages]); + const groupedItems = useMemo( - () => groupActivityMessages(allMessages), - [allMessages], + () => groupActivityMessages(regularMessages), + [regularMessages], ); return { visibleMessages, allMessages, groupedItems, + footerActionMessages, toolCallIds, permissionsByToolCallId, childrenByParentToolCallId, diff --git a/apps/web/lib/types/backend.ts b/apps/web/lib/types/backend.ts index 4c239a7416..8565b3f73d 100644 --- a/apps/web/lib/types/backend.ts +++ b/apps/web/lib/types/backend.ts @@ -229,6 +229,8 @@ export type TaskSessionStateChangedPayload = { metadata?: Record; is_passthrough?: boolean; error_message?: string; + /** When true, the frontend should not show an error toast for this state change. */ + suppress_toast?: boolean; // Workflow-related fields (sent during workflow transitions) review_status?: string; workflow_step_id?: string; diff --git a/apps/web/lib/ws/handlers/agent-session.ts b/apps/web/lib/ws/handlers/agent-session.ts index 2755ab01b2..5c659a32e0 100644 --- a/apps/web/lib/ws/handlers/agent-session.ts +++ b/apps/web/lib/ws/handlers/agent-session.ts @@ -162,7 +162,7 @@ export function registerTaskSessionHandlers(store: StoreApi): WsHandle upsertTaskSessionList(store, taskId, sessionId, payload, sessionUpdate); extractContextWindow(store, sessionId, payload); - if (newState === "FAILED") { + if (newState === "FAILED" && payload.suppress_toast !== true) { store.getState().setSessionFailureNotification({ sessionId, taskId,