diff --git a/apps/backend/internal/agent/runtime/routingerr/sanitize.go b/apps/backend/internal/agent/runtime/routingerr/sanitize.go index c0a1e20ea7..67790e2450 100644 --- a/apps/backend/internal/agent/runtime/routingerr/sanitize.go +++ b/apps/backend/internal/agent/runtime/routingerr/sanitize.go @@ -2,6 +2,7 @@ package routingerr import ( "regexp" + "strings" "unicode/utf8" ) @@ -40,20 +41,67 @@ var redactions = []redaction{ {regexp.MustCompile(`/home/[^/\s]+/`), "/home//"}, } -// Redact applies the same credential and home-path rules as Sanitize but -// performs no truncation, so a caller that needs to bound the result by a -// different rule (e.g. keeping the newest bytes of a longer budget cut) sees -// every credential intact and redacted rather than racing a head-truncation -// that runs before it can see the whole input. Redact is idempotent: -// applying it twice equals applying it once. +var ( + localUnixPathPattern = regexp.MustCompile(`/(?:[^/\s"']+/)+[^\r\n\s"'<>]+`) + localWindowsPathPattern = regexp.MustCompile(`(?i)[A-Z]:[\\/][^\r\n"']+`) +) + +func redactLocalPaths(s string) string { + s = localWindowsPathPattern.ReplaceAllStringFunc(s, func(path string) string { + if len(path) >= 4 && path[2:4] == "//" { + return path + } + return "[path-redacted]" + }) + matches := localUnixPathPattern.FindAllStringIndex(s, -1) + if len(matches) == 0 { + return s + } + + var redacted strings.Builder + redacted.Grow(len(s)) + last := 0 + for _, match := range matches { + start, end := match[0], match[1] + redacted.WriteString(s[last:start]) + path := s[start:end] + if start >= 2 && s[start-2:start] == ":/" { + redacted.WriteString(path) + last = end + continue + } + redacted.WriteString(redactUnixPath(path)) + last = end + } + redacted.WriteString(s[last:]) + return redacted.String() +} + +func redactUnixPath(path string) string { + if strings.Contains(path, "[path-redacted]") { + return path + } + switch { + case strings.HasPrefix(path, "/Users/"): + return "/Users//[path-redacted]" + case strings.HasPrefix(path, "/home/"): + return "/home//[path-redacted]" + default: + return "[path-redacted]" + } +} + +// Redact applies the credential and local-path rules without truncation. +// The function is idempotent: applying it twice equals applying it once. func Redact(s string) string { for _, r := range redactions { s = r.pattern.ReplaceAllString(s, r.replace) } + s = redactLocalPaths(s) return s } -// Sanitize redacts likely credentials, normalizes home paths, and truncates +// Sanitize redacts likely credentials and local paths, then truncates // to MaxRawExcerptBytes on a rune boundary so a multi-byte character (e.g. // Vietnamese, CJK) is never split into invalid UTF-8. The function is // idempotent: applying it twice equals applying it once. diff --git a/apps/backend/internal/agent/runtime/routingerr/sanitize_test.go b/apps/backend/internal/agent/runtime/routingerr/sanitize_test.go index 56e58328b8..b8e611085b 100644 --- a/apps/backend/internal/agent/runtime/routingerr/sanitize_test.go +++ b/apps/backend/internal/agent/runtime/routingerr/sanitize_test.go @@ -69,6 +69,30 @@ func TestSanitize_RedactionsGolden(t *testing.T) { mustNotHave: []string{"/home/bob/"}, mustHave: []string{"/home//"}, }, + { + name: "temporary workspace path", + in: "file at /tmp/kandev/task/repo/main.go failed", + mustNotHave: []string{"/tmp/kandev/task/repo/main.go"}, + mustHave: []string{"[path-redacted]"}, + }, + { + name: "workspace root path", + in: "checkout failed in /workspace/kandev/repo/main.go", + mustNotHave: []string{"/workspace/kandev/repo/main.go"}, + mustHave: []string{"[path-redacted]"}, + }, + { + name: "path redaction does not consume a following endpoint", + in: "cannot read /workspace/.env while connecting to https://endpoint.example.test", + mustNotHave: []string{"/workspace/.env"}, + mustHave: []string{"https://endpoint.example.test"}, + }, + { + name: "windows workspace path", + in: `checkout failed in C:\Users\alice\workspace\repo\main.go`, + mustNotHave: []string{`C:\Users\alice\workspace\repo\main.go`}, + mustHave: []string{"[path-redacted]"}, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -96,6 +120,7 @@ func TestSanitize_Idempotent(t *testing.T) { "--api-key=ABCDEFGHIJKLMNOPQRSTUV --rest", "password: hunter2 token: foobar secret=abc", "/Users/me/projects/x /home/me/x", + "/tmp/kandev/repo/main.go C:\\workspace\\repo\\main.go", } for _, in := range inputs { first := Sanitize(in) diff --git a/apps/backend/internal/orchestrator/executor/executor_launch_failure_classification_test.go b/apps/backend/internal/orchestrator/executor/executor_launch_failure_classification_test.go index 9d259db6c4..a0654c6440 100644 --- a/apps/backend/internal/orchestrator/executor/executor_launch_failure_classification_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_launch_failure_classification_test.go @@ -23,6 +23,15 @@ func TestClassifyLaunchFailureUsesTypedBaseBranchCategory(t *testing.T) { } } +func TestClassifyLaunchFailureUsesWorkspaceCheckoutCategory(t *testing.T) { + classification := classifyLaunchFailure(errors.Join( + errors.New("PR ref could not be materialized"), worktree.ErrWorkspaceCheckoutFailed, + )) + if classification.code != models.LaunchErrorCategoryWorkspaceCheckoutFailed { + t.Fatalf("classification code = %q, want %q", classification.code, models.LaunchErrorCategoryWorkspaceCheckoutFailed) + } +} + func TestTransitionLaunchFailurePersistsTypedErrorAndExactTaskRepository(t *testing.T) { repo := newMockRepository() repo.sessions["session-1"] = &models.TaskSession{ @@ -58,21 +67,21 @@ func TestTransitionLaunchFailurePersistsTypedErrorAndExactTaskRepository(t *test } } -func TestLaunchFailureReviewActionRequiresSuccessfulEligibilityResolver(t *testing.T) { +func TestGenericLaunchFailureAlwaysOffersRetryLaunch(t *testing.T) { exec := &Executor{} exec.launchFailureReviewEligibility = func(context.Context, string) (bool, error) { return true, nil } errorValue := exec.buildLastAgentError(context.Background(), "task-1", "", errors.New("start failed")) - if len(errorValue.RecoveryActions) != 1 || errorValue.RecoveryActions[0] != models.RecoveryActionMarkReviewDone { - t.Fatalf("eligible recovery actions = %#v, want mark_review_done", errorValue.RecoveryActions) + if len(errorValue.RecoveryActions) != 1 || errorValue.RecoveryActions[0] != models.RecoveryActionRetryLaunch { + t.Fatalf("eligible recovery actions = %#v, want retry_launch", errorValue.RecoveryActions) } exec.launchFailureReviewEligibility = func(context.Context, string) (bool, error) { return false, errors.New("lookup failed") } errorValue = exec.buildLastAgentError(context.Background(), "task-1", "", errors.New("start failed")) - if len(errorValue.RecoveryActions) != 0 { + if len(errorValue.RecoveryActions) != 1 || errorValue.RecoveryActions[0] != models.RecoveryActionRetryLaunch { t.Fatalf("failed eligibility lookup exposed recovery actions = %#v", errorValue.RecoveryActions) } } diff --git a/apps/backend/internal/orchestrator/executor/launch_failure.go b/apps/backend/internal/orchestrator/executor/launch_failure.go index eaded569a1..e8c2416b06 100644 --- a/apps/backend/internal/orchestrator/executor/launch_failure.go +++ b/apps/backend/internal/orchestrator/executor/launch_failure.go @@ -20,6 +20,11 @@ type launchFailureClassification struct { func classifyLaunchFailure(err error) launchFailureClassification { switch { + case errors.Is(err, worktree.ErrWorkspaceCheckoutFailed): + return launchFailureClassification{ + code: models.LaunchErrorCategoryWorkspaceCheckoutFailed, + message: "The workspace could not be prepared for this launch.", + } case errors.Is(err, worktree.ErrInvalidBaseBranch): return launchFailureClassification{ code: models.LaunchErrorCategoryBaseBranchMissing, @@ -38,20 +43,27 @@ func classifyLaunchFailure(err error) launchFailureClassification { } } -func launchFailureRecoveryActions(taskRepositoryID string, markReviewDone bool) []string { +func launchFailureRecoveryActions(category, taskRepositoryID string, markReviewDone bool) []string { actions := make([]string, 0, 3) if strings.TrimSpace(taskRepositoryID) != "" { - actions = append(actions, - models.RecoveryActionRetryDefault, - models.RecoveryActionPickBaseBranch, - ) + switch category { + case models.LaunchErrorCategoryBaseBranchMissing: + actions = append(actions, + models.RecoveryActionRetryDefault, + models.RecoveryActionPickBaseBranch, + ) + case models.LaunchErrorCategoryDefaultBranchUnresolved: + actions = append(actions, models.RecoveryActionPickBaseBranch) + } } - if markReviewDone { + if category == models.LaunchErrorCategoryWorkspaceCheckoutFailed || category == models.LaunchErrorCategoryGenericLaunchFailure { + actions = append(actions, models.RecoveryActionRetryLaunch) + } + if category == models.LaunchErrorCategoryPRAlreadyClosed && markReviewDone { actions = append(actions, models.RecoveryActionMarkReviewDone) } - return models.NormalizeRecoveryActions(actions) + return models.NormalizeRecoveryActionsForCategory(category, actions) } - func (e *Executor) buildLastAgentError( ctx context.Context, taskID, taskRepositoryID string, @@ -80,7 +92,7 @@ func (e *Executor) buildLastAgentError( OccurredAt: occurredAt, Code: classification.code, Details: details, - RecoveryActions: launchFailureRecoveryActions(taskRepositoryID, markReviewDone), + RecoveryActions: launchFailureRecoveryActions(classification.code, taskRepositoryID, markReviewDone), TaskRepositoryID: taskRepositoryID, StampValue: models.StableLaunchErrorStamp( taskID, classification.code, taskRepositoryID, occurredAt.Format(time.RFC3339Nano), diff --git a/apps/backend/internal/orchestrator/handlers/handlers.go b/apps/backend/internal/orchestrator/handlers/handlers.go index e697be94ba..0d75169890 100644 --- a/apps/backend/internal/orchestrator/handlers/handlers.go +++ b/apps/backend/internal/orchestrator/handlers/handlers.go @@ -312,7 +312,7 @@ func (h *Handlers) wsRecoverTaskLaunch(ctx context.Context, msg *ws.Message) (*w if req.ErrorStamp == "" { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "error_stamp is required", nil) } - if req.Action != "mark_review_done" && req.TaskRepositoryID == "" { + if req.Action != "mark_review_done" && req.Action != "retry_launch" && req.TaskRepositoryID == "" { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "task_repository_id is required for branch recovery", nil) } if req.Action == "pick_base_branch" && req.BaseBranch == "" { diff --git a/apps/backend/internal/orchestrator/task_launch_recovery.go b/apps/backend/internal/orchestrator/task_launch_recovery.go index f9f0fd50a6..fa39bdbc6b 100644 --- a/apps/backend/internal/orchestrator/task_launch_recovery.go +++ b/apps/backend/internal/orchestrator/task_launch_recovery.go @@ -27,6 +27,7 @@ const ( taskLaunchRecoveryRetryDefault = models.RecoveryActionRetryDefault taskLaunchRecoveryPickBranch = models.RecoveryActionPickBaseBranch taskLaunchRecoveryMarkDone = models.RecoveryActionMarkReviewDone + taskLaunchRecoveryRetryLaunch = models.RecoveryActionRetryLaunch ) // TaskLaunchRecoveryRequest is the server-side shape of task.launch.recover. @@ -118,6 +119,11 @@ func (s *Service) RecoverTaskLaunch(ctx context.Context, req *TaskLaunchRecovery if err != nil { return nil, err } + case taskLaunchRecoveryRetryLaunch: + responseSessionID, err = s.relaunchAndClearTaskLaunchRecovery(ctx, req, source) + if err != nil { + return nil, err + } case taskLaunchRecoveryMarkDone: if err := s.markTaskLaunchReviewDone(ctx, req, source); err != nil { return nil, err @@ -159,11 +165,11 @@ func (s *Service) validateTaskLaunchRecoveryRequest(req *TaskLaunchRecoveryReque return ErrTaskLaunchRecoveryInvalid } switch req.Action { - case taskLaunchRecoveryRetryDefault, taskLaunchRecoveryPickBranch, taskLaunchRecoveryMarkDone: + case taskLaunchRecoveryRetryDefault, taskLaunchRecoveryPickBranch, taskLaunchRecoveryRetryLaunch, taskLaunchRecoveryMarkDone: default: return fmt.Errorf("unsupported task launch recovery action %q", req.Action) } - if req.Action != taskLaunchRecoveryMarkDone && strings.TrimSpace(req.TaskRepositoryID) == "" { + if (req.Action == taskLaunchRecoveryRetryDefault || req.Action == taskLaunchRecoveryPickBranch) && strings.TrimSpace(req.TaskRepositoryID) == "" { return fmt.Errorf("task_repository_id is required for %s", req.Action) } if req.Action == taskLaunchRecoveryPickBranch && strings.TrimSpace(req.BaseBranch) == "" { @@ -195,11 +201,13 @@ func (s *Service) loadTaskLaunchRecoverySource(ctx context.Context, req *TaskLau return nil, err } } - if req.Action != taskLaunchRecoveryMarkDone { + if req.Action == taskLaunchRecoveryRetryDefault || req.Action == taskLaunchRecoveryPickBranch { activeRepositoryID := source.taskRepositoryID() if activeRepositoryID == "" || activeRepositoryID != req.TaskRepositoryID { return nil, fmt.Errorf("task_repository_id does not match the active launch error") } + } else if req.Action == taskLaunchRecoveryRetryLaunch && req.TaskRepositoryID != "" && source.taskRepositoryID() != req.TaskRepositoryID { + return nil, fmt.Errorf("task_repository_id does not match the active launch error") } return source, nil } @@ -250,6 +258,7 @@ func (s *Service) loadAuthoritativeTaskLaunchRecoveryError( Stamp: lastError.Stamp(), OccurredAt: lastError.OccurredAt, Preview: lastError.Message, + Details: lastError.Details, Category: lastError.Code, RecoveryActions: lastError.RecoveryActions, }, @@ -261,6 +270,7 @@ func (s *Service) loadAuthoritativeTaskLaunchRecoveryError( Stamp: launchError.Stamp(), OccurredAt: launchError.OccurredAt, Preview: launchError.Message, + Details: launchError.Details, Category: launchError.Code, RecoveryActions: launchError.RecoveryActions, } diff --git a/apps/backend/internal/orchestrator/task_launch_recovery_test.go b/apps/backend/internal/orchestrator/task_launch_recovery_test.go index ee4107c997..1cac6b1188 100644 --- a/apps/backend/internal/orchestrator/task_launch_recovery_test.go +++ b/apps/backend/internal/orchestrator/task_launch_recovery_test.go @@ -59,6 +59,18 @@ type blockingTaskLaunchRecoveryWorktree struct { err error } +func TestValidateTaskLaunchRecoveryRequestAllowsRetryLaunchWithoutRepository(t *testing.T) { + svc := &Service{} + err := svc.validateTaskLaunchRecoveryRequest(&TaskLaunchRecoveryRequest{ + TaskID: "task-1", + Action: models.RecoveryActionRetryLaunch, + ErrorStamp: "error-1", + }) + if err != nil { + t.Fatalf("retry_launch validation error = %v", err) + } +} + func (f blockingTaskLaunchRecoveryWorktree) ResolveRemoteDefaultBranch(context.Context, string) (string, error) { f.started <- struct{}{} <-f.release @@ -69,6 +81,13 @@ func seedTaskLaunchRecoveryFixture(t *testing.T, repo *sqliterepo.Repository, ta t.Helper() ctx := context.Background() now := time.Now().UTC() + category := models.LaunchErrorCategoryGenericLaunchFailure + switch action { + case models.RecoveryActionRetryDefault, models.RecoveryActionPickBaseBranch: + category = models.LaunchErrorCategoryBaseBranchMissing + case models.RecoveryActionMarkReviewDone: + category = models.LaunchErrorCategoryPRAlreadyClosed + } if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-recovery", Name: "Recovery", CreatedAt: now, UpdatedAt: now}); err != nil { t.Fatalf("CreateWorkspace: %v", err) } @@ -81,7 +100,7 @@ func seedTaskLaunchRecoveryFixture(t *testing.T, repo *sqliterepo.Repository, ta models.MetaKeyLastLaunchError: models.TaskLaunchError{ Message: "launch failed", OccurredAt: now, - Code: models.LaunchErrorCategoryGenericLaunchFailure, + Code: category, RecoveryActions: []string{action}, TaskRepositoryID: taskRepositoryID, StampValue: "recovery-stamp", diff --git a/apps/backend/internal/repoclone/clone.go b/apps/backend/internal/repoclone/clone.go index 8f3cf6709f..6452cfe7cf 100644 --- a/apps/backend/internal/repoclone/clone.go +++ b/apps/backend/internal/repoclone/clone.go @@ -524,7 +524,7 @@ func (c *Cloner) refreshWorkspaceRepository( return err } if prNumber > 0 { - prRefspec := fmt.Sprintf("pull/%d/head:refs/remotes/origin/pr/%d", prNumber, prNumber) + prRefspec := fmt.Sprintf("pull/%d/head:refs/kandev/pull/%d/head", prNumber, prNumber) if err := runFetch(prRefspec); err != nil { return fmt.Errorf("refresh scoped workspace pull request: %w", err) } diff --git a/apps/backend/internal/repoclone/clone_auth_test.go b/apps/backend/internal/repoclone/clone_auth_test.go index 74606a2fca..09a5da9e09 100644 --- a/apps/backend/internal/repoclone/clone_auth_test.go +++ b/apps/backend/internal/repoclone/clone_auth_test.go @@ -174,7 +174,7 @@ helper=${GIT_CONFIG_VALUE_1#!} t.Fatal(err) } if got := string(args); !strings.Contains(got, "fetch\n--prune\n--force\n--no-tags\norigin\n") || - !strings.Contains(got, "pull/42/head:refs/remotes/origin/pr/42\n") || strings.Contains(got, "--all") { + !strings.Contains(got, "pull/42/head:refs/kandev/pull/42/head\n") || strings.Contains(got, "--all") { t.Fatalf("refresh args = %q", got) } scope, err := os.ReadFile(capturePath + ".scope") diff --git a/apps/backend/internal/task/models/launch_errors.go b/apps/backend/internal/task/models/launch_errors.go index 874c772b5d..2d0df3efc1 100644 --- a/apps/backend/internal/task/models/launch_errors.go +++ b/apps/backend/internal/task/models/launch_errors.go @@ -20,6 +20,7 @@ const ( LaunchErrorCategoryBaseBranchMissing = "base_branch_missing" LaunchErrorCategoryPRAlreadyClosed = "pr_already_closed" LaunchErrorCategoryDefaultBranchUnresolved = "default_branch_unresolved" + LaunchErrorCategoryWorkspaceCheckoutFailed = "workspace_checkout_failed" LaunchErrorCategoryGenericLaunchFailure = "generic_launch_failure" ) @@ -28,6 +29,7 @@ const ( RecoveryActionRetryDefault = "retry_default" RecoveryActionPickBaseBranch = "pick_base_branch" RecoveryActionMarkReviewDone = "mark_review_done" + RecoveryActionRetryLaunch = "retry_launch" ) const ( @@ -74,9 +76,42 @@ func NormalizeRecoveryActions(actions []string) []string { return result } +// NormalizeRecoveryActionsForCategory retains only the actions that are +// meaningful for a typed launch-failure category. Unknown categories keep the +// legacy generic normalization so unrelated runtime errors remain compatible. +func NormalizeRecoveryActionsForCategory(category string, actions []string) []string { + normalized := NormalizeRecoveryActions(actions) + var allowed map[string]struct{} + switch category { + case LaunchErrorCategoryBaseBranchMissing: + allowed = map[string]struct{}{ + RecoveryActionRetryDefault: {}, RecoveryActionPickBaseBranch: {}, + } + case LaunchErrorCategoryDefaultBranchUnresolved: + allowed = map[string]struct{}{RecoveryActionPickBaseBranch: {}} + case LaunchErrorCategoryWorkspaceCheckoutFailed, LaunchErrorCategoryGenericLaunchFailure: + if len(normalized) == 0 { + return nil + } + return []string{RecoveryActionRetryLaunch} + case LaunchErrorCategoryPRAlreadyClosed: + allowed = map[string]struct{}{RecoveryActionMarkReviewDone: {}} + default: + return normalized + } + + result := make([]string, 0, len(normalized)) + for _, action := range normalized { + if _, ok := allowed[action]; ok { + result = append(result, action) + } + } + return result +} + func isKnownRecoveryAction(action string) bool { switch action { - case RecoveryActionRetryDefault, RecoveryActionPickBaseBranch, RecoveryActionMarkReviewDone: + case RecoveryActionRetryDefault, RecoveryActionPickBaseBranch, RecoveryActionMarkReviewDone, RecoveryActionRetryLaunch: return true default: return false @@ -85,7 +120,8 @@ func isKnownRecoveryAction(action string) bool { func normalizeLastAgentError(value LastAgentError) LastAgentError { value.Message = truncateUTF8Bytes(value.Message, maxLaunchErrorMessageBytes) - value.RecoveryActions = NormalizeRecoveryActions(value.RecoveryActions) + value.Code = truncateUTF8Bytes(value.Code, maxLaunchErrorCategoryBytes) + value.RecoveryActions = NormalizeRecoveryActionsForCategory(value.Code, value.RecoveryActions) value.TaskRepositoryID = truncateUTF8Bytes(value.TaskRepositoryID, maxTaskRepositoryIDBytes) value.StampValue = boundedLaunchErrorStamp(value.StampValue) value.Details = truncateUTF8Bytes(value.Details, maxLaunchErrorDetailsBytes) @@ -94,10 +130,10 @@ func normalizeLastAgentError(value LastAgentError) LastAgentError { func normalizeTaskLaunchError(value TaskLaunchError) TaskLaunchError { value.Message = truncateUTF8Bytes(value.Message, maxLaunchErrorMessageBytes) - value.RecoveryActions = NormalizeRecoveryActions(value.RecoveryActions) + value.Code = truncateUTF8Bytes(value.Code, maxLaunchErrorCategoryBytes) + value.RecoveryActions = NormalizeRecoveryActionsForCategory(value.Code, value.RecoveryActions) value.TaskRepositoryID = truncateUTF8Bytes(value.TaskRepositoryID, maxTaskRepositoryIDBytes) value.StampValue = boundedLaunchErrorStamp(value.StampValue) - value.Code = truncateUTF8Bytes(value.Code, maxLaunchErrorCategoryBytes) value.Details = truncateUTF8Bytes(value.Details, maxLaunchErrorDetailsBytes) return value } diff --git a/apps/backend/internal/task/models/launch_errors_test.go b/apps/backend/internal/task/models/launch_errors_test.go index 505e680c3b..de19a0fe19 100644 --- a/apps/backend/internal/task/models/launch_errors_test.go +++ b/apps/backend/internal/task/models/launch_errors_test.go @@ -30,6 +30,46 @@ func TestLoadLastAgentErrorUsesExplicitStampAndNormalizesRecoveryFields(t *testi }, lastError.RecoveryActions) } +func TestNormalizeRecoveryActionsForCategoryFiltersActionsByLaunchCause(t *testing.T) { + tests := []struct { + name string + category string + input []string + want []string + }{ + { + name: "generic launch failure", + category: LaunchErrorCategoryGenericLaunchFailure, + input: []string{RecoveryActionRetryDefault, RecoveryActionPickBaseBranch, RecoveryActionMarkReviewDone}, + want: []string{RecoveryActionRetryLaunch}, + }, + { + name: "workspace checkout failure", + category: LaunchErrorCategoryWorkspaceCheckoutFailed, + input: []string{RecoveryActionRetryDefault, RecoveryActionRetryLaunch}, + want: []string{RecoveryActionRetryLaunch}, + }, + { + name: "missing base branch", + category: LaunchErrorCategoryBaseBranchMissing, + input: []string{RecoveryActionRetryLaunch, RecoveryActionRetryDefault, RecoveryActionPickBaseBranch}, + want: []string{RecoveryActionRetryDefault, RecoveryActionPickBaseBranch}, + }, + { + name: "closed pull request", + category: LaunchErrorCategoryPRAlreadyClosed, + input: []string{RecoveryActionRetryLaunch, RecoveryActionMarkReviewDone}, + want: []string{RecoveryActionMarkReviewDone}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.want, NormalizeRecoveryActionsForCategory(test.category, test.input)) + }) + } +} + func TestTaskLaunchErrorStoreIsNoOpForSameStamp(t *testing.T) { firstTime := time.Date(2026, 8, 19, 10, 0, 0, 0, time.UTC) secondTime := firstTime.Add(time.Minute) diff --git a/apps/backend/internal/task/statussummary/model.go b/apps/backend/internal/task/statussummary/model.go index 69012f0be1..b12dfb7701 100644 --- a/apps/backend/internal/task/statussummary/model.go +++ b/apps/backend/internal/task/statussummary/model.go @@ -23,6 +23,7 @@ const ( // MaxActiveErrorPreviewBytes keeps an error decoration safe to send with // every task row without turning it into a message-stream transport. MaxActiveErrorPreviewBytes = 512 + MaxActiveErrorDetailsBytes = 4096 maxSessionIDBytes = 256 maxTaskRepositoryIDBytes = 256 maxPendingActionBytes = 128 @@ -64,6 +65,7 @@ type ActiveErrorSummary struct { Stamp string `json:"stamp"` OccurredAt time.Time `json:"occurred_at"` Preview string `json:"preview"` + Details string `json:"details,omitempty"` Category string `json:"category,omitempty"` RecoveryActions []string `json:"recovery_actions,omitempty"` } @@ -156,6 +158,7 @@ func validateActiveError(activeError *ActiveErrorSummary) error { {"active error task repository id", activeError.TaskRepositoryID, maxTaskRepositoryIDBytes}, {"active error stamp", activeError.Stamp, maxActiveErrorStampBytes}, {"active error preview", activeError.Preview, MaxActiveErrorPreviewBytes}, + {"active error details", activeError.Details, MaxActiveErrorDetailsBytes}, {"active error category", activeError.Category, maxActiveErrorCategoryBytes}, } for _, field := range fields { @@ -166,7 +169,7 @@ func validateActiveError(activeError *ActiveErrorSummary) error { if len(activeError.RecoveryActions) > 3 { return fmt.Errorf("active error has more than three recovery actions") } - if !slices.Equal(activeError.RecoveryActions, models.NormalizeRecoveryActions(activeError.RecoveryActions)) { + if !slices.Equal(activeError.RecoveryActions, models.NormalizeRecoveryActionsForCategory(activeError.Category, activeError.RecoveryActions)) { return fmt.Errorf("active error has unknown or duplicate recovery actions") } return nil diff --git a/apps/backend/internal/task/statussummary/model_test.go b/apps/backend/internal/task/statussummary/model_test.go index 4d19554bd8..4553a8ae4a 100644 --- a/apps/backend/internal/task/statussummary/model_test.go +++ b/apps/backend/internal/task/statussummary/model_test.go @@ -171,7 +171,7 @@ func TestTaskStatusSummaryProjectsTaskOwnedErrorFields(t *testing.T) { if got.ActiveError.Category != "pr_already_closed" { t.Fatalf("active error category = %q", got.ActiveError.Category) } - wantActions := []string{"mark_review_done", "retry_default"} + wantActions := []string{"mark_review_done"} if !reflect.DeepEqual(got.ActiveError.RecoveryActions, wantActions) { t.Fatalf("active error actions = %#v, want %#v", got.ActiveError.RecoveryActions, wantActions) } diff --git a/apps/backend/internal/task/statussummary/projector_events.go b/apps/backend/internal/task/statussummary/projector_events.go index 10eb54a54d..5c6e4f1a17 100644 --- a/apps/backend/internal/task/statussummary/projector_events.go +++ b/apps/backend/internal/task/statussummary/projector_events.go @@ -332,7 +332,7 @@ func applySummaryBaseline(state *projectionState, summary *TaskStatusSummary) { } if summary.ActiveError != nil { copy := *summary.ActiveError - copy.RecoveryActions = normalizeRecoveryActions(copy.RecoveryActions) + copy.RecoveryActions = normalizeRecoveryActionsForCategory(copy.Category, copy.RecoveryActions) if copy.SessionID != "" { state.errors[copy.SessionID] = © } else { diff --git a/apps/backend/internal/task/statussummary/projector_helpers.go b/apps/backend/internal/task/statussummary/projector_helpers.go index a8da4613f3..f451de0dea 100644 --- a/apps/backend/internal/task/statussummary/projector_helpers.go +++ b/apps/backend/internal/task/statussummary/projector_helpers.go @@ -170,18 +170,21 @@ func errorFromMap(now time.Time, sessionID string, data map[string]interface{}) occurredAt = now.UTC() } preview := truncateString(message, MaxActiveErrorPreviewBytes) + details := truncateString(stringField(data, "details"), MaxActiveErrorDetailsBytes) stamp := stringField(data, "stamp") if stamp == "" { stamp = occurredAt.UTC().Format(time.RFC3339Nano) + ":" + preview } + category := truncateString(firstString(data, "category", "code"), maxActiveErrorCategoryBytes) return &ActiveErrorSummary{ SessionID: truncateString(sessionID, maxSessionIDBytes), TaskRepositoryID: truncateString(stringField(data, "task_repository_id"), maxTaskRepositoryIDBytes), Stamp: truncateString(stamp, maxActiveErrorStampBytes), OccurredAt: occurredAt.UTC(), Preview: preview, - Category: truncateString(firstString(data, "category", "code"), maxActiveErrorCategoryBytes), - RecoveryActions: normalizeRecoveryActions(recoveryActionsValue(data["recovery_actions"])), + Details: details, + Category: category, + RecoveryActions: normalizeRecoveryActionsForCategory(category, recoveryActionsValue(data["recovery_actions"])), }, true } @@ -191,7 +194,7 @@ func errorEqual(a, b *ActiveErrorSummary) bool { } return a.SessionID == b.SessionID && a.TaskRepositoryID == b.TaskRepositoryID && a.Stamp == b.Stamp && a.OccurredAt.Equal(b.OccurredAt) && - a.Preview == b.Preview && a.Category == b.Category && + a.Preview == b.Preview && a.Details == b.Details && a.Category == b.Category && slices.Equal(a.RecoveryActions, b.RecoveryActions) } @@ -199,6 +202,10 @@ func normalizeRecoveryActions(actions []string) []string { return models.NormalizeRecoveryActions(actions) } +func normalizeRecoveryActionsForCategory(category string, actions []string) []string { + return models.NormalizeRecoveryActionsForCategory(category, actions) +} + func recoveryActionsValue(value interface{}) []string { switch actions := value.(type) { case []string: diff --git a/apps/backend/internal/task/statussummary/rebuild.go b/apps/backend/internal/task/statussummary/rebuild.go index 4360139f30..6ebe308c7e 100644 --- a/apps/backend/internal/task/statussummary/rebuild.go +++ b/apps/backend/internal/task/statussummary/rebuild.go @@ -140,8 +140,9 @@ func normalizeRebuildError(input *ActiveErrorSummary, now time.Time) *ActiveErro copy.Preview = truncateString(copy.Preview, MaxActiveErrorPreviewBytes) copy.SessionID = truncateString(copy.SessionID, maxSessionIDBytes) copy.TaskRepositoryID = truncateString(copy.TaskRepositoryID, maxTaskRepositoryIDBytes) + copy.Details = truncateString(copy.Details, MaxActiveErrorDetailsBytes) copy.Category = truncateString(copy.Category, maxActiveErrorCategoryBytes) - copy.RecoveryActions = normalizeRecoveryActions(copy.RecoveryActions) + copy.RecoveryActions = normalizeRecoveryActionsForCategory(copy.Category, copy.RecoveryActions) if copy.Stamp == "" { copy.Stamp = copy.OccurredAt.UTC().Format(time.RFC3339Nano) + ":" + copy.Preview } diff --git a/apps/backend/internal/worktree/errors.go b/apps/backend/internal/worktree/errors.go index 1a4b2235c2..86d8caed08 100644 --- a/apps/backend/internal/worktree/errors.go +++ b/apps/backend/internal/worktree/errors.go @@ -57,6 +57,10 @@ var ( // ErrNonFastForward is returned when a fetch/pull is rejected due to non-fast-forward updates. ErrNonFastForward = errors.New("non-fast-forward update rejected") + // ErrWorkspaceCheckoutFailed is returned when a launch cannot materialize + // the requested checkout ref without risking an existing local branch. + ErrWorkspaceCheckoutFailed = errors.New("workspace checkout failed") + // ErrGitCryptFailed is returned when git-crypt unlock fails during worktree creation. ErrGitCryptFailed = errors.New("git-crypt unlock failed") diff --git a/apps/backend/internal/worktree/manager_checkout_branch_test.go b/apps/backend/internal/worktree/manager_checkout_branch_test.go index b6c188875c..c56c33d045 100644 --- a/apps/backend/internal/worktree/manager_checkout_branch_test.go +++ b/apps/backend/internal/worktree/manager_checkout_branch_test.go @@ -259,7 +259,7 @@ func TestCreateWorktree_ManagedRefreshRunsOnlyForMaterialization(t *testing.T) { func TestCreateWorktree_ManagedRefreshUsesRefreshedPRHead(t *testing.T) { repoPath, wantSHA := initGitRepoWithPullRef(t, 974, "feature/fork-pr") - runGit(t, repoPath, "fetch", "origin", "pull/974/head:refs/remotes/origin/pr/974") + runGit(t, repoPath, "fetch", "origin", "pull/974/head:"+pullRequestSnapshotRef(974)) runGit(t, repoPath, "remote", "set-url", "origin", "https://127.0.0.1:1/never.git") mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) @@ -445,9 +445,9 @@ func advanceRemoteBranch(t *testing.T, repoPath, branch string) string { func initManagedPRCheckoutBranch(t *testing.T, prNumber int, branch string) (string, string) { t.Helper() repoPath, _ := initGitRepoWithPullRef(t, prNumber, branch) - prRef := fmt.Sprintf("pull/%d/head:refs/remotes/origin/pr/%d", prNumber, prNumber) + prRef := fmt.Sprintf("pull/%d/head:%s", prNumber, pullRequestSnapshotRef(prNumber)) runGit(t, repoPath, "fetch", "origin", prRef) - runGit(t, repoPath, "branch", branch, fmt.Sprintf("origin/pr/%d", prNumber)) + runGit(t, repoPath, "branch", branch, pullRequestSnapshotRef(prNumber)) originURL := strings.TrimSpace(runGit(t, repoPath, "remote", "get-url", "origin")) remoteClone := filepath.Join(t.TempDir(), "remote-pr-clone") runGit(t, filepath.Dir(remoteClone), "clone", originURL, remoteClone) @@ -455,7 +455,7 @@ func initManagedPRCheckoutBranch(t *testing.T, prNumber int, branch string) (str runGit(t, remoteClone, "config", "user.name", "Remote User") runGit(t, remoteClone, "config", "commit.gpgsign", "false") runGit(t, remoteClone, "fetch", "origin", prRef) - runGit(t, remoteClone, "checkout", "-b", branch, fmt.Sprintf("origin/pr/%d", prNumber)) + runGit(t, remoteClone, "checkout", "-b", branch, pullRequestSnapshotRef(prNumber)) writeRepoFile(t, remoteClone, "remote-pr-refresh.txt", "refreshed PR\n") runGit(t, remoteClone, "add", "remote-pr-refresh.txt") runGit(t, remoteClone, "commit", "-m", "refreshed PR commit") diff --git a/apps/backend/internal/worktree/manager_git.go b/apps/backend/internal/worktree/manager_git.go index 5238293e9b..58242aecb7 100644 --- a/apps/backend/internal/worktree/manager_git.go +++ b/apps/backend/internal/worktree/manager_git.go @@ -15,6 +15,13 @@ import ( "github.com/kandev/kandev/internal/common/subproc" ) +// pullRequestSnapshotRef is internal state owned by Kandev. Keeping PR heads +// outside refs/remotes/origin prevents ordinary remote pruning or a user branch +// named pr/ from changing the commit selected for a task launch. +func pullRequestSnapshotRef(prNumber int) string { + return fmt.Sprintf("refs/kandev/pull/%d/head", prNumber) +} + // isGitRepo checks if a path is a Git repository. func (m *Manager) isGitRepo(path string) bool { gitDir := filepath.Join(path, ".git") @@ -212,18 +219,16 @@ func (m *Manager) prepareCheckoutFromRefreshedOrigin(ctx context.Context, repoPa } // prepareBranchFromRefreshedOrigin selects a provider-refreshed source branch -// without contacting origin. A PR number selects the dedicated origin/pr/ -// ref, which is also how fork PR heads are kept available after the -// authenticated refresh. When both refs exist, the selected ref is the one -// that contains the other. A local-only ref is preserved, a refreshed remote -// ref is returned to the caller as the worktree start point, and divergence or -// an unverified relationship fails closed. +// without contacting origin. A PR number selects the dedicated Kandev-owned +// snapshot ref, which is also how fork PR heads are kept available after the +// authenticated refresh. PR preparation never creates or resets a local +// branch with the PR's source name. func (m *Manager) prepareBranchFromRefreshedOrigin( ctx context.Context, repoPath, localBranch, sourceBranch string, prNumber int, ) (string, error) { remoteRef := "origin/" + sourceBranch if prNumber > 0 { - remoteRef = fmt.Sprintf("origin/pr/%d", prNumber) + remoteRef = pullRequestSnapshotRef(prNumber) } localExists, err := m.branchExists(ctx, repoPath, localBranch) if err != nil { @@ -233,6 +238,12 @@ func (m *Manager) prepareBranchFromRefreshedOrigin( if err != nil { return "", err } + if prNumber > 0 { + if !remoteExists { + return "", fmt.Errorf("required fetched remote ref %q is missing: %w", remoteRef, ErrWorkspaceCheckoutFailed) + } + return remoteRef, nil + } if !localExists && !remoteExists { return "", nil } diff --git a/apps/backend/internal/worktree/manager_lifecycle.go b/apps/backend/internal/worktree/manager_lifecycle.go index c8adb9b911..b11630f3b4 100644 --- a/apps/backend/internal/worktree/manager_lifecycle.go +++ b/apps/backend/internal/worktree/manager_lifecycle.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "time" @@ -988,13 +989,36 @@ func (m *Manager) validateTaskDir(taskDir string) error { // addWorktreeForBranch creates the git worktree, trying the checkout branch directly first // and falling back to a suffixed branch if the checkout branch is already in use. // When a checkout branch is specified, it sets the upstream tracking branch to -// origin/ so ahead/behind counts are relative to the PR's remote branch. +// origin/ when that ref points to the worktree's start point. func (m *Manager) addWorktreeForBranch(ctx context.Context, req CreateRequest, worktreePath, fallbackBranch, startPoint, baseRef string) (string, string, error) { if req.CheckoutBranch == "" { id, err := m.gitAddWorktree(ctx, req.RepositoryPath, fallbackBranch, worktreePath, baseRef) return id, fallbackBranch, err } + // A pull request head is an immutable remote ref for this launch. Never + // update or reset a local branch with the same name as the PR head: that + // branch may belong to another task and may contain unpushed work. + if req.PRNumber > 0 { + branchName := req.CheckoutBranch + exists, probeErr := m.branchExists(ctx, req.RepositoryPath, branchName) + if probeErr != nil { + return "", "", fmt.Errorf("verify PR checkout branch %q: %w: %w", branchName, ErrWorkspaceCheckoutFailed, probeErr) + } + if exists { + return m.addPRWorktreeWithUniqueBranch(ctx, req, worktreePath, startPoint) + } + id, addErr := m.gitAddWorktree(ctx, req.RepositoryPath, branchName, worktreePath, startPoint) + if addErr == nil { + m.setUpstreamIfMatchesStartPoint(ctx, worktreePath, branchName, req.CheckoutBranch, startPoint) + return id, branchName, nil + } + if !errors.Is(addErr, ErrBranchExists) { + return "", "", fmt.Errorf("materialize PR checkout branch %q: %w: %w", branchName, ErrWorkspaceCheckoutFailed, addErr) + } + return m.addPRWorktreeWithUniqueBranch(ctx, req, worktreePath, startPoint) + } + // Try checking out the PR branch directly (common case: single task per PR). var id string var err error @@ -1021,6 +1045,51 @@ func (m *Manager) addWorktreeForBranch(ctx context.Context, req CreateRequest, w return id, suffixed, err } +const prCheckoutBranchAttempts = 20 + +func prCheckoutBranchCandidates(checkoutBranch, taskID string) []string { + suffix := TaskDirSuffix(taskID) + if suffix == "" { + suffix = SmallSuffix(3) + } + candidates := make([]string, 0, prCheckoutBranchAttempts) + for attempt := 0; attempt < prCheckoutBranchAttempts; attempt++ { + candidate := checkoutBranch + "-" + suffix + if attempt > 0 { + candidate += "-" + strconv.Itoa(attempt) + } + candidates = append(candidates, candidate) + } + return candidates +} + +func (m *Manager) addPRWorktreeWithUniqueBranch(ctx context.Context, req CreateRequest, worktreePath, startPoint string) (string, string, error) { + for _, branchName := range prCheckoutBranchCandidates(req.CheckoutBranch, req.TaskID) { + exists, probeErr := m.branchExists(ctx, req.RepositoryPath, branchName) + if probeErr != nil { + return "", "", fmt.Errorf("verify PR checkout branch %q: %w: %w", branchName, ErrWorkspaceCheckoutFailed, probeErr) + } + if exists { + continue + } + id, addErr := m.gitAddWorktree(ctx, req.RepositoryPath, branchName, worktreePath, startPoint) + if addErr == nil { + m.setUpstreamIfMatchesStartPoint(ctx, worktreePath, branchName, req.CheckoutBranch, startPoint) + return id, branchName, nil + } + if !errors.Is(addErr, ErrBranchExists) { + return "", "", fmt.Errorf("materialize PR checkout branch %q: %w: %w", branchName, ErrWorkspaceCheckoutFailed, addErr) + } + } + return "", "", fmt.Errorf( + "materialize PR checkout branch %q after %d candidates: %w: %w", + req.CheckoutBranch, + prCheckoutBranchAttempts, + ErrWorkspaceCheckoutFailed, + ErrBranchExists, + ) +} + // setUpstreamIfExists sets the upstream tracking branch for a worktree branch // to origin/ if the remote-tracking ref exists. Non-fatal on failure. func (m *Manager) setUpstreamIfExists(ctx context.Context, worktreePath, localBranch, remoteBranch string) { @@ -1041,6 +1110,25 @@ func (m *Manager) setUpstreamIfExists(ctx context.Context, worktreePath, localBr } } +func (m *Manager) setUpstreamIfMatchesStartPoint( + ctx context.Context, + worktreePath, localBranch, remoteBranch, startPoint string, +) { + upstream := "origin/" + remoteBranch + upstreamOID, upstreamErr := m.refCommitOID(ctx, worktreePath, upstream) + startOID, startErr := m.refCommitOID(ctx, worktreePath, startPoint) + if upstreamErr != nil || startErr != nil || upstreamOID != startOID { + return + } + m.setUpstreamIfExists(ctx, worktreePath, localBranch, remoteBranch) +} + +func (m *Manager) refCommitOID(ctx context.Context, worktreePath, ref string) (string, error) { + cmd := m.newNonInteractiveGitCmd(ctx, worktreePath, "rev-parse", "--verify", ref+"^{commit}") + output, err := runGitCmdCombinedOutput(ctx, cmd) + return strings.TrimSpace(string(output)), err +} + // FetchBranchResult holds the outcome of a fetchBranchToLocal call. type FetchBranchResult struct { StartPoint string // Ref to use as worktree start point (e.g., "origin/branch"); empty = use local branch @@ -1078,10 +1166,35 @@ func (m *Manager) fetchBranchToLocalWithPolicy( // the manager_git.go probes fixed in PR #1216 (70s lock-held trace, // signal:killed). With this ordering the budget only counts actual // git execution time. - refspec := branch + ":" + branch if prNumber > 0 { - refspec = fmt.Sprintf("pull/%d/head:%s", prNumber, branch) + remoteRef := pullRequestSnapshotRef(prNumber) + refspec := fmt.Sprintf("+refs/pull/%d/head:%s", prNumber, remoteRef) + output, err, fetchCtxErr := m.runGitCombinedAfterAcquire(ctx, m.fetchTimeout, repoPath, "fetch", gitNoTags, "origin", refspec) + if err != nil { + outputStr := string(output) + if isRemoteBranchMissingError(outputStr) || isRemoteRefMissingError(errors.New(outputStr)) { + return nil, fmt.Errorf( + "fetch pull request head %d found no remote ref: %w: %w", + prNumber, ErrWorkspaceCheckoutFailed, newConfirmedRemoteRefMissingError(outputStr), + ) + } + reason := classifyGitFallbackReason(err, outputStr, fetchCtxErr) + return nil, fmt.Errorf( + "fetch pull request head %d failed (%s): %w: %w", + prNumber, reason, ErrWorkspaceCheckoutFailed, syncFailureCause(reason, err, fetchCtxErr), + ) + } + exists, verifyErr := m.branchExists(ctx, repoPath, remoteRef) + if verifyErr != nil { + return nil, fmt.Errorf("verify fetched pull request head %d: %w: %w", prNumber, ErrWorkspaceCheckoutFailed, verifyErr) + } + if !exists { + return nil, fmt.Errorf("verify fetched pull request head %d: %w", prNumber, ErrWorkspaceCheckoutFailed) + } + return &FetchBranchResult{StartPoint: remoteRef}, nil } + + refspec := branch + ":" + branch output, err, fetchCtxErr := m.runGitCombinedAfterAcquire(ctx, m.fetchTimeout, repoPath, "fetch", gitNoTags, "origin", refspec) if err == nil { return &FetchBranchResult{}, nil @@ -1991,10 +2104,11 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe return nil, fmt.Errorf("restore empty-remote worktree branch: %s: %w", strings.TrimSpace(string(output)), branchErr) } } else { - if _, fetchErr := m.fetchBranchToLocalWithPolicy( + fetchResult, fetchErr := m.fetchBranchToLocalWithPolicy( ctx, req.RepositoryPath, existing.Branch, req.PRNumber, req.PullBeforeWorktree && !req.RemoteSyncHandled, - ); fetchErr != nil { + ) + if fetchErr != nil { m.logger.Warn("failed to restore worktree branch during recreate", zap.String("worktree_id", existing.ID), zap.String("branch", existing.Branch), @@ -2011,6 +2125,13 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe } return nil, fetchErr } + if fetchResult != nil && fetchResult.StartPoint != "" { + branchCmd := m.newNonInteractiveGitCmd(ctx, req.RepositoryPath, "branch", existing.Branch, fetchResult.StartPoint) + if output, branchErr := runGitCmdCombinedOutput(ctx, branchCmd); branchErr != nil { + return nil, fmt.Errorf("restore worktree branch from refreshed ref: %s: %w", strings.TrimSpace(string(output)), branchErr) + } + exists = true + } } } if exists && req.RemoteContribution != nil { diff --git a/apps/backend/internal/worktree/manager_pr_head_collision_test.go b/apps/backend/internal/worktree/manager_pr_head_collision_test.go new file mode 100644 index 0000000000..c694bab589 --- /dev/null +++ b/apps/backend/internal/worktree/manager_pr_head_collision_test.go @@ -0,0 +1,74 @@ +package worktree + +import ( + "context" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestCreateWorktreePRHeadCollisionPreservesLocalBranch(t *testing.T) { + const branch = "feature/unit-7-reject-leg-wa-waj" + + repoPath, firstPRHeadSHA := initGitRepoWithPullRef(t, 3294, branch) + runGit(t, repoPath, "fetch", "origin", pullHeadRef(3294)+":"+pullRequestSnapshotRef(3294)) + runGit(t, repoPath, "branch", branch, pullRequestSnapshotRef(3294)) + + originURL := strings.TrimSpace(runGit(t, repoPath, "remote", "get-url", "origin")) + remoteClone := filepath.Join(t.TempDir(), "pr-3408-clone") + runGit(t, filepath.Dir(remoteClone), "clone", originURL, remoteClone) + runGit(t, remoteClone, "config", "user.email", "test@example.com") + runGit(t, remoteClone, "config", "user.name", "Test User") + runGit(t, remoteClone, "config", "commit.gpgsign", "false") + runGit(t, remoteClone, "checkout", "-b", "tmp-pr-head-3408", "main") + writeTestFile(t, filepath.Join(remoteClone, "pr-3408.txt"), "second PR head\n") + runGit(t, remoteClone, "add", "pr-3408.txt") + runGit(t, remoteClone, "commit", "-m", "second PR head") + secondPRHeadSHA := strings.TrimSpace(runGit(t, remoteClone, "rev-parse", "HEAD")) + runGit(t, remoteClone, "push", "origin", "HEAD:"+pullHeadRef(3408)) + runGit(t, remoteClone, "push", "origin", "main:refs/heads/"+branch) + runGit(t, repoPath, "fetch", "origin", "refs/heads/"+branch+":refs/remotes/origin/"+branch) + + collisionBranch := branch + "-" + TaskDirSuffix("task-pr-3408") + runGit(t, repoPath, "branch", collisionBranch, "main") + + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-pr-3408", + SessionID: "session-pr-3408", + TaskTitle: "PR 3408", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "main", + CheckoutBranch: branch, + PRNumber: 3408, + TaskDirName: "task-pr-3408", + RepoName: "repo-1", + }) + if err != nil { + t.Fatalf("Create() should materialize the exact PR head: %v", err) + } + + if wt.Branch != collisionBranch+"-1" { + t.Fatalf("PR worktree should retry a colliding task branch, got %q, want %q", wt.Branch, collisionBranch+"-1") + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", branch)); got != firstPRHeadSHA { + t.Fatalf("local branch SHA = %q, want preserved SHA %q", got, firstPRHeadSHA) + } + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")); got != secondPRHeadSHA { + t.Fatalf("worktree HEAD SHA = %q, want exact PR 3408 head %q", got, secondPRHeadSHA) + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", pullRequestSnapshotRef(3408))); got != secondPRHeadSHA { + t.Fatalf("stored PR ref SHA = %q, want %q", got, secondPRHeadSHA) + } + cmd := exec.Command("git", "rev-parse", "--abbrev-ref", "@{upstream}") + cmd.Dir = wt.Path + if out, err := cmd.CombinedOutput(); err == nil { + t.Fatalf("PR worktree should not track a divergent source branch, got %q", strings.TrimSpace(string(out))) + } +} diff --git a/apps/backend/internal/worktree/manager_pr_refspec_test.go b/apps/backend/internal/worktree/manager_pr_refspec_test.go index 845cdb7080..4a233309df 100644 --- a/apps/backend/internal/worktree/manager_pr_refspec_test.go +++ b/apps/backend/internal/worktree/manager_pr_refspec_test.go @@ -11,10 +11,11 @@ import ( ) // TestFetchBranchToLocal_PRNumberUsesPullRefspec verifies that when a PR -// number is supplied, the manager fetches refs/pull//head into the local -// branch. This is the fork-PR path: the head branch does not exist on origin -// by name (only as a pull//head ref), so the previous branch-name fetch -// would fail with "couldn't find remote ref". +// number is supplied, the manager fetches refs/pull//head into an isolated +// Kandev-owned ref. This is the fork-PR path: the head branch does not +// exist on origin by name (only as a pull//head ref), so the previous +// branch-name fetch would fail with "couldn't find remote ref" or overwrite a +// local branch with the same name. func TestFetchBranchToLocal_PRNumberUsesPullRefspec(t *testing.T) { repoPath, prHeadSHA := initGitRepoWithPullRef(t, 974, "feature/enrich-linear-issue-hap") @@ -36,9 +37,66 @@ func TestFetchBranchToLocal_PRNumberUsesPullRefspec(t *testing.T) { t.Fatalf("expected no warning, got %q", result.Warning) } - gotSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/enrich-linear-issue-hap")) + if result.StartPoint != pullRequestSnapshotRef(974) { + t.Fatalf("start point = %q, want %q", result.StartPoint, pullRequestSnapshotRef(974)) + } + gotSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", pullRequestSnapshotRef(974))) if gotSHA != prHeadSHA { - t.Fatalf("local branch SHA = %q, want %q (PR head)", gotSHA, prHeadSHA) + t.Fatalf("isolated PR ref SHA = %q, want %q (PR head)", gotSHA, prHeadSHA) + } +} + +func TestFetchBranchToLocal_PRNumberSnapshotSurvivesPruneAndOriginPRBranch(t *testing.T) { + const prNumber = 976 + const headBranch = "feature/pr-snapshot" + repoPath, prHeadSHA := initGitRepoWithPullRef(t, prNumber, headBranch) + + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + result, err := mgr.fetchBranchToLocal(context.Background(), repoPath, headBranch, prNumber) + if err != nil { + t.Fatalf("fetchBranchToLocal(pr=%d) unexpected error: %v", prNumber, err) + } + const expectedRef = "refs/kandev/pull/976/head" + if result.StartPoint != expectedRef { + t.Fatalf("start point = %q, want %q", result.StartPoint, expectedRef) + } + + originURL := strings.TrimSpace(runGit(t, repoPath, "remote", "get-url", "origin")) + remoteClone := filepath.Join(t.TempDir(), "origin-pr-branch") + runGit(t, filepath.Dir(remoteClone), "clone", originURL, remoteClone) + runGit(t, remoteClone, "config", "user.email", "remote@example.com") + runGit(t, remoteClone, "config", "user.name", "Remote User") + runGit(t, remoteClone, "config", "commit.gpgsign", "false") + runGit(t, remoteClone, "checkout", "-b", "pr/976", "main") + writeTestFile(t, filepath.Join(remoteClone, "origin-pr-branch.txt"), "origin branch\n") + runGit(t, remoteClone, "add", "origin-pr-branch.txt") + runGit(t, remoteClone, "commit", "-m", "origin pr branch") + originPRBranchSHA := strings.TrimSpace(runGit(t, remoteClone, "rev-parse", "HEAD")) + runGit(t, remoteClone, "push", "origin", "HEAD:refs/heads/pr/976") + + // A routine refresh must not prune or overwrite the Kandev-owned snapshot. + runGit(t, repoPath, "fetch", "--all", "--prune") + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", expectedRef)); got != prHeadSHA { + t.Fatalf("Kandev PR snapshot SHA = %q, want %q", got, prHeadSHA) + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "refs/remotes/origin/pr/976")); got != originPRBranchSHA { + t.Fatalf("origin/pr/976 SHA = %q, want real branch SHA %q", got, originPRBranchSHA) + } + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-pr-snapshot", SessionID: "session-pr-snapshot", TaskTitle: "PR snapshot", + RepositoryID: "repo-1", RepositoryPath: repoPath, BaseBranch: "main", + CheckoutBranch: headBranch, PRNumber: prNumber, RemoteSyncHandled: true, + TaskDirName: "task-pr-snapshot", RepoName: "repo-1", + }) + if err != nil { + t.Fatalf("Create() should use the Kandev snapshot after normal fetch: %v", err) + } + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")); got != prHeadSHA { + t.Fatalf("worktree HEAD = %q, want PR snapshot %q", got, prHeadSHA) } } @@ -109,14 +167,9 @@ func TestCreateWorktree_PRNumberCreatesWorktreeFromForkRef(t *testing.T) { } // TestCreateWorktree_PRNumberSecondWorktreeForSameForkPR exercises the -// retry-path regression flagged in PR #990 review: when a fork PR's head -// branch is already checked out in another worktree, the refspec fetch is -// refused and the manager retries with a bare `pull//head` fetch. That -// retry updates FETCH_HEAD but does NOT create `origin/` because -// the branch doesn't exist on origin — so returning `StartPoint: -// "origin/"` would make the downstream `git worktree add` fail -// with an invalid ref. Empty StartPoint must be returned so the caller -// falls back to the local branch (already at the PR head from task 1). +// PR-head isolation when two worktrees use the same source branch name. Each +// worktree must point at the immutable PR ref while its local branch remains +// unique. func TestCreateWorktree_PRNumberSecondWorktreeForSameForkPR(t *testing.T) { repoPath, prHeadSHA := initGitRepoWithPullRef(t, 974, "feature/fork-pr") diff --git a/apps/backend/internal/worktree/manager_recreate_recovery_test.go b/apps/backend/internal/worktree/manager_recreate_recovery_test.go index a7d7892032..5bffd93832 100644 --- a/apps/backend/internal/worktree/manager_recreate_recovery_test.go +++ b/apps/backend/internal/worktree/manager_recreate_recovery_test.go @@ -455,7 +455,7 @@ func TestRecreate_ForkPRFetchesPullHeadRef(t *testing.T) { func TestRecreate_ManagedRefreshUsesRefreshedPRHeadWithoutNetwork(t *testing.T) { repoPath, wantSHA := initGitRepoWithPullRef(t, 975, "feature/managed-fork-pr") - runGit(t, repoPath, "fetch", "origin", "pull/975/head:refs/remotes/origin/pr/975") + runGit(t, repoPath, "fetch", "origin", "pull/975/head:"+pullRequestSnapshotRef(975)) runGit(t, repoPath, "remote", "set-url", "origin", "https://127.0.0.1:1/never.git") worktreePath := filepath.Join(t.TempDir(), "task-managed-pr", "repo-1") if err := os.MkdirAll(worktreePath, 0755); err != nil { diff --git a/apps/web/components/task/chat/chat-input-area.tsx b/apps/web/components/task/chat/chat-input-area.tsx index e78b6ec511..0a77bebeb0 100644 --- a/apps/web/components/task/chat/chat-input-area.tsx +++ b/apps/web/components/task/chat/chat-input-area.tsx @@ -312,6 +312,8 @@ type ChatInputAreaProps = { onRequestChangesTooltipDismiss?: () => void; panelState: ChatPanelState; isSending: boolean; + /** The task-owned launch card renders recovery for the failed start. */ + launchErrorOwned?: boolean; hideSessionsDropdown?: boolean; minimalToolbar?: boolean; /** Hide ACP/session-specific controls (model picker, mode, MCP, reset context, @@ -428,6 +430,7 @@ export function ChatInputArea({ onScrollToStart, statusTaskId = null, showAgentStartHint = false, + launchErrorOwned = false, }: ChatInputAreaProps) { const { resolvedSessionId, taskId, isAgentBusy } = panelState; const statusRowTaskId = resolveStatusRowTaskId(taskId, statusTaskId); @@ -456,6 +459,7 @@ export function ChatInputArea({ minimalToolbar, hideAgentControls, hidePlanMode, + launchErrorOwned, }); return (
({ + containerState: { + showNewSessionDialog: false, + setShowNewSessionDialog: vi.fn(), + contextPopoverOpen: false, + setContextPopoverOpen: vi.fn(), + height: "auto", + containerRef: { current: null }, + resizeHandleProps: { onMouseDown: vi.fn(), onDoubleClick: vi.fn() }, + value: "", + inputRef: { current: null }, + addFiles: vi.fn().mockResolvedValue(undefined), + fileInputRef: { current: null }, + handleChange: vi.fn(), + handleSubmitWithReset: vi.fn(), + allItems: [], + hasPendingAttachmentUploads: false, + isInputFocused: false, + setIsInputFocused: vi.fn(), + hasClarification: false, + hasPendingComments: false, + hasContextZone: false, + showFocusHint: false, + inputPlaceholder: "Ask to make changes", + isDisabled: false, + submitDisabled: false, + submitDisabledReason: undefined, + }, +})); + +vi.mock("./use-chat-input-container", () => ({ + useChatInputContainer: () => containerState, +})); + +vi.mock("./session-stopped-banner", () => ({ + SessionStoppedBanner: () =>
, +})); + +vi.mock("./chat-input-body", () => ({ + ChatInputBody: () =>
, +})); + +vi.mock("@/hooks/domains/session/use-session-recovery-actions", () => ({ + useSessionRecoveryActions: () => ({}), +})); + +vi.mock("@/hooks/use-is-utility-configured", () => ({ + useIsUtilityConfigured: () => false, +})); + +vi.mock("@/hooks/use-utility-agent-generator", () => ({ + useUtilityAgentGenerator: () => ({ + enhancePrompt: vi.fn(), + isEnhancingPrompt: false, + }), +})); + +vi.mock("@/hooks/use-prompt-result-delivery", () => ({ + usePromptResultDelivery: () => ({ + pendingResult: null, + captureScope: vi.fn(), + deliver: vi.fn(), + applyPending: vi.fn(), + copyPending: vi.fn(), + }), +})); + +vi.mock("@/lib/i18n", () => ({ + t: (key: string) => key, +})); + +const baseProps = { + onSubmit: vi.fn(), + sessionId: "session-1", + taskId: "task-1", + taskDescription: "", + planModeEnabled: false, + onPlanModeChange: vi.fn(), + isAgentBusy: false, + isStarting: false, + isSending: false, + onCancel: vi.fn(), +}; + +afterEach(() => cleanup()); + +describe("ChatInputContainer launch-error ownership", () => { + it("hides the editor when the task launch card owns a failed session", () => { + render(); + + expect(screen.queryByTestId("chat-input-body")).toBeNull(); + expect(screen.queryByTestId("session-stopped-banner")).toBeNull(); + }); + + it("renders the stopped banner when the failed session has no launch-card owner", () => { + render(); + + expect(screen.queryByTestId("chat-input-body")).toBeNull(); + expect(screen.getByTestId("session-stopped-banner")).toBeTruthy(); + }); + + it("keeps the editor visible when the owned launch error is not a failed session", () => { + render(); + + expect(screen.getByTestId("chat-input-body")).toBeTruthy(); + expect(screen.queryByTestId("session-stopped-banner")).toBeNull(); + }); +}); diff --git a/apps/web/components/task/chat/chat-input-container.tsx b/apps/web/components/task/chat/chat-input-container.tsx index 52a612870b..f047055bde 100644 --- a/apps/web/components/task/chat/chat-input-container.tsx +++ b/apps/web/components/task/chat/chat-input-container.tsx @@ -21,6 +21,7 @@ import { useIsUtilityConfigured } from "@/hooks/use-is-utility-configured"; import { usePromptResultDelivery } from "@/hooks/use-prompt-result-delivery"; import { PromptResultRecovery } from "@/components/prompt-result-recovery"; import { t } from "@/lib/i18n"; +import { shouldHideChatInputForLaunchError, shouldRenderStoppedSessionBanner } from "./types"; // Re-export ImageAttachment type for consumers export type { ImageAttachment } from "./image-attachment-preview"; @@ -96,6 +97,8 @@ type ChatInputContainerProps = { isCompleted?: boolean; sessionErrorMessage?: string; needsRecovery?: boolean; + /** The task-owned launch card renders the failed-start recovery. */ + launchErrorOwned?: boolean; executorUnavailable?: boolean; executorUnavailableReason?: string; contextItems?: ContextItem[]; @@ -287,6 +290,13 @@ function useChatPromptEnhancement({ return { handleEnhancePrompt, isEnhancingPrompt, isUtilityConfigured, promptDelivery }; } +function useChatInputRecoveryActions(taskId: string | null, sessionId: string | null) { + return useSessionRecoveryActions({ + taskId: taskId ?? "", + sessionId: sessionId ?? "", + }); +} + export const ChatInputContainer = forwardRef( // eslint-disable-next-line complexity -- top-level component chooses the stopped or editor surface after shared hook setup. function ChatInputContainer(props, ref) { @@ -322,10 +332,7 @@ export const ChatInputContainer = forwardRef ({ import { MessageListFooter } from "./message-list-footer"; const AGENT_STATUS_TEST_ID = "agent-status"; +const ACTION_MESSAGE_TEST_ID = "action-message"; const SESSION_ID = "session-1"; +const FAILED_SESSION_STATE = "FAILED" as const; afterEach(cleanup); @@ -45,23 +47,30 @@ const laterFailure = { }, } satisfies Message; +// eslint-disable-next-line max-lines-per-function -- footer ownership cases share one focused fixture. describe("MessageListFooter", () => { it("lets an actionable footer failure own the failure presentation", () => { render( , ); - expect(screen.getByTestId("action-message")).toBeTruthy(); + expect(screen.getByTestId(ACTION_MESSAGE_TEST_ID)).toBeTruthy(); expect(screen.queryByTestId(AGENT_STATUS_TEST_ID)).toBeNull(); }); it("keeps the generic status for a failed session without an actionable footer", () => { - render(); + render( + , + ); expect(screen.getByTestId(AGENT_STATUS_TEST_ID)).toBeTruthy(); }); @@ -81,13 +90,17 @@ describe("MessageListFooter", () => { it("switches ownership when missing-branch recovery arrives after failure", () => { const { rerender } = render( - , + , ); expect(screen.getByTestId(AGENT_STATUS_TEST_ID)).toBeTruthy(); rerender( { ); expect(screen.queryByTestId(AGENT_STATUS_TEST_ID)).toBeNull(); - expect(screen.getByTestId("action-message")).toBeTruthy(); + expect(screen.getByTestId(ACTION_MESSAGE_TEST_ID)).toBeTruthy(); }); it("does not restore stale missing-branch recovery after a later failure", () => { render( { ); expect(screen.getByTestId(AGENT_STATUS_TEST_ID)).toBeTruthy(); - expect(screen.queryByTestId("action-message")).toBeNull(); + expect(screen.queryByTestId(ACTION_MESSAGE_TEST_ID)).toBeNull(); + }); + + it("hides launch failure footer surfaces when the task card owns the error", () => { + render( + , + ); + + expect(screen.queryByTestId(AGENT_STATUS_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(ACTION_MESSAGE_TEST_ID)).toBeNull(); + }); + + it("keeps stale launch failure footer history when the current card owns the error", () => { + const staleFailure = { + ...actionableFailure, + id: "failure-stale", + content: "Stale branch recovery", + metadata: { + ...actionableFailure.metadata, + launch_error_stamp: "old-stamp", + }, + } satisfies Message; + const currentFailure = { + ...actionableFailure, + id: "failure-current", + content: "Current branch recovery", + metadata: { + ...actionableFailure.metadata, + launch_error_stamp: "current-stamp", + }, + } satisfies Message; + + render( + , + ); + + expect(screen.getByText("Stale branch recovery")).toBeTruthy(); + expect(screen.queryByText("Current branch recovery")).toBeNull(); }); }); diff --git a/apps/web/components/task/chat/message-list-footer.tsx b/apps/web/components/task/chat/message-list-footer.tsx index 6cd8dcffce..d0a424d312 100644 --- a/apps/web/components/task/chat/message-list-footer.tsx +++ b/apps/web/components/task/chat/message-list-footer.tsx @@ -3,6 +3,7 @@ import type { Message, TaskSessionState } from "@/lib/types/http"; import { AgentStatus } from "@/components/task/chat/messages/agent-status"; import { MessageRenderer } from "@/components/task/chat/message-renderer"; +import { filterLaunchErrorMessages } from "./message-list-shared"; type MessageListFooterProps = { sessionState?: TaskSessionState; @@ -10,6 +11,12 @@ type MessageListFooterProps = { messages: Message[]; isWorking?: boolean; footerActionMessages?: Message[]; + /** The task-owned launch card is rendering the current failure. */ + launchErrorOwned?: boolean; + /** Stamp used to keep historical launch-only footer rows visible. */ + launchErrorStamp?: string; + /** Timestamp used to identify unstamped synthetic rows from the active failure. */ + launchErrorOccurredAt?: string; }; function isMissingBranchFailure(message: Message): boolean { @@ -45,18 +52,25 @@ export function MessageListFooter({ messages, isWorking, footerActionMessages = [], + launchErrorOwned = false, + launchErrorStamp, + launchErrorOccurredAt, }: MessageListFooterProps) { const currentActionableFailure = findCurrentActionableFailure(messages, footerActionMessages); const recoveryOwnsFailure = + !launchErrorOwned && sessionState === "FAILED" && currentActionableFailure !== undefined && isMissingBranchFailure(currentActionableFailure); - const visibleFooterActionMessages = footerActionMessages.filter( - (message) => !isMissingBranchFailure(message) || message.id === currentActionableFailure?.id, - ); + const visibleFooterActionMessages = launchErrorOwned + ? filterLaunchErrorMessages(footerActionMessages, true, launchErrorStamp, launchErrorOccurredAt) + : footerActionMessages.filter( + (message) => + !isMissingBranchFailure(message) || message.id === currentActionableFailure?.id, + ); return ( <> - {!recoveryOwnsFailure && ( + {!recoveryOwnsFailure && !(launchErrorOwned && sessionState === "FAILED") && ( void; autoScrollEnabled: boolean; dividerBeforeItemKey?: string | null; + launchErrorOwned: boolean; + launchErrorStamp?: string; + launchErrorOccurredAt?: string; }; /** @@ -474,6 +479,9 @@ function NativeMessageListBody({ onScrollToMessage, autoScrollEnabled, dividerBeforeItemKey, + launchErrorOwned, + launchErrorStamp, + launchErrorOccurredAt, }: NativeMessageListBodyProps) { return (
@@ -515,6 +523,9 @@ function NativeMessageListBody({ messages={messages} isWorking={isWorking} footerActionMessages={footerActionMessages} + launchErrorOwned={launchErrorOwned} + launchErrorStamp={launchErrorStamp} + launchErrorOccurredAt={launchErrorOccurredAt} /> {/* Bottom anchor keeps the view pinned while auto-scroll is enabled. @@ -558,14 +569,43 @@ export const NativeMessageList = memo( dividerBeforeItemKey, anchoredBarHeight, isVisible = true, + launchErrorOwned = false, + launchErrorStamp, + launchErrorOccurredAt, }: MessageListProps, ref, ) { const scrollRef = useRef(null); + const visibleItems = useMemo( + () => + filterLaunchErrorItems(items, launchErrorOwned, launchErrorStamp, launchErrorOccurredAt), + [items, launchErrorOwned, launchErrorStamp, launchErrorOccurredAt], + ); + const visibleMessages = useMemo( + () => + filterLaunchErrorMessages( + messages, + launchErrorOwned, + launchErrorStamp, + launchErrorOccurredAt, + ), + [messages, launchErrorOwned, launchErrorStamp, launchErrorOccurredAt], + ); + const visibleFooterActionMessages = useMemo( + () => + filterLaunchErrorMessages( + footerActionMessages ?? [], + launchErrorOwned, + launchErrorStamp, + launchErrorOccurredAt, + ), + [footerActionMessages, launchErrorOwned, launchErrorStamp, launchErrorOccurredAt], + ); + const { isInitialLoading, showLoadingState } = getConversationLoadingState({ messagesLoading, - messagesCount: messages.length, + messagesCount: visibleMessages.length, isWorking, sessionState, }); @@ -574,15 +614,15 @@ export const NativeMessageList = memo( }); const { activeTurnId } = useSessionTurn(sessionId); const effectiveActiveTurnId = getEffectiveActiveTurnId(activeTurnId, isWorking); - const streamingMessageId = getStreamingAgentMessageId(messages); - const lastTurnGroupId = useMemo(() => getLastTurnGroupId(items), [items]); + const streamingMessageId = getStreamingAgentMessageId(visibleMessages); + const lastTurnGroupId = useMemo(() => getLastTurnGroupId(visibleItems), [visibleItems]); const autoScrollEnabled = useTranscriptAutoScrollEnabled(sessionId); const { handleScrollToMessage, sentinelRef, retryLoadMore, showRecovery } = useNativeMessageListScroll({ scrollRef, ref, - items, - messages, + items: visibleItems, + messages: visibleMessages, isWorking, sessionId, enabled: autoScrollEnabled, @@ -617,9 +657,9 @@ export const NativeMessageList = memo( > {stickyPromptBar} ); diff --git a/apps/web/components/task/chat/message-list-shared.test.tsx b/apps/web/components/task/chat/message-list-shared.test.tsx index f2b0badb37..3fc4ddb76d 100644 --- a/apps/web/components/task/chat/message-list-shared.test.tsx +++ b/apps/web/components/task/chat/message-list-shared.test.tsx @@ -93,6 +93,8 @@ import { getLastUserMessageId, getFirstUserMessageId, isElementFullyVisible, + filterLaunchErrorItems, + filterLaunchErrorMessages, resolveLastPromptControls, resolveLastPromptEdge, shouldAutoScrollToBottom, @@ -131,6 +133,106 @@ describe("getEffectiveActiveTurnId", () => { }); }); +describe("launch error-owned transcript filtering", () => { + it("removes launch-only messages only when the task card owns the error", () => { + const emptyTurn = { + id: "empty-turn-1", + metadata: { empty_turn: true }, + } as unknown as Message; + const runtimeFailure = { + id: "runtime-failure-1", + metadata: { failure_kind: "provider_quota_limited" }, + } as unknown as Message; + + expect(filterLaunchErrorMessages([emptyTurn, runtimeFailure], false)).toEqual([ + emptyTurn, + runtimeFailure, + ]); + expect(filterLaunchErrorMessages([emptyTurn, runtimeFailure], true)).toEqual([runtimeFailure]); + }); + + it("removes preparation and previous-agent rows while retaining transcript messages", () => { + const runtimeMessage = { id: "message-1" } as unknown as Message; + const items: RenderItem[] = [ + { type: "prepare_progress", id: "prepare-1", sessionId: "s1" }, + { + type: "agent_error_notice", + id: "agent-error-1", + sessionId: "s1", + error: { message: "matching error", stamp: "stamp-1" }, + }, + { + type: "agent_error_notice", + id: "agent-error-2", + sessionId: "s1", + error: { message: "unrelated error", stamp: "old-stamp" }, + }, + { + type: "message", + message: { + id: "empty-1", + created_at: "2026-06-14T12:00:01Z", + metadata: { empty_turn: true }, + } as unknown as Message, + }, + { type: "message", message: runtimeMessage }, + ]; + + expect(filterLaunchErrorItems(items, true, "stamp-1", "2026-06-14T12:00:00Z")).toEqual([ + { + type: "agent_error_notice", + id: "agent-error-2", + sessionId: "s1", + error: { message: "unrelated error", stamp: "old-stamp" }, + }, + { type: "message", message: runtimeMessage }, + ]); + }); + + it("keeps stale launch-only history when the current failure has no carried stamp", () => { + const staleEmptyTurn = { + id: "empty-stale", + created_at: "2026-07-21T00:00:00Z", + metadata: { empty_turn: true }, + } as unknown as Message; + const currentEmptyTurn = { + id: "empty-current", + created_at: "2026-07-22T00:00:00Z", + metadata: { empty_turn: true }, + } as unknown as Message; + + expect(filterLaunchErrorMessages([staleEmptyTurn, currentEmptyTurn], true)).toEqual([ + staleEmptyTurn, + ]); + expect( + filterLaunchErrorItems( + [ + { type: "message", message: staleEmptyTurn }, + { type: "message", message: currentEmptyTurn }, + ], + true, + "current-stamp", + "2026-07-22T00:00:00Z", + ), + ).toEqual([{ type: "message", message: staleEmptyTurn }]); + }); + + it("filters only launch-only rows carrying the active failure stamp", () => { + const staleMissingBranch = { + id: "missing-stale", + metadata: { failure_kind: "missing_pr_branch", launch_error_stamp: "old-stamp" }, + } as unknown as Message; + const currentMissingBranch = { + id: "missing-current", + metadata: { failure_kind: "missing_pr_branch", launch_error_stamp: "current-stamp" }, + } as unknown as Message; + + expect( + filterLaunchErrorMessages([staleMissingBranch, currentMissingBranch], true, "current-stamp"), + ).toEqual([staleMissingBranch]); + }); +}); + describe("canReassertDividerScroll", () => { it("never applies without a divider target", () => { expect( diff --git a/apps/web/components/task/chat/message-list-shared.tsx b/apps/web/components/task/chat/message-list-shared.tsx index 05a1c06a8c..8b18e206af 100644 --- a/apps/web/components/task/chat/message-list-shared.tsx +++ b/apps/web/components/task/chat/message-list-shared.tsx @@ -17,6 +17,7 @@ import { lastAgentErrorStamp, readLastAgentError, } from "@/lib/session-last-agent-error"; +import { isLaunchErrorSurfaceMessage } from "./types"; import { useTranslation } from "react-i18next"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; @@ -67,6 +68,12 @@ export type MessageListProps = { * remain mounted while inactive and use this transition to recover missed * oldest-page sentinel observations. */ isVisible?: boolean; + /** The task-owned launch card renders the matching failure. */ + launchErrorOwned?: boolean; + /** Stamp used to distinguish a matching last-agent-error notice. */ + launchErrorStamp?: string; + /** Timestamp used to identify unstamped synthetic rows from the active failure. */ + launchErrorOccurredAt?: string; }; /** Imperative handle exposed by `MessageList`, letting the chat panel scroll @@ -91,6 +98,86 @@ export function getItemKey(item: RenderItem): string { return item.message.id; } +const LAUNCH_ERROR_STAMP_KEYS = ["launch_error_stamp", "error_stamp"] as const; + +function launchErrorSurfaceStamp(message: Message): string | undefined { + const metadata = message.metadata as Record | undefined; + for (const key of LAUNCH_ERROR_STAMP_KEYS) { + const value = metadata?.[key]; + if (typeof value === "string" && value.length > 0) return value; + } + return undefined; +} + +function launchErrorSurfaceMessageIds( + messages: Message[], + launchErrorStamp?: string, + launchErrorOccurredAt?: string, +): Set { + const launchMessages = messages.filter(isLaunchErrorSurfaceMessage); + if (launchMessages.length === 0) return new Set(); + + const unstampedMessages = launchMessages.filter( + (message) => launchErrorSurfaceStamp(message) === undefined, + ); + const hiddenIds = new Set(); + if (launchErrorStamp) { + const stampedMatches = launchMessages.filter( + (message) => launchErrorSurfaceStamp(message) === launchErrorStamp, + ); + stampedMatches.forEach((message) => hiddenIds.add(message.id)); + } + + const occurredAt = launchErrorOccurredAt ? Date.parse(launchErrorOccurredAt) : Number.NaN; + if (!Number.isNaN(occurredAt)) { + unstampedMessages.forEach((message) => { + const createdAt = Date.parse(message.created_at); + if (!Number.isNaN(createdAt) && createdAt >= occurredAt) hiddenIds.add(message.id); + }); + } + + if (hiddenIds.size > 0 || launchErrorStamp || launchErrorOccurredAt) return hiddenIds; + + // Older synthetic rows do not carry the launch stamp. Keep historical rows + // intact and hide only the most recent row when no identity is available. + const fallbackMessage = launchMessages[launchMessages.length - 1]; + return fallbackMessage ? new Set([fallbackMessage.id]) : new Set(); +} + +/** Remove the launch-only transcript decorations once the task card owns them. */ +export function filterLaunchErrorMessages( + messages: Message[], + launchErrorOwned: boolean, + launchErrorStamp?: string, + launchErrorOccurredAt?: string, +): Message[] { + if (!launchErrorOwned) return messages; + const hiddenIds = launchErrorSurfaceMessageIds(messages, launchErrorStamp, launchErrorOccurredAt); + return messages.filter((message) => !hiddenIds.has(message.id)); +} + +/** Remove synthetic launch rows while preserving ordinary transcript activity. */ +export function filterLaunchErrorItems( + items: RenderItem[], + launchErrorOwned: boolean, + launchErrorStamp?: string, + launchErrorOccurredAt?: string, +): RenderItem[] { + if (!launchErrorOwned) return items; + const hiddenMessageIds = launchErrorSurfaceMessageIds( + items.flatMap((item) => (item.type === "message" ? [item.message] : [])), + launchErrorStamp, + launchErrorOccurredAt, + ); + return items.filter((item) => { + if (item.type === "prepare_progress") return false; + if (item.type === "agent_error_notice") { + return !launchErrorStamp || lastAgentErrorStamp(item.error) !== launchErrorStamp; + } + return item.type !== "message" || !hiddenMessageIds.has(item.message.id); + }); +} + /** The active turn id, but only while the agent is working — turns no longer * in progress aren't treated as active. */ export function getEffectiveActiveTurnId( diff --git a/apps/web/components/task/chat/stopped-banner-props.test.ts b/apps/web/components/task/chat/stopped-banner-props.test.ts index 06dba7a166..ca44b317d3 100644 --- a/apps/web/components/task/chat/stopped-banner-props.test.ts +++ b/apps/web/components/task/chat/stopped-banner-props.test.ts @@ -1,6 +1,7 @@ import { renderHook } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import { buildStoppedBannerProps } from "./chat-input-container"; +import { shouldHideChatInputForLaunchError, shouldRenderStoppedSessionBanner } from "./types"; import { useComposerProps } from "./use-composer-props"; function composerArgs(errorMessage: string) { @@ -51,6 +52,20 @@ function composerArgs(errorMessage: string) { } describe("stopped-session error propagation", () => { + it("gives the task launch card ownership of a failed session", () => { + expect( + shouldRenderStoppedSessionBanner({ + isFailed: true, + isCompleted: false, + executorUnavailable: false, + launchErrorOwned: true, + }), + ).toBe(false); + expect(shouldHideChatInputForLaunchError({ isFailed: true, launchErrorOwned: true })).toBe( + true, + ); + }); + it("threads the sanitized session error from panel state into composer props", () => { const errorMessage = "pods is forbidden: RBAC denied the request"; const { result } = renderHook(() => useComposerProps(composerArgs(errorMessage))); diff --git a/apps/web/components/task/chat/types.test.ts b/apps/web/components/task/chat/types.test.ts index c9769da237..284e4e103b 100644 --- a/apps/web/components/task/chat/types.test.ts +++ b/apps/web/components/task/chat/types.test.ts @@ -1,6 +1,13 @@ import { describe, expect, it } from "vitest"; import { sessionId as toSessionId, taskId as toTaskId, type Message } from "@/lib/types/http"; -import { isRichOutputMessage, kandevToolStemOf, type ToolCallMetadata } from "./types"; +import { + isLaunchErrorSurfaceMessage, + isMatchingTaskLaunchError, + isTaskLaunchErrorOwnedBySession, + isRichOutputMessage, + kandevToolStemOf, + type ToolCallMetadata, +} from "./types"; function toolCall(rawName: string, field: "tool_name" | "title" | "content" = "title"): Message { const metadata: Record = { @@ -65,3 +72,65 @@ describe("isRichOutputMessage", () => { ); }); }); + +describe("task launch error presentation predicates", () => { + it("matches a summary error only by session and stamp", () => { + const activeError = { session_id: "session-1", stamp: "stamp-1" }; + + expect( + isMatchingTaskLaunchError(activeError, { + sessionId: "session-1", + errorStamp: "stamp-1", + }), + ).toBe(true); + expect( + isMatchingTaskLaunchError(activeError, { + sessionId: "session-2", + errorStamp: "stamp-1", + }), + ).toBe(false); + expect( + isMatchingTaskLaunchError(activeError, { + sessionId: "session-1", + errorStamp: "old-stamp", + }), + ).toBe(false); + }); + + it("matches task-level errors only when the candidate has no session", () => { + const activeError = { stamp: "stamp-1" }; + + expect(isMatchingTaskLaunchError(activeError, { errorStamp: "stamp-1" })).toBe(true); + expect( + isMatchingTaskLaunchError(activeError, { + sessionId: "session-1", + errorStamp: "stamp-1", + }), + ).toBe(false); + }); + + it("keeps a task-wide card visible without giving it ownership of a prior session", () => { + const activeError = { stamp: "stamp-1" }; + + expect(isTaskLaunchErrorOwnedBySession(activeError, null)).toBe(true); + expect(isTaskLaunchErrorOwnedBySession(activeError, undefined)).toBe(true); + expect(isTaskLaunchErrorOwnedBySession(activeError, "prior-session")).toBe(false); + }); + + it("gives a session-owned error ownership only of its exact session", () => { + const activeError = { session_id: "session-1", stamp: "stamp-1" }; + + expect(isTaskLaunchErrorOwnedBySession(activeError, "session-1")).toBe(true); + expect(isTaskLaunchErrorOwnedBySession(activeError, null)).toBe(false); + expect(isTaskLaunchErrorOwnedBySession(activeError, "session-2")).toBe(false); + }); + + it.each([ + { metadata: { empty_turn: true }, expected: true }, + { metadata: { failure_kind: "missing_pr_branch" }, expected: true }, + { metadata: { failure_kind: "provider_quota_limited" }, expected: false }, + { metadata: undefined, expected: false }, + ])("classifies launch-only message noise", ({ metadata, expected }) => { + expect(isLaunchErrorSurfaceMessage({ id: "message-1", metadata } as never)).toBe(expected); + }); +}); diff --git a/apps/web/components/task/chat/types.ts b/apps/web/components/task/chat/types.ts index 45fd69ddb9..d23bdfb682 100644 --- a/apps/web/components/task/chat/types.ts +++ b/apps/web/components/task/chat/types.ts @@ -1,6 +1,7 @@ "use client"; import type { Message } from "@/lib/types/http"; +import type { TaskStatusSummaryActiveError } from "@/lib/types/task-status-summary"; import { extractKandevStem } from "./messages/kandev/parse"; export type SubagentTaskPayload = { @@ -187,6 +188,64 @@ export function isSubagentMessage(message: Message): boolean { return metadata?.normalized?.kind === "subagent_task"; } +export type TaskLaunchErrorIdentity = Pick; + +export function shouldRenderStoppedSessionBanner(input: { + isFailed: boolean; + isCompleted: boolean; + executorUnavailable: boolean; + launchErrorOwned?: boolean; +}): boolean { + return ( + !input.launchErrorOwned && (input.isFailed || input.isCompleted || input.executorUnavailable) + ); +} + +export function shouldHideChatInputForLaunchError(input: { + isFailed: boolean; + launchErrorOwned?: boolean; +}): boolean { + return input.launchErrorOwned === true && input.isFailed; +} + +/** Matches one rendered error surface to the task-owned launch error. */ +export function isMatchingTaskLaunchError( + activeError: TaskLaunchErrorIdentity | null | undefined, + candidate: { sessionId?: string | null; errorStamp?: string | null }, +): boolean { + if (!activeError?.stamp || !candidate.errorStamp) return false; + return ( + (activeError.session_id ?? null) === (candidate.sessionId ?? null) && + activeError.stamp === candidate.errorStamp + ); +} + +/** A task-wide launch error stays visible while any prior session is selected. */ +export function isTaskLaunchErrorVisibleForSession( + activeError: TaskLaunchErrorIdentity | null | undefined, + sessionId: string | null | undefined, +): boolean { + if (!activeError?.stamp) return false; + if (sessionId === undefined || activeError.session_id == null) return true; + return activeError.session_id === sessionId; +} + +/** Whether the active launch error owns the selected session's surfaces. */ +export function isTaskLaunchErrorOwnedBySession( + activeError: TaskLaunchErrorIdentity | null | undefined, + sessionId: string | null | undefined, +): boolean { + if (!activeError?.stamp) return false; + if (activeError.session_id == null) return sessionId == null; + return sessionId != null && activeError.session_id === sessionId; +} + +/** Messages that are only useful while a launch failure has no typed owner. */ +export function isLaunchErrorSurfaceMessage(message: Message): boolean { + const metadata = message.metadata as Record | undefined; + return metadata?.empty_turn === true || metadata?.failure_kind === "missing_pr_branch"; +} + export type StatusMetadata = { progress?: number; status?: string; diff --git a/apps/web/components/task/chat/use-composer-props.ts b/apps/web/components/task/chat/use-composer-props.ts index 27f28e0366..3d1918ee81 100644 --- a/apps/web/components/task/chat/use-composer-props.ts +++ b/apps/web/components/task/chat/use-composer-props.ts @@ -16,6 +16,7 @@ type ComposerPropsArgs = { handleSubmit: (payload: ChatSubmitPayload) => ChatSubmitResult; handleCancelTurn: () => Promise; isSending: boolean; + launchErrorOwned?: boolean; showRequestChangesTooltip: boolean; onRequestChangesTooltipDismiss?: () => void; onClarificationResolved: () => void; @@ -41,6 +42,7 @@ export function useComposerProps(args: ComposerPropsArgs) { handleSubmit, handleCancelTurn, isSending, + launchErrorOwned, showRequestChangesTooltip, onRequestChangesTooltipDismiss, onClarificationResolved, @@ -75,6 +77,7 @@ export function useComposerProps(args: ComposerPropsArgs) { isPreparingEnvironment: panelState.isPreparingEnvironment, isMoving, isSending, + launchErrorOwned, onCancel: handleCancelTurn, placeholder, pendingClarification: panelState.pendingClarification, diff --git a/apps/web/components/task/simple/chat-entries.test.ts b/apps/web/components/task/simple/chat-entries.test.ts index f2d8b944a4..70eb1f5db1 100644 --- a/apps/web/components/task/simple/chat-entries.test.ts +++ b/apps/web/components/task/simple/chat-entries.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { buildRunErrorsFromSessions, + filterVisibleRunErrors, hasMatchingSessionLaunchError, liveSessionMetadataFromStore, mergeLiveSessionMetadata, @@ -137,6 +138,32 @@ describe("hasMatchingSessionLaunchError", () => { }); }); +describe("filterVisibleRunErrors", () => { + it("keeps ordinary provider errors when the active summary is not a launch error", () => { + const errors = buildRunErrorsFromSessions([ + session({ + metadata: { + last_agent_error: { + message: "The provider rejected the request.", + code: "provider_auth_required", + stamp: "provider-stamp-1", + }, + }, + }), + ]); + + expect( + filterVisibleRunErrors(errors, { + session_id: "s1", + stamp: "provider-stamp-1", + occurred_at: "2026-08-02T15:00:01Z", + preview: "The provider rejected the request.", + category: "provider_auth_required", + }), + ).toEqual(errors); + }); +}); + describe("liveSessionMetadataFromStore", () => { const metadata = { last_agent_error: { message: "boom", remediation_url: URL } }; diff --git a/apps/web/components/task/simple/chat-entries.ts b/apps/web/components/task/simple/chat-entries.ts index d4eea92b2b..f9889a8a1a 100644 --- a/apps/web/components/task/simple/chat-entries.ts +++ b/apps/web/components/task/simple/chat-entries.ts @@ -2,6 +2,8 @@ import type { CommentTurnContext } from "./turn-context"; import { groupSortKey, type SessionGroup } from "./session-groups"; import { normalizeRemediationUrl } from "@/lib/remediation-url"; import { lastAgentErrorStamp, readLastAgentError } from "@/lib/session-last-agent-error"; +import { isTypedTaskLaunchError } from "./components/task-launch-error-entry"; +import { isMatchingTaskLaunchError } from "@/components/task/chat/types"; import type { RunError, TaskComment, @@ -9,6 +11,7 @@ import type { TaskSession, TimelineEvent, } from "@/app/office/tasks/[id]/types"; +import type { TaskStatusSummary } from "@/lib/types/task-status-summary"; /** * Discriminated union of every row that can appear in the task chat @@ -85,9 +88,25 @@ export function hasMatchingSessionLaunchError( runErrors: RunError[], ): boolean { if (!summarySessionId) return false; - return runErrors.some( + return runErrors.some((error) => + isMatchingTaskLaunchError( + { session_id: summarySessionId, stamp: summaryStamp ?? "" }, + { sessionId: error.sessionId, errorStamp: error.errorStamp }, + ), + ); +} + +export function filterVisibleRunErrors( + runErrors: RunError[], + activeError: TaskStatusSummary["active_error"], +): RunError[] { + if (!isTypedTaskLaunchError(activeError)) return runErrors; + return runErrors.filter( (error) => - error.sessionId === summarySessionId && (!summaryStamp || error.errorStamp === summaryStamp), + !isMatchingTaskLaunchError(activeError, { + sessionId: error.sessionId, + errorStamp: error.errorStamp, + }), ); } diff --git a/apps/web/components/task/simple/components/run-error-entry.tsx b/apps/web/components/task/simple/components/run-error-entry.tsx index e51b0019c4..181dfe48bb 100644 --- a/apps/web/components/task/simple/components/run-error-entry.tsx +++ b/apps/web/components/task/simple/components/run-error-entry.tsx @@ -54,6 +54,7 @@ function TypedRunLaunchErrorEntry({ stamp: error.errorStamp ?? "", occurred_at: error.failedAt, preview, + details: error.failureDetails, category: error.failureCode, recovery_actions: error.recoveryActions, }} diff --git a/apps/web/components/task/simple/components/task-chat-launch-error.tsx b/apps/web/components/task/simple/components/task-chat-launch-error.tsx index 3dfe5251aa..8bed9e6ba9 100644 --- a/apps/web/components/task/simple/components/task-chat-launch-error.tsx +++ b/apps/web/components/task/simple/components/task-chat-launch-error.tsx @@ -1,17 +1,16 @@ "use client"; -import { useMemo } from "react"; import type { TaskRepository } from "@/lib/types/http"; import type { TaskStatusSummary } from "@/lib/types/task-status-summary"; -import type { RunError } from "@/app/office/tasks/[id]/types"; -import { hasMatchingSessionLaunchError } from "../chat-entries"; +import { isTaskLaunchErrorVisibleForSession } from "@/components/task/chat/types"; import { isTypedTaskLaunchError, TaskLaunchErrorEntry } from "./task-launch-error-entry"; type TaskChatLaunchErrorProps = { taskId: string; workspaceId: string; statusSummary?: TaskStatusSummary | null; - runErrors: RunError[]; + /** When supplied, only render the error that belongs to this session. */ + sessionId?: string | null; repositories?: TaskRepository[]; }; @@ -19,17 +18,14 @@ export function TaskChatLaunchError({ taskId, workspaceId, statusSummary, - runErrors, + sessionId, repositories, }: TaskChatLaunchErrorProps) { - const error = useMemo(() => { - const candidate = statusSummary?.active_error; - if (!isTypedTaskLaunchError(candidate)) return null; - if (hasMatchingSessionLaunchError(candidate.session_id, candidate.stamp, runErrors)) { - return null; - } - return candidate; - }, [runErrors, statusSummary]); + const candidate = statusSummary?.active_error; + const error = + isTypedTaskLaunchError(candidate) && isTaskLaunchErrorVisibleForSession(candidate, sessionId) + ? candidate + : null; if (!error) return null; return ( diff --git a/apps/web/components/task/simple/components/task-launch-error-entry.test.tsx b/apps/web/components/task/simple/components/task-launch-error-entry.test.tsx index 860c09f7db..e0e358ad0f 100644 --- a/apps/web/components/task/simple/components/task-launch-error-entry.test.tsx +++ b/apps/web/components/task/simple/components/task-launch-error-entry.test.tsx @@ -43,11 +43,15 @@ vi.mock("@/components/task/task-launch-branch-picker", () => ({ const TASK_ID = "task-1"; const TASK_REPOSITORY_ID = "task-repo-1"; const ERROR_STAMP = "launch-stamp-1"; +const WORKSPACE_ID = "workspace-1"; +const OCCURRED_AT = "2026-08-19T10:00:00Z"; +const ERROR_ENTRY_TEST_ID = "task-launch-error-entry"; +const RETRY_LAUNCH_BUTTON_TEST_ID = "task-launch-retry_launch-button"; const error: TaskStatusSummaryActiveError = { task_repository_id: TASK_REPOSITORY_ID, stamp: ERROR_STAMP, - occurred_at: "2026-08-19T10:00:00Z", + occurred_at: OCCURRED_AT, preview: "The selected base branch is not available.", category: "base_branch_missing", recovery_actions: ["retry_default", "pick_base_branch", "mark_review_done"], @@ -78,6 +82,7 @@ describe("TaskLaunchErrorEntry", () => { "base_branch_missing", "default_branch_unresolved", "pr_already_closed", + "workspace_checkout_failed", "generic_launch_failure", ])("classifies launch category %s as a typed launch error", (category) => { expect( @@ -88,30 +93,121 @@ describe("TaskLaunchErrorEntry", () => { ).toBe(true); }); + it("renders checkout details and retries the launch without branch settings", async () => { + const checkoutError: TaskStatusSummaryActiveError = { + session_id: "session-1", + stamp: "checkout-stamp-1", + occurred_at: OCCURRED_AT, + preview: "The workspace could not be prepared for this launch.", + details: "The pull request head was fetched into its isolated ref.", + category: "workspace_checkout_failed", + recovery_actions: ["retry_launch"], + }; + + render( + , + ); + + expect(screen.getByText(checkoutError.preview)).toBeTruthy(); + fireEvent.click(screen.getByRole("button", { name: /show details/i })); + expect(screen.getByText(checkoutError.details!)).toBeTruthy(); + expect(screen.getByTestId(RETRY_LAUNCH_BUTTON_TEST_ID)).toBeTruthy(); + fireEvent.click(screen.getByTestId(RETRY_LAUNCH_BUTTON_TEST_ID)); + + await waitFor(() => expect(requestMock).toHaveBeenCalledTimes(1)); + expect(requestMock).toHaveBeenCalledWith("task.launch.recover", { + task_id: TASK_ID, + session_id: "session-1", + action: "retry_launch", + error_stamp: "checkout-stamp-1", + }); + }); + + it("keeps recovery failures inside the launch card", async () => { + requestMock.mockRejectedValueOnce(new Error("recovery failed")); + render( + , + ); + + fireEvent.click(screen.getByTestId(RETRY_LAUNCH_BUTTON_TEST_ID)); + + await waitFor(() => expect(screen.getByTestId("task-launch-recovery-error")).toBeTruthy()); + expect(toastMock).not.toHaveBeenCalled(); + }); + + it("clears a recovery failure when a newer launch error arrives", async () => { + requestMock.mockRejectedValueOnce(new Error("recovery failed")); + const { rerender } = render( + , + ); + + fireEvent.click(screen.getByTestId(RETRY_LAUNCH_BUTTON_TEST_ID)); + await waitFor(() => expect(screen.getByTestId("task-launch-recovery-error")).toBeTruthy()); + + rerender( + , + ); + + await waitFor(() => expect(screen.queryByTestId("task-launch-recovery-error")).toBeNull()); + }); + it("renders a typed summary error without recovery actions", () => { render( , ); - expect(screen.getByTestId("task-launch-error-entry")).toBeTruthy(); + expect(screen.getByTestId(ERROR_ENTRY_TEST_ID)).toBeTruthy(); expect(screen.getByText(error.preview)).toBeTruthy(); expect(screen.queryByTestId("task-launch-retry_default-button")).toBeNull(); }); + it("renders a task-wide launch error while a prior session is selected", () => { + render( + , + ); + + expect(screen.getByTestId(ERROR_ENTRY_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId("task-launch-mark_review_done-button")).toBeTruthy(); + }); + it("renders the bounded preview and sends the exact task recovery payload", async () => { render( { />, ); - expect(screen.getByTestId("task-launch-error-entry")).toBeTruthy(); + expect(screen.getByTestId(ERROR_ENTRY_TEST_ID)).toBeTruthy(); expect(screen.getByText(error.preview)).toBeTruthy(); fireEvent.click(screen.getByTestId("task-launch-retry_default-button")); @@ -144,7 +240,7 @@ describe("TaskLaunchErrorEntry", () => { render( , @@ -165,7 +261,7 @@ describe("TaskLaunchErrorEntry", () => { render( , @@ -187,13 +283,13 @@ describe("TaskLaunchErrorEntry", () => { <> @@ -210,6 +306,6 @@ describe("TaskLaunchErrorEntry", () => { expect(buttons[1].getAttribute("disabled")).not.toBeNull(); }); resolveRequest?.({ ok: true }); - await waitFor(() => expect(screen.queryAllByTestId("task-launch-error-entry")).toHaveLength(2)); + await waitFor(() => expect(screen.queryAllByTestId(ERROR_ENTRY_TEST_ID)).toHaveLength(2)); }); }); diff --git a/apps/web/components/task/simple/components/task-launch-error-entry.tsx b/apps/web/components/task/simple/components/task-launch-error-entry.tsx index 6331033c11..7e7616ab2c 100644 --- a/apps/web/components/task/simple/components/task-launch-error-entry.tsx +++ b/apps/web/components/task/simple/components/task-launch-error-entry.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { IconAlertTriangle, IconCheck, @@ -9,7 +9,7 @@ import { IconRefresh, } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; -import { useToast } from "@/components/toast-provider"; +import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@kandev/ui/collapsible"; import { getWebSocketClient } from "@/lib/ws/connection"; import type { TaskRepository } from "@/lib/types/http"; import type { TaskLaunchRecoveryAction } from "@/lib/types/task-launch-error"; @@ -26,10 +26,64 @@ type TaskLaunchErrorEntryProps = { const pendingRecoveryRequests = new Map>(); +function useTaskLaunchRecovery({ + taskId, + error, +}: Pick) { + const [pendingAction, setPendingAction] = useState(null); + const [recoveryError, setRecoveryError] = useState(false); + + useEffect(() => { + setRecoveryError(false); + }, [error.stamp]); + + const sendRecovery = async (action: TaskLaunchRecoveryAction, baseBranch?: string) => { + const client = getWebSocketClient(); + if (!client || pendingAction) return; + setRecoveryError(false); + const requestKey = JSON.stringify({ taskId, stamp: error.stamp, action, baseBranch }); + const existingRequest = pendingRecoveryRequests.get(requestKey); + if (existingRequest) { + setPendingAction(action); + try { + await existingRequest; + } catch { + // The request owner reports the recovery error to avoid duplicate toasts. + } finally { + setPendingAction(null); + } + return; + } + setPendingAction(action); + const request = Promise.resolve().then(() => + client.request("task.launch.recover", { + task_id: taskId, + ...(error.session_id ? { session_id: error.session_id } : {}), + ...(error.task_repository_id ? { task_repository_id: error.task_repository_id } : {}), + action, + ...(baseBranch ? { base_branch: baseBranch } : {}), + error_stamp: error.stamp, + }), + ); + pendingRecoveryRequests.set(requestKey, request); + try { + await request; + } catch { + setRecoveryError(true); + } finally { + pendingRecoveryRequests.delete(requestKey); + setPendingAction(null); + } + }; + + return { pendingAction, recoveryError, sendRecovery }; +} + const TASK_LAUNCH_ERROR_CATEGORIES = new Set([ "base_branch_missing", "default_branch_unresolved", "pr_already_closed", + "workspace_checkout_failed", "generic_launch_failure", ]); @@ -51,6 +105,8 @@ function categoryTitle(category: string | undefined, t: (key: string) => string) return t("task:launchErrorDefaultBranchUnresolved"); case "pr_already_closed": return t("task:launchErrorPrAlreadyClosed"); + case "workspace_checkout_failed": + return t("task:launchErrorWorkspaceCheckoutFailed"); default: return t("task:launchErrorGeneric"); } @@ -64,6 +120,8 @@ function actionLabel(action: TaskLaunchRecoveryAction, t: (key: string) => strin return t("task:pickBaseBranch"); case "mark_review_done": return t("task:markReviewDone"); + case "retry_launch": + return t("task:retryLaunch"); } } @@ -76,6 +134,7 @@ function RecoveryActionIcon({ }) { if (pending) return ; if (action === "retry_default") return ; + if (action === "retry_launch") return ; if (action === "pick_base_branch") return ; return ; } @@ -99,14 +158,14 @@ function TaskLaunchRecoveryActions({ }) { const { t } = useTranslation(); return ( -
+
{actions.map((action) => { const opensBranchPicker = action === "pick_base_branch"; const button = (
); diff --git a/apps/web/components/task/simple/task-chat.tsx b/apps/web/components/task/simple/task-chat.tsx index 0b8c72ef79..c7085edb1f 100644 --- a/apps/web/components/task/simple/task-chat.tsx +++ b/apps/web/components/task/simple/task-chat.tsx @@ -35,6 +35,7 @@ import type { import { buildLaterAgentReplyMap, buildRunErrorsFromSessions, + filterVisibleRunErrors, mergeChatEntries, type ChatEntry, } from "./chat-entries"; @@ -615,6 +616,10 @@ export function TaskChat({ ); const turnCtx = useMemo(() => buildCommentTurnContext(comments, sessions), [comments, sessions]); const runErrors = useMemo(() => buildRunErrorsFromSessions(sessions), [sessions]); + const visibleRunErrors = useMemo( + () => filterVisibleRunErrors(runErrors, statusSummary?.active_error), + [runErrors, statusSummary?.active_error], + ); const laterAgentReplyMap = useMemo(() => buildLaterAgentReplyMap(comments), [comments]); const entries = useMemo( () => @@ -624,10 +629,10 @@ export function TaskChat({ groups: renderedGroups, decisions, turnCtx, - runErrors, + runErrors: visibleRunErrors, laterAgentReplyMap, }), - [comments, timeline, renderedGroups, decisions, turnCtx, runErrors, laterAgentReplyMap], + [comments, timeline, renderedGroups, decisions, turnCtx, visibleRunErrors, laterAgentReplyMap], ); useChatAutoScroll(scrollParent ?? null, sessions, taskId); @@ -652,7 +657,6 @@ export function TaskChat({ taskId={taskId} workspaceId={workspaceId} statusSummary={statusSummary} - runErrors={runErrors} repositories={repositories} /> {isEmpty ? ( diff --git a/apps/web/components/task/task-chat-panel.launch-error.test.tsx b/apps/web/components/task/task-chat-panel.launch-error.test.tsx new file mode 100644 index 0000000000..af1f609ad5 --- /dev/null +++ b/apps/web/components/task/task-chat-panel.launch-error.test.tsx @@ -0,0 +1,213 @@ +import type { ReactNode } from "react"; +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { Message } from "@/lib/types/http"; + +const launchError = { + stamp: "task-wide-launch-error", + occurred_at: "2026-08-20T10:00:00Z", + preview: "The workspace could not be prepared.", + category: "workspace_checkout_failed", +}; + +const priorTranscriptMessage = { + id: "prior-transcript", + author_type: "agent", + content: "The prior session stopped after its checkout failed.", + type: "status", + created_at: "2026-08-19T10:00:00Z", +} as unknown as Message; + +const appStoreState = { + userSettings: { + showAnchoredPromptBar: false, + showScrollToLastPrompt: false, + showScrollToStart: false, + }, + taskSessions: { + items: { + "prior-session": { name: null, agent_profile_id: null, last_read_message_id: null }, + }, + }, + agentProfiles: { items: [] }, + messages: { bySession: { "prior-session": [priorTranscriptMessage] } }, +}; + +const panelState = { + resolvedSessionId: "prior-session", + session: { state: "FAILED", error_message: "The prior session stopped." }, + taskId: "task-1", + isWorking: false, + messagesLoading: false, + historyRefreshPending: false, + isInitialMessagesLoading: false, + groupedItems: [], + allMessages: [priorTranscriptMessage], + footerActionMessages: [], + permissionsByToolCallId: new Map(), + childrenByParentToolCallId: new Map(), + agentMessageCount: 0, + pendingClarification: null, + pendingClarificationGroup: null, +}; + +vi.mock("./panel-primitives", () => ({ + PanelRoot: ({ children }: { children: ReactNode }) =>
{children}
, + PanelBody: ({ children }: { children: ReactNode }) =>
{children}
, +})); + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (state: typeof appStoreState) => unknown) => selector(appStoreState), + useAppStoreApi: () => ({ + getState: () => appStoreState, + }), +})); + +vi.mock("@/hooks/domains/settings/use-settings-data", () => ({ + useSettingsData: () => undefined, +})); + +vi.mock("@/hooks/use-responsive-breakpoint", () => ({ + useResponsiveBreakpoint: () => ({ isMobile: false }), +})); + +vi.mock("./task-archived-context", () => ({ + useIsTaskArchived: () => false, +})); + +vi.mock("./task-launch-error-context", () => ({ + useTaskLaunchErrorContext: () => ({ + taskId: "task-1", + workspaceId: "workspace-1", + statusSummary: { active_error: launchError }, + repositories: [], + }), +})); + +vi.mock("@/hooks/domains/task/use-task-status-summary", () => ({ + useTaskStatusSummary: () => ({ active_error: launchError }), +})); + +vi.mock("./chat/use-chat-panel-state", () => ({ + useChatPanelState: () => panelState, +})); + +vi.mock("./chat/chat-input-area", () => ({ + ChatInputArea: ({ + launchErrorOwned, + panelState: state, + }: { + launchErrorOwned?: boolean; + panelState: typeof panelState; + }) => ( +
+ {!launchErrorOwned && state.session?.state === "FAILED" && ( +
prior session recovery
+ )} +
+ ), + useSubmitHandler: () => ({ isSending: false, handleSubmit: vi.fn() }), + useChatPanelHandlers: () => ({ handleCancelTurn: vi.fn() }), +})); + +vi.mock("@/components/task/chat/message-list", () => ({ + MessageList: ({ + messages, + launchErrorOwned, + }: { + messages: Message[]; + launchErrorOwned?: boolean; + }) => ( +
+ {!launchErrorOwned && + messages.map((message) =>
{message.content}
)} +
+ ), +})); + +vi.mock("./simple/components/task-chat-launch-error", () => ({ + TaskChatLaunchError: ({ + statusSummary, + }: { + statusSummary?: { active_error?: typeof launchError }; + }) => + statusSummary?.active_error ? ( +
{statusSummary.active_error.preview}
+ ) : null, +})); + +vi.mock("@/components/shared/task-markdown-file-link-provider", () => ({ + TaskMarkdownFileLinkProvider: ({ children }: { children: ReactNode }) => <>{children}, +})); + +vi.mock("./chat/clarification-panel-section", () => ({ + ClarificationPanelSection: () => null, +})); + +vi.mock("./chat/use-composer-agent-start-hint", () => ({ + useComposerAgentStartHint: () => false, +})); + +vi.mock("@/hooks/use-panel-search", () => ({ + usePanelSearch: () => undefined, +})); + +vi.mock("@/hooks/domains/session/use-session-search", () => ({ + useSessionSearch: () => ({ + isOpen: false, + query: "", + hits: [], + activeHitId: null, + isSearching: false, + setQuery: vi.fn(), + setActiveHit: vi.fn(), + open: vi.fn(), + close: vi.fn(), + }), +})); + +vi.mock("@/hooks/use-lazy-load-messages", () => ({ + useLazyLoadMessages: () => ({ loadMoreRaw: vi.fn(), hasMore: false }), +})); + +vi.mock("@/components/task/chat/use-drain-older-messages", () => ({ + useDrainOlderMessages: () => undefined, +})); + +vi.mock("./chat/use-session-read-tracking", () => ({ + useSessionReadTracking: () => null, +})); + +vi.mock("@/lib/state/dockview-store", () => ({ + useDockviewStore: (selector: (state: { scrollTarget: null }) => unknown) => + selector({ scrollTarget: null }), + getState: () => ({ scrollTarget: null }), +})); + +vi.mock("@/hooks/domains/session/load-message-window", () => ({ + loadMessageWindowAround: vi.fn(), +})); + +vi.mock("@/lib/session-workspace-path", () => ({ + getSessionWorkspacePath: () => undefined, +})); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +import { TaskChatPanel } from "./task-chat-panel"; + +afterEach(() => cleanup()); + +describe("TaskChatPanel launch-error ownership", () => { + it("keeps a task-wide card and prior failed-session surfaces together", () => { + render(); + + expect(screen.getByTestId("task-launch-error-entry").textContent).toContain( + launchError.preview, + ); + expect(screen.getByTestId("session-stopped-banner")).toBeTruthy(); + expect(screen.getByText(priorTranscriptMessage.content)).toBeTruthy(); + }); +}); diff --git a/apps/web/components/task/task-chat-panel.tsx b/apps/web/components/task/task-chat-panel.tsx index 1046fba94d..3708514965 100644 --- a/apps/web/components/task/task-chat-panel.tsx +++ b/apps/web/components/task/task-chat-panel.tsx @@ -52,8 +52,10 @@ import { useTranslation } from "react-i18next"; import { loadMessageWindowAround } from "@/hooks/domains/session/load-message-window"; import { TaskChatLaunchError } from "./simple/components/task-chat-launch-error"; +import { isTypedTaskLaunchError } from "./simple/components/task-launch-error-entry"; import { useTaskLaunchErrorContext } from "./task-launch-error-context"; import { useTaskStatusSummary } from "@/hooks/domains/task/use-task-status-summary"; +import { isTaskLaunchErrorOwnedBySession } from "@/components/task/chat/types"; import { TaskMarkdownFileLinkProvider } from "@/components/shared/task-markdown-file-link-provider"; /** Returns a `clarificationKey` that increments each time a pending @@ -1024,6 +1026,11 @@ export const TaskChatPanel = memo(function TaskChatPanel({ pendingClarification, pendingClarificationGroup, } = panelState; + const activeLaunchError = launchStatusSummary?.active_error; + const launchErrorOwned = Boolean( + isTypedTaskLaunchError(activeLaunchError) && + isTaskLaunchErrorOwnedBySession(activeLaunchError, resolvedSessionId), + ); const showAgentStartHint = useComposerAgentStartHint( resolvedSessionId, session?.state, @@ -1165,7 +1172,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({ taskId={launchErrorContext.taskId} workspaceId={launchErrorContext.workspaceId} statusSummary={launchStatusSummary} - runErrors={[]} + sessionId={resolvedSessionId} repositories={launchErrorContext.repositories} /> )} @@ -1197,6 +1204,9 @@ export const TaskChatPanel = memo(function TaskChatPanel({ onFirstMessageHiddenChange={setIsFirstMessageHidden} anchoredBarHeight={showAnchoredBar && lastPromptMessage ? anchoredBarHeight : 0} isVisible={transcriptIsVisible} + launchErrorOwned={launchErrorOwned} + launchErrorStamp={launchErrorOwned ? activeLaunchError?.stamp : undefined} + launchErrorOccurredAt={launchErrorOwned ? activeLaunchError?.occurred_at : undefined} stickyPromptBar={ showAnchoredBar && lastPromptMessage ? ( ); @@ -1279,6 +1290,7 @@ type ChatFooterProps = { statusTaskId: string | null; /** Recovered-idle sessions render the composer hint (see ChatInputArea). */ showAgentStartHint: boolean; + launchErrorOwned: boolean; }; /** @@ -1306,6 +1318,7 @@ function ChatFooter({ onScrollToStart, statusTaskId, showAgentStartHint, + launchErrorOwned, }: ChatFooterProps) { const { t } = useTranslation(); if (isArchived) { @@ -1335,6 +1348,7 @@ function ChatFooter({ onScrollToStart={onScrollToStart} statusTaskId={statusTaskId} showAgentStartHint={showAgentStartHint} + launchErrorOwned={launchErrorOwned} /> ); } diff --git a/apps/web/components/task/task-launch-error-context.test.tsx b/apps/web/components/task/task-launch-error-context.test.tsx index 0ca4c9dcb5..8c27aad5a5 100644 --- a/apps/web/components/task/task-launch-error-context.test.tsx +++ b/apps/web/components/task/task-launch-error-context.test.tsx @@ -49,6 +49,12 @@ beforeEach(() => { }); describe("TaskLaunchErrorProvider", () => { + it("does not toast for a typed launch error while its card is visible", () => { + renderProvider(initialSummary); + + expect(toastMock).not.toHaveBeenCalled(); + }); + it.each(["provider_auth_required", "model_capacity"])( "does not toast for ordinary active error category %s", (category) => { diff --git a/apps/web/components/task/task-launch-error-context.tsx b/apps/web/components/task/task-launch-error-context.tsx index 71748d8d3a..7983b2dbe3 100644 --- a/apps/web/components/task/task-launch-error-context.tsx +++ b/apps/web/components/task/task-launch-error-context.tsx @@ -1,16 +1,9 @@ "use client"; -import { createContext, useContext, useEffect, useMemo, useRef, type ReactNode } from "react"; -import { useToast } from "@/components/toast-provider"; -import { t } from "@/lib/i18n"; -import { getSessionStorage, setSessionStorage } from "@/lib/local-storage"; +import { createContext, useContext, useMemo, type ReactNode } from "react"; import type { TaskRepository } from "@/lib/types/http"; import type { TaskStatusSummary } from "@/lib/types/task-status-summary"; import { useTaskStatusSummary } from "@/hooks/domains/task/use-task-status-summary"; -import { isTypedTaskLaunchError } from "./simple/components/task-launch-error-entry"; - -// i18n-exempt: persisted browser storage key, not user-facing copy. -const LAUNCH_ERROR_TOASTS_KEY = "kandev.task-launch-error-toasts"; export type TaskLaunchErrorContextValue = { taskId: string; @@ -28,39 +21,8 @@ export function TaskLaunchErrorProvider({ value: TaskLaunchErrorContextValue; children: ReactNode; }) { - const { toast } = useToast(); - const shownToastStampsRef = useRef(new Set()); const statusSummary = useTaskStatusSummary(value.taskId, value.statusSummary); - useEffect(() => { - const activeError = statusSummary?.active_error; - if (!isTypedTaskLaunchError(activeError)) return; - - const localStamp = `${value.taskId}:${activeError.stamp}`; - if (shownToastStampsRef.current.has(localStamp)) return; - const shownByTask = getSessionStorage>(LAUNCH_ERROR_TOASTS_KEY, {}); - if ((shownByTask[value.taskId] ?? []).includes(activeError.stamp)) { - shownToastStampsRef.current.add(localStamp); - return; - } - - shownToastStampsRef.current.add(localStamp); - setSessionStorage(LAUNCH_ERROR_TOASTS_KEY, { - ...shownByTask, - [value.taskId]: [...(shownByTask[value.taskId] ?? []), activeError.stamp].slice(-20), - }); - toast({ - title: t("task:taskFailedToStart"), - description: t("task:launchFailedSeeDetails"), - variant: "error", - }); - }, [ - statusSummary?.active_error?.category, - statusSummary?.active_error?.stamp, - toast, - value.taskId, - ]); - const contextValue = useMemo(() => ({ ...value, statusSummary }), [statusSummary, value]); return ( diff --git a/apps/web/components/task/task-layout.tsx b/apps/web/components/task/task-layout.tsx index 3595b338fb..4e8381b7b7 100644 --- a/apps/web/components/task/task-layout.tsx +++ b/apps/web/components/task/task-layout.tsx @@ -93,7 +93,6 @@ export const TaskLayout = memo(function TaskLayout({ taskId={launchErrorContext.taskId} workspaceId={launchErrorContext.workspaceId} statusSummary={launchErrorContext.statusSummary} - runErrors={[]} repositories={launchErrorContext.repositories} />
diff --git a/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts b/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts index db5a7f003d..9f1a95fa26 100644 --- a/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts +++ b/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts @@ -24,7 +24,10 @@ async function switchMobileTask(testPage: Page, title: string): Promise { const sheet = testPage.getByRole("dialog", { name: "Tasks" }); const taskRow = sheet.getByTestId("sidebar-task-item").filter({ hasText: title }); await expect(taskRow).toBeVisible({ timeout: 15_000 }); - await taskRow.tap(); + // This test covers task-switch transcript continuity, not touch activation. + // Use a click so the row's long-press drag sensor cannot consume the gesture + // while the mobile drawer is under CI load. + await taskRow.click(); await expect(sheet).not.toBeVisible({ timeout: 10_000 }); } diff --git a/apps/web/e2e/tests/kanban/wip-overflow-queue.spec.ts b/apps/web/e2e/tests/kanban/wip-overflow-queue.spec.ts index 0e0988995b..fce37a2bbc 100644 --- a/apps/web/e2e/tests/kanban/wip-overflow-queue.spec.ts +++ b/apps/web/e2e/tests/kanban/wip-overflow-queue.spec.ts @@ -71,7 +71,7 @@ test("dragging into a feeder wakes an open pull target without reload", async ({ x: feederBox!.x + feederBox!.width / 2, y: feederBox!.y + Math.min(160, feederBox!.height / 2), }; - await testPage.mouse.move(feederPoint.x, feederPoint.y); + await testPage.mouse.move(feederPoint.x, feederPoint.y, { steps: 1 }); await expect(feederColumn).toHaveClass(/bg-primary\/5/); await testPage.mouse.up(); diff --git a/apps/web/e2e/tests/kubernetes/kubernetes-executor.spec.ts b/apps/web/e2e/tests/kubernetes/kubernetes-executor.spec.ts index 09fac91af0..def96d39ff 100644 --- a/apps/web/e2e/tests/kubernetes/kubernetes-executor.spec.ts +++ b/apps/web/e2e/tests/kubernetes/kubernetes-executor.spec.ts @@ -52,9 +52,12 @@ async function assertUserVisibleFailure(page: Page, taskId: string, expected: Re await page.goto(`/t/${taskId}`); const session = new SessionPage(page); await session.waitForLoad(30_000); - const banner = session.activeChat().getByTestId("failed-session-banner"); - await expect(banner).toBeVisible({ timeout: 30_000 }); - await expect(banner).toContainText(expected); + const launchError = session.activeChat().getByTestId("task-launch-error-entry"); + await expect(launchError).toBeVisible({ timeout: 30_000 }); + await launchError.getByRole("button", { name: "Show details" }).click(); + const details = launchError.getByTestId("task-launch-error-details"); + await expect(details).toBeVisible(); + await expect(details).toContainText(expected); } async function createProfile( diff --git a/apps/web/e2e/tests/office/provider-remediation-link.spec.ts b/apps/web/e2e/tests/office/provider-remediation-link.spec.ts index e099c0dfa5..f2a50bf577 100644 --- a/apps/web/e2e/tests/office/provider-remediation-link.spec.ts +++ b/apps/web/e2e/tests/office/provider-remediation-link.spec.ts @@ -11,9 +11,6 @@ test.describe("Office run-error remediation link", () => { const task = await apiClient.createTask(officeSeed.workspaceId, "Office remediation link", { workflow_id: officeSeed.workflowId, }); - await apiClient.rawRequest("PATCH", `/api/v1/office/tasks/${task.id}`, { - assignee_agent_profile_id: officeSeed.agentId, - }); await apiClient.seedTaskSession(task.id, { state: "FAILED", agentProfileId: officeSeed.agentId, @@ -71,9 +68,6 @@ test.describe("Office run-error remediation link", () => { const task = await apiClient.createTask(officeSeed.workspaceId, title, { workflow_id: officeSeed.workflowId, }); - await apiClient.rawRequest("PATCH", `/api/v1/office/tasks/${task.id}`, { - assignee_agent_profile_id: officeSeed.agentId, - }); await apiClient.seedTaskSession(task.id, { state: "FAILED", agentProfileId: officeSeed.agentId, diff --git a/apps/web/e2e/tests/pr/mobile-pr-watcher-missing-branch.spec.ts b/apps/web/e2e/tests/pr/mobile-pr-watcher-missing-branch.spec.ts index 5a287ab9a0..8f298146bc 100644 --- a/apps/web/e2e/tests/pr/mobile-pr-watcher-missing-branch.spec.ts +++ b/apps/web/e2e/tests/pr/mobile-pr-watcher-missing-branch.spec.ts @@ -75,7 +75,7 @@ test.describe("mobile PR watcher missing branch", () => { const recovery = chat.getByTestId("task-launch-error-entry"); await expect(recovery).toHaveCount(1, { timeout: 30_000 }); - await expect(recovery).toContainText("The selected base branch is not available."); + await expect(recovery).toContainText("The workspace could not be prepared for this launch."); await expect(recovery).not.toContainText(prBranch); const actionButtons = recovery.locator("button[data-testid^='task-launch-']"); await expect(actionButtons).not.toHaveCount(0); diff --git a/apps/web/e2e/tests/pr/pr-watcher-missing-branch.spec.ts b/apps/web/e2e/tests/pr/pr-watcher-missing-branch.spec.ts index 44467fabb7..63746028e6 100644 --- a/apps/web/e2e/tests/pr/pr-watcher-missing-branch.spec.ts +++ b/apps/web/e2e/tests/pr/pr-watcher-missing-branch.spec.ts @@ -18,7 +18,8 @@ test.describe("PR watcher missing branch", () => { const prBranch = "feature/already-merged-and-deleted"; - // Mock an open PR whose head branch was deleted on the remote. + // Mock an open PR whose head branch was deleted on the remote. The PR + // head cannot be materialized, so this is a workspace-preparation failure. await apiClient.mockGitHubAddPRs([ { number: 999, @@ -82,7 +83,7 @@ test.describe("PR watcher missing branch", () => { }, { timeout: 60_000, message: "waiting for the durable PR launch-error projection" }, ) - .toBe("base_branch_missing"); + .toBe("workspace_checkout_failed"); // --- Navigate to the task session view --- await testPage.goto(`/t/${task.id}`); @@ -95,7 +96,7 @@ test.describe("PR watcher missing branch", () => { const chat = session.activeChat(); const recovery = chat.getByTestId("task-launch-error-entry"); await expect(recovery).toHaveCount(1, { timeout: 30_000 }); - await expect(recovery).toContainText("The selected base branch is not available."); + await expect(recovery).toContainText("The workspace could not be prepared for this launch."); await expect(recovery).not.toContainText(prBranch); await expect(recovery.getByTestId("task-launch-archive-button")).toHaveCount(0); await expect(recovery.getByTestId("task-launch-delete-button")).toHaveCount(0); diff --git a/apps/web/e2e/tests/task/create-task-github-url.spec.ts b/apps/web/e2e/tests/task/create-task-github-url.spec.ts index 687d670261..dbb23e05c3 100644 --- a/apps/web/e2e/tests/task/create-task-github-url.spec.ts +++ b/apps/web/e2e/tests/task/create-task-github-url.spec.ts @@ -3,6 +3,7 @@ import { test, expect } from "../../fixtures/test-base"; import { useRegularMode } from "../../helpers/regular-mode"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; +import { createEmptyRemoteRepository } from "../../helpers/empty-remote-repository"; // Exercises the regular task-create dialog (New Task in the sidebar); run with office off. useRegularMode(); @@ -342,6 +343,10 @@ test.describe("Task creation from GitHub URL", () => { const repoDir = `${backend.tmpDir}/repos/e2e-repo`; execSync("git checkout -b feature/pr-branch", { cwd: repoDir, env: gitEnv }); execSync('git commit --allow-empty -m "pr commit"', { cwd: repoDir, env: gitEnv }); + execSync("git push origin feature/pr-branch:refs/pull/99/head", { + cwd: repoDir, + env: gitEnv, + }); execSync("git checkout main", { cwd: repoDir, env: gitEnv }); // Pre-seed a GitHub-backed repository @@ -431,7 +436,6 @@ test.describe("Task creation from GitHub URL", () => { test.setTimeout(90_000); const { execSync } = await import("child_process"); - const fs = await import("fs"); const gitEnv = { ...process.env, HOME: backend.tmpDir, @@ -441,17 +445,21 @@ test.describe("Task creation from GitHub URL", () => { GIT_COMMITTER_EMAIL: "e2e@test.local", }; - // Create a fresh repo with the PR branch available locally. - // No remote needed — the worktree manager falls back to local branches. - const repoDir = `${backend.tmpDir}/repos/e2e-pr-wt`; - fs.mkdirSync(repoDir, { recursive: true }); + // Create a fresh repo with an offline origin that publishes the PR + // snapshot ref used by the worktree manager. + const repository = createEmptyRemoteRepository(backend.tmpDir, "pr-wt"); + const repoDir = repository.localPath; - execSync("git init -b main", { cwd: repoDir, env: gitEnv }); execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv }); + execSync("git push origin main", { cwd: repoDir, env: gitEnv }); // Create the PR branch locally and switch back to main execSync("git checkout -b feature/pr-branch", { cwd: repoDir, env: gitEnv }); execSync('git commit --allow-empty -m "pr branch commit"', { cwd: repoDir, env: gitEnv }); + execSync("git push origin feature/pr-branch:refs/pull/77/head", { + cwd: repoDir, + env: gitEnv, + }); execSync("git checkout main", { cwd: repoDir, env: gitEnv }); // Register the repo with a unique provider name to avoid collisions with @@ -540,7 +548,7 @@ test.describe("Task creation from GitHub URL", () => { await session.expectTerminalHasText("feature/pr-branch"); }); - test("shows fetch warning banner when PR branch has no remote", async ({ + test("shows a launch error when the PR branch has no remote snapshot", async ({ testPage, apiClient, seedData, @@ -559,9 +567,8 @@ test.describe("Task creation from GitHub URL", () => { GIT_COMMITTER_EMAIL: "e2e@test.local", }; - // Create a repo with the PR branch locally but NO remote. - // This causes fetchBranchToLocal to fail the fetch and fall back to local, - // which produces a warning that should be displayed in the UI. + // Create a repo with the PR branch locally but NO remote. PR launches must + // fail closed when the immutable pull-request snapshot cannot be fetched. const repoDir = `${backend.tmpDir}/repos/e2e-warning-repo`; fs.mkdirSync(repoDir, { recursive: true }); execSync("git init -b main", { cwd: repoDir, env: gitEnv }); @@ -596,7 +603,7 @@ test.describe("Task creation from GitHub URL", () => { }, ]); - // Need worktree executor to trigger fetchBranchToLocal (local executor doesn't produce warnings) + // Need worktree executor to exercise the authenticated PR snapshot fetch. const { executors } = await apiClient.listExecutors(); const worktreeExec = executors.find((e) => e.type === "worktree"); const worktreeProfile = worktreeExec?.profiles?.[0]; @@ -637,28 +644,17 @@ test.describe("Task creation from GitHub URL", () => { const session = new SessionPage(testPage); await session.waitForLoad(); - // Wait for agent to complete (preparation is done by this point) - await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible({ - timeout: 30_000, - }); + const launchError = session.activeChat().getByTestId("task-launch-error-entry"); + await expect(launchError).toBeVisible({ timeout: 30_000 }); + await expect(launchError).toContainText(/launch needs attention/i); - // The warning banner should persist after preparation completes. - // It shows because fetchBranchToLocal failed to reach origin (no remote) - // and fell back to the local branch. - const warningBanner = testPage.getByTestId("prepare-warning-banner"); - await expect(warningBanner).toBeVisible({ timeout: 10_000 }); - await expect(warningBanner).toContainText("Could not fetch latest from origin"); - - // The "Details" toggle should be visible and expand raw git output on click. - const detailsBtn = warningBanner.getByRole("button", { name: "Details" }); + const detailsBtn = launchError.getByRole("button", { name: "Show details" }); await expect(detailsBtn).toBeVisible(); await detailsBtn.click(); - // After expanding, raw git output should appear (e.g. "fatal:" from the failed fetch) - await expect(warningBanner.locator("pre")).toBeVisible(); - - // Clicking again should collapse the details. - await detailsBtn.click(); - await expect(warningBanner.locator("pre")).not.toBeVisible(); + await expect(launchError.getByTestId("task-launch-error-details")).toContainText( + /pull request head 200|no remote ref|workspace checkout failed/i, + ); + await expect(testPage.getByTestId("prepare-warning-banner")).toHaveCount(0); }); test("two tasks from the same PR URL create independent worktrees", async ({ @@ -670,7 +666,6 @@ test.describe("Task creation from GitHub URL", () => { test.setTimeout(120_000); const { execSync } = await import("child_process"); - const fs = await import("fs"); const gitEnv = { ...process.env, HOME: backend.tmpDir, @@ -680,13 +675,18 @@ test.describe("Task creation from GitHub URL", () => { GIT_COMMITTER_EMAIL: "e2e@test.local", }; - // Create a repo with the PR branch locally. - const repoDir = `${backend.tmpDir}/repos/e2e-shared-pr`; - fs.mkdirSync(repoDir, { recursive: true }); - execSync("git init -b main", { cwd: repoDir, env: gitEnv }); + // Create a repo whose offline origin publishes the immutable PR snapshot + // used by both independent worktrees. + const repository = createEmptyRemoteRepository(backend.tmpDir, "shared-pr"); + const repoDir = repository.localPath; execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv }); + execSync("git push origin main", { cwd: repoDir, env: gitEnv }); execSync("git checkout -b feature/shared-pr", { cwd: repoDir, env: gitEnv }); execSync('git commit --allow-empty -m "shared pr commit"', { cwd: repoDir, env: gitEnv }); + execSync("git push origin feature/shared-pr:refs/pull/50/head", { + cwd: repoDir, + env: gitEnv, + }); execSync("git checkout main", { cwd: repoDir, env: gitEnv }); await apiClient.createRepository(seedData.workspaceId, repoDir, "main", { diff --git a/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts b/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts index e2e09d48bf..1ec3dc794f 100644 --- a/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts +++ b/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts @@ -221,16 +221,20 @@ test.describe("task launch failure recovery", () => { expect(launchError.task_repository_id).toBe(taskRepository.id); expect(launchError.recovery_actions).toEqual(["retry_default", "pick_base_branch"]); - const pointerToast = testPage - .getByTestId("toast-message") - .filter({ hasText: "The task launch failed. Open the task details for recovery actions." }); - await expect(pointerToast).toBeVisible({ timeout: 30_000 }); - await expect(pointerToast).not.toContainText("branch-that-no-longer-exists"); + await expect( + testPage.getByTestId("toast-message").filter({ + hasText: "The task launch failed. Open the task details for recovery actions.", + }), + ).toHaveCount(0); const card = testPage.getByTestId("task-launch-error-entry"); await expect(card).toHaveCount(1, { timeout: 30_000 }); await expect(card).toContainText("The selected base branch is not available."); await expect(card).not.toContainText("branch-that-no-longer-exists"); + await expect(testPage.getByTestId("last-agent-error-notice")).toHaveCount(0); + await expect(testPage.getByTestId("prepare-progress-panel")).toHaveCount(0); + await expect(testPage.getByTestId("missing-branch-recovery")).toHaveCount(0); + await expect(testPage.getByTestId("recovery-resume-button")).toHaveCount(0); restoreSeedRepositoryOrigin(seedData); await testPage.reload(); diff --git a/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts b/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts index f3f9f4145c..47ad74984c 100644 --- a/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts +++ b/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts @@ -81,6 +81,10 @@ test.describe("mobile task launch failure recovery", () => { const card = testPage.getByTestId("task-launch-error-entry"); await expect(card).toHaveCount(1, { timeout: 30_000 }); + await expect(testPage.getByTestId("last-agent-error-notice")).toHaveCount(0); + await expect(testPage.getByTestId("prepare-progress-panel")).toHaveCount(0); + await expect(testPage.getByTestId("missing-branch-recovery")).toHaveCount(0); + await expect(testPage.getByTestId("recovery-resume-button")).toHaveCount(0); const actionButtons = card.locator("button[data-testid^='task-launch-']"); await expect(actionButtons).not.toHaveCount(0); for (const button of await actionButtons.all()) { diff --git a/apps/web/lib/types/task-launch-error.ts b/apps/web/lib/types/task-launch-error.ts index 8fba233ba9..e3f3e5dc6a 100644 --- a/apps/web/lib/types/task-launch-error.ts +++ b/apps/web/lib/types/task-launch-error.ts @@ -2,6 +2,7 @@ export const TASK_LAUNCH_RECOVERY_ACTIONS = [ "retry_default", "pick_base_branch", "mark_review_done", + "retry_launch", ] as const; export type TaskLaunchRecoveryAction = (typeof TASK_LAUNCH_RECOVERY_ACTIONS)[number]; diff --git a/apps/web/lib/types/task-status-summary.ts b/apps/web/lib/types/task-status-summary.ts index 99a5d544f8..d64ea55ec1 100644 --- a/apps/web/lib/types/task-status-summary.ts +++ b/apps/web/lib/types/task-status-summary.ts @@ -7,6 +7,7 @@ export type TaskStatusSummaryActiveError = { stamp: string; occurred_at: string; preview: string; + details?: string; category?: string; recovery_actions?: TaskLaunchRecoveryAction[]; }; diff --git a/apps/web/src/locales/en/task.json b/apps/web/src/locales/en/task.json index 2df46974b7..05a324b62b 100644 --- a/apps/web/src/locales/en/task.json +++ b/apps/web/src/locales/en/task.json @@ -1999,13 +1999,16 @@ "launchErrorBaseBranchMissing": "The task could not start because its base branch is unavailable.", "launchErrorDefaultBranchUnresolved": "The task could not resolve the repository default branch.", "launchErrorPrAlreadyClosed": "The task is linked to a closed pull request.", + "launchErrorWorkspaceCheckoutFailed": "The task could not prepare its workspace for launch.", "launchErrorGeneric": "The task could not start.", "launchErrorSessionPreview": "The task launch needs attention.", "launchNeedsAttention": "Launch needs attention", + "launchErrorNoChanges": "No task or branch settings were changed.", "launchPromptPreview": "Launch prompt preview", "retryWithDefaultBranch": "Retry with default branch", "pickBaseBranch": "Pick a base branch", "markReviewDone": "Mark review done", + "retryLaunch": "Retry launch", "chooseBaseBranch": "Choose a base branch", "chooseBaseBranchDescription": "Select the branch to use before the task relaunches.", "baseBranchNotSet": "Base branch not set", diff --git a/apps/web/src/locales/pseudo/task.json b/apps/web/src/locales/pseudo/task.json index 9fcbfe75f2..898f35874e 100644 --- a/apps/web/src/locales/pseudo/task.json +++ b/apps/web/src/locales/pseudo/task.json @@ -1999,13 +1999,16 @@ "launchErrorBaseBranchMissing": "Ţĥē ţàśķ ćōũĺď ńōţ śţàŕţ ƀēćàũśē ĩţś ƀàśē ƀŕàńćĥ ĩś ũńàvàĩĺàƀĺē.", "launchErrorDefaultBranchUnresolved": "Ţĥē ţàśķ ćōũĺď ńōţ ŕēśōĺvē ţĥē ŕēƥōśĩţōŕŷ ďēƒàũĺţ ƀŕàńćĥ.", "launchErrorPrAlreadyClosed": "Ţĥē ţàśķ ĩś ĺĩńķēď ţō à ćĺōśēď ƥũĺĺ ŕēqũēśţ.", + "launchErrorWorkspaceCheckoutFailed": "Ţĥē ţàśķ ćōũĺď ńōţ ƥŕēƥàŕē ĩţś ŵōŕķśƥàćē ƒōŕ ĺàũńćĥ.", "launchErrorGeneric": "Ţĥē ţàśķ ćōũĺď ńōţ śţàŕţ.", "launchErrorSessionPreview": "Ţĥē ţàśķ ĺàũńćĥ ńēēďś àţţēńţĩōń.", "launchNeedsAttention": "Ĺàũńćĥ ńēēďś àţţēńţĩōń", + "launchErrorNoChanges": "Ńō ţàśķ ōŕ ƀŕàńćĥ śēţţĩńĝś ŵēŕē ćĥàńĝēď.", "launchPromptPreview": "Ĺàũńćĥ ƥŕōḿƥţ ƥŕēvĩēŵ", "retryWithDefaultBranch": "Ŕēţŕŷ ŵĩţĥ ďēƒàũĺţ ƀŕàńćĥ", "pickBaseBranch": "Ƥĩćķ à ƀàśē ƀŕàńćĥ", "markReviewDone": "Ḿàŕķ ŕēvĩēŵ ďōńē", + "retryLaunch": "Ŕēţŕŷ ĺàũńćĥ", "chooseBaseBranch": "Ćĥōōśē à ƀàśē ƀŕàńćĥ", "chooseBaseBranchDescription": "Śēĺēćţ ţĥē ƀŕàńćĥ ţō ũśē ƀēƒōŕē ţĥē ţàśķ ŕēĺàũńćĥēś.", "baseBranchNotSet": "Ɓàśē ƀŕàńćĥ ńōţ śēţ", diff --git a/apps/web/src/locales/pt-pt/task.json b/apps/web/src/locales/pt-pt/task.json index becd2955e2..116417299f 100644 --- a/apps/web/src/locales/pt-pt/task.json +++ b/apps/web/src/locales/pt-pt/task.json @@ -1999,13 +1999,16 @@ "launchErrorBaseBranchMissing": "Não foi possível iniciar a tarefa porque o branch base não está disponível.", "launchErrorDefaultBranchUnresolved": "Não foi possível resolver o branch predefinido do repositório.", "launchErrorPrAlreadyClosed": "A tarefa está associada a um pull request fechado.", + "launchErrorWorkspaceCheckoutFailed": "Não foi possível preparar o workspace da tarefa para o arranque.", "launchErrorGeneric": "Não foi possível iniciar a tarefa.", "launchErrorSessionPreview": "O início da tarefa requer atenção.", "launchNeedsAttention": "O início requer atenção", + "launchErrorNoChanges": "As definições da tarefa e do branch não foram alteradas.", "launchPromptPreview": "Pré-visualização do prompt de início", "retryWithDefaultBranch": "Tentar novamente com o branch predefinido", "pickBaseBranch": "Escolher um branch base", "markReviewDone": "Marcar revisão como concluída", + "retryLaunch": "Tentar iniciar novamente", "chooseBaseBranch": "Escolher um branch base", "chooseBaseBranchDescription": "Selecione o branch a usar antes de reiniciar a tarefa.", "baseBranchNotSet": "Branch base não definido", diff --git a/apps/web/src/locales/zh-cn/task.json b/apps/web/src/locales/zh-cn/task.json index ac4a727f50..4993aad5a2 100644 --- a/apps/web/src/locales/zh-cn/task.json +++ b/apps/web/src/locales/zh-cn/task.json @@ -1999,13 +1999,16 @@ "launchErrorBaseBranchMissing": "任务无法启动，因为基础分支不可用。", "launchErrorDefaultBranchUnresolved": "无法解析仓库的默认分支。", "launchErrorPrAlreadyClosed": "任务关联的拉取请求已关闭。", + "launchErrorWorkspaceCheckoutFailed": "任务无法准备启动所需的工作区。", "launchErrorGeneric": "任务无法启动。", "launchErrorSessionPreview": "任务启动需要处理。", "launchNeedsAttention": "启动需要处理", + "launchErrorNoChanges": "任务或分支设置未更改。", "launchPromptPreview": "启动提示词预览", "retryWithDefaultBranch": "使用默认分支重试", "pickBaseBranch": "选择基础分支", "markReviewDone": "标记评审完成", + "retryLaunch": "重试启动", "chooseBaseBranch": "选择基础分支", "chooseBaseBranchDescription": "选择任务重新启动前要使用的分支。", "baseBranchNotSet": "未设置基础分支", diff --git a/apps/web/src/locales/zh-hk/task.json b/apps/web/src/locales/zh-hk/task.json index 540539117f..9743b07bb9 100644 --- a/apps/web/src/locales/zh-hk/task.json +++ b/apps/web/src/locales/zh-hk/task.json @@ -1999,13 +1999,16 @@ "launchErrorBaseBranchMissing": "任務無法啓動，因為基礎分支不可用。", "launchErrorDefaultBranchUnresolved": "無法解析儲存庫的預設分支。", "launchErrorPrAlreadyClosed": "任務關聯的拉取要求已關閉。", + "launchErrorWorkspaceCheckoutFailed": "任務無法準備啓動所需的工作區。", "launchErrorGeneric": "任務無法啓動。", "launchErrorSessionPreview": "任務啓動需要處理。", "launchNeedsAttention": "啓動需要處理", + "launchErrorNoChanges": "任務或分支設定未有變更。", "launchPromptPreview": "啓動提示詞預覽", "retryWithDefaultBranch": "使用預設分支重試", "pickBaseBranch": "選擇基礎分支", "markReviewDone": "標記審查完成", + "retryLaunch": "重試啓動", "chooseBaseBranch": "選擇基礎分支", "chooseBaseBranchDescription": "選擇任務重新啓動前要使用的分支。", "baseBranchNotSet": "未設定基礎分支", diff --git a/apps/web/src/locales/zh-tw/task.json b/apps/web/src/locales/zh-tw/task.json index 68857ec2d4..6815ee81cd 100644 --- a/apps/web/src/locales/zh-tw/task.json +++ b/apps/web/src/locales/zh-tw/task.json @@ -1999,13 +1999,16 @@ "launchErrorBaseBranchMissing": "任務無法啟動，因為基礎分支不可用。", "launchErrorDefaultBranchUnresolved": "無法解析儲存庫的預設分支。", "launchErrorPrAlreadyClosed": "任務關聯的提取要求已關閉。", + "launchErrorWorkspaceCheckoutFailed": "任務無法準備啟動所需的工作區。", "launchErrorGeneric": "任務無法啟動。", "launchErrorSessionPreview": "任務啟動需要處理。", "launchNeedsAttention": "啟動需要處理", + "launchErrorNoChanges": "任務或分支設定沒有變更。", "launchPromptPreview": "啟動提示詞預覽", "retryWithDefaultBranch": "使用預設分支重試", "pickBaseBranch": "選擇基礎分支", "markReviewDone": "標記審查完成", + "retryLaunch": "重試啟動", "chooseBaseBranch": "選擇基礎分支", "chooseBaseBranchDescription": "選擇任務重新啟動前要使用的分支。", "baseBranchNotSet": "未設定基礎分支", diff --git a/docs/plans/task-launch-error-consolidation/plan.md b/docs/plans/task-launch-error-consolidation/plan.md new file mode 100644 index 0000000000..617962bba9 --- /dev/null +++ b/docs/plans/task-launch-error-consolidation/plan.md @@ -0,0 +1,159 @@ +--- +created: 2026-09-05 +status: completed +requirements: + - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 + - REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-001 +system_design: + - ../../specs/tasks/system-design/task-launch-failure-recovery.md + - ../../specs/tasks/system-design/remote-contribution-tasks.md +legacy_specs: [] +--- + +# Implementation Plan: Task Launch Error Consolidation + +## Overview + +The change first isolates pull-request refs from reused local branch names. +Then it makes recovery actions category-specific. The final work order gives +one error card ownership of the desktop and phone presentation. + +## Confirmed root cause + +PR #3408 uses `feature/unit-7-reject-leg-wa-waj` at +`a76c5d8de780b0b857af8195eab4772bb1a5839b`. The shared clone still has the same +local branch from PR #3294 at `f825122f586978ed5e33fe48d61e401a610318e2`. + +Neither commit contains the other. The required fetch tried to update the local +branch directly. Git rejected the update as non-fast-forward, and preparation +stopped before the agent started. + +The backend stored one typed launch error. The frontend also derived a previous +agent error, a failed preparation row, an empty-turn warning, a generic failed +status, and a stopped-session recovery banner from the same failed start. + +## Scope + +### In scope + +- Preserve a reused local branch and prepare the exact pull-request head. +- Classify checkout preparation separately from base-branch errors. +- Offer only category-valid recovery actions. +- Render one matching launch-error card on desktop and phone. +- Keep bounded technical details in one disclosure. +- Prove the error and recovery flows with unit and Playwright tests. + +### Out of scope + +- Reconcile provider and local history after an agent starts. +- Change generic runtime-error recovery after successful agent startup. +- Add new provider-specific branch pickers. +- Change pull, push, merge, rebase, or force-push policy. + +## Technical approach + +### Pull-request ref isolation + +Update `apps/backend/internal/worktree/manager_lifecycle.go` to fetch a +pull-request head into the Kandev-owned `refs/kandev/pull//head` ref. Verify +that full ref before worktree creation. If a same-named local branch has +unrelated history, create a unique task branch from the verified ref using a +task-owned suffix and bounded collision retries. Set a source-branch upstream +only when its commit matches the verified PR start point. + +Keep the named local branch unchanged. Continue to use the existing Git +admission, timeout, and non-interactive environment. + +### Failure category and recovery + +Add `workspace_checkout_failed` and `retry_launch` to +`apps/backend/internal/task/models/launch_errors.go`. Preserve unknown-value +filtering in the task-status projection. + +Update `apps/backend/internal/orchestrator/executor/launch_failure.go` so the +failure category selects the action list. A repository row must not add branch +actions to a generic error. + +Extend `task.launch.recover` in +`apps/backend/internal/orchestrator/task_launch_recovery.go` and its WebSocket +handler. `retry_launch` must validate the current stamp and relaunch without a +base-branch write. + +### Single frontend owner + +Derive one ownership flag in `apps/web/components/task/task-chat-panel.tsx` from +the active error, session identity, and stamp. Pass this flag to the transcript +and composer boundaries. + +Update `TaskLaunchErrorEntry` with category-specific titles, cause text, a +no-change statement, and a technical-details disclosure. Keep action state and +WebSocket requests in the existing component. + +When the flag is active, suppress the matching last-agent-error notice, +preparation row, empty-turn notice, failed-agent status, stopped-session banner, +and focused-task toast. Do not suppress unrelated historical or runtime errors. + +### Mobile contract + +Desktop and phone use the task Chat surface. The phone card stacks full-width +actions. All touch actions have at least a 44-pixel hit area. + +The task transcript owns scrolling. The details disclosure wraps its bounded +content without an inner scroll region. Base-branch selection keeps the existing +`MobilePickerSheet` behavior. + +## Tests + +- `apps/backend/internal/worktree/manager_pr_head_collision_test.go` covers two + pull requests that reuse one branch name with divergent histories. +- `apps/backend/internal/orchestrator/executor/executor_launch_failure_classification_test.go` + covers category-specific actions. +- `apps/backend/internal/orchestrator/task_launch_recovery_test.go` covers + authorized, stale, and failed `retry_launch` requests. +- Frontend tests cover one-owner arbitration, the details disclosure, action + payloads, launch-error ownership in the composer, recovery-state reset, and + preservation of unrelated runtime errors. + +## E2E tests + +- `apps/web/e2e/tests/task/launch-failure-recovery.spec.ts` proves one desktop + error card, one retry action, and no matching duplicate surfaces. +- `apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts` proves the + same result on `mobile-chrome`, 44-pixel actions, and no horizontal overflow. + +These tests map to +`AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.2`, +`AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.7`, and +`AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.8`. + +## Work orders + +- [x] [Task 01: Isolate reused pull-request refs](task-01-isolate-reused-pr-refs.md) +- [x] [Task 02: Classify launch recovery actions](task-02-classify-launch-recovery-actions.md) +- [x] [Task 03: Consolidate launch-error presentation](task-03-consolidate-launch-error-presentation.md) + +## Verification results + +- Backend focused race coverage passed: 1,212 tests across worktree, task + models, status summary, and executor packages, plus 11 targeted task-launch + recovery tests. +- Frontend focused coverage passed: 10 Vitest files and 152 tests. +- Review-remediation frontend coverage passed: 7 Vitest files and 75 tests, + including task-wide launch-error visibility, composer ownership, and + recovery-state reset. +- `pnpm run typecheck`, `pnpm run lint`, `pnpm run i18n:check`, and + `pnpm run i18n:ratchet` passed. +- Managed Playwright coverage passed: desktop launch-failure recovery 3/3 and + mobile launch-failure recovery 2/2. +- The broader orchestrator package run was attempted but exceeded its timeout + in an existing SQLite/concurrency test; the focused race and recovery suites + passed independently. + +## Risks + +- A ref-selection error can discard local-only commits. Tests must prove that + the existing branch remains unchanged. +- Error events can arrive in a different order from task-status updates. The + owner logic must converge after either event order. +- Old persisted generic errors can contain branch actions. Projection and UI + normalization must not show actions that conflict with the current category. diff --git a/docs/plans/task-launch-error-consolidation/task-01-isolate-reused-pr-refs.md b/docs/plans/task-launch-error-consolidation/task-01-isolate-reused-pr-refs.md new file mode 100644 index 0000000000..6c9cb3eaeb --- /dev/null +++ b/docs/plans/task-launch-error-consolidation/task-01-isolate-reused-pr-refs.md @@ -0,0 +1,85 @@ +--- +id: "01-isolate-reused-pr-refs" +title: "Isolate reused pull-request refs" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 + - REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-001 +acceptance_criteria: + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.2 + - AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.2 +system_design: + - ../../specs/tasks/system-design/task-launch-failure-recovery.md + - ../../specs/tasks/system-design/remote-contribution-tasks.md +--- + +# Task 01: Isolate reused pull-request refs + +## Summary + +Prepare each pull-request head through a PR-scoped internal ref. Preserve any +same-named local branch that contains different history. + +## In scope + +- Fetch and verify a pull-request head without updating the named local branch. +- Create a unique task branch when the named branch has unrelated history. +- Preserve Git admission, timeout, credential, and cancellation behavior. + +## Out of scope + +- Provider-history reconciliation after launch. +- Remote branch mutation. +- Changes to non-pull-request local branch refresh. + +## Acceptance + +- Two pull requests can reuse one head-branch name without a fetch conflict. +- The new worktree starts at the requested pull-request head. +- The prior local branch and its commit remain unchanged. + +## Verification + +```bash +cd apps/backend && go test ./internal/worktree -race +``` + +## Files likely touched + +- `apps/backend/internal/worktree/manager_lifecycle.go` +- `apps/backend/internal/worktree/manager_pr_head_collision_test.go` + +## Dependencies + +None. + +## Risks + +- Ref cleanup must not remove a user-owned ref. +- A local-ahead branch must keep its local-only commits. + +## Parallelism + +`sequential` + +## Inputs + +- Task launch failure recovery design, pull-request checkout isolation. +- Remote contribution task requirement `AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.2`. +- ADR `2026-08-10-remote-contribution-head-drift`. + +## Results + +- Added a Kandev-owned `refs/kandev/pull//head` fetch ref with full-ref + verification and typed `ErrWorkspaceCheckoutFailed` failures. +- PR worktrees now create a unique local branch when the requested source name + already exists, without resetting or updating that branch. The fallback + uses a task-owned suffix and bounded retries for branch-name collisions. +- PR worktrees only track the source branch when its remote-tracking commit + matches the verified PR head. Ordinary `fetch --all --prune` cannot remove + or replace the internal snapshot. +- Recreate restores deleted PR branches from the isolated ref. +- Verification passed: `go test ./internal/worktree` (368 passed). diff --git a/docs/plans/task-launch-error-consolidation/task-02-classify-launch-recovery-actions.md b/docs/plans/task-launch-error-consolidation/task-02-classify-launch-recovery-actions.md new file mode 100644 index 0000000000..6477a0aae6 --- /dev/null +++ b/docs/plans/task-launch-error-consolidation/task-02-classify-launch-recovery-actions.md @@ -0,0 +1,94 @@ +--- +id: "02-classify-launch-recovery-actions" +title: "Classify launch recovery actions" +status: completed +wave: 2 +depends_on: + - "01-isolate-reused-pr-refs" +plan: "plan.md" +requirements: + - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 +acceptance_criteria: + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.2 + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.4 + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.8 +system_design: + - ../../specs/tasks/system-design/task-launch-failure-recovery.md +--- + +# Task 02: Classify launch recovery actions + +## Summary + +Give checkout preparation a stable error category. Derive recovery actions from +the category and add a stamp-checked launch retry. + +## In scope + +- Add `workspace_checkout_failed` and `retry_launch` wire values. +- Map checkout preparation errors to the new category. +- Make every action list category-specific. +- Relaunch without a base-branch write for `retry_launch`. +- Keep authorization and current-stamp checks before relaunch. + +## Out of scope + +- New persistence tables. +- Automatic retries. +- Changes to `session.recover`. + +## Acceptance + +- A checkout error offers only `retry_launch`. +- A generic error does not offer base-branch actions because a repository row exists. +- A stale or foreign retry request makes no change and starts no session. + +## Verification + +```bash +cd apps/backend && go test ./internal/task/models ./internal/orchestrator/executor ./internal/orchestrator/... -race +``` + +## Files likely touched + +- `apps/backend/internal/worktree/errors.go` +- `apps/backend/internal/task/models/launch_errors.go` +- `apps/backend/internal/task/models/launch_errors_test.go` +- `apps/backend/internal/orchestrator/executor/launch_failure.go` +- `apps/backend/internal/orchestrator/executor/executor_launch_failure_classification_test.go` +- `apps/backend/internal/orchestrator/task_launch_recovery.go` +- `apps/backend/internal/orchestrator/task_launch_recovery_test.go` +- `apps/backend/internal/orchestrator/handlers/handlers.go` + +## Dependencies + +Task 01. + +## Risks + +- Old persisted action arrays can conflict with the new category matrix. +- A retry must not clear the current error before launch succeeds. + +## Parallelism + +`sequential` + +## Inputs + +- Task launch failure recovery design, error model and recovery contract. +- Existing task-status projection normalization tests. + +## Results + +- Added the `workspace_checkout_failed` category and `retry_launch` action. +- Checkout failures now use category-specific recovery actions, including + retry without changing a task repository base branch. +- Task-scoped recovery validates the current error stamp and repository + identity before relaunch or mutation. +- Persisted and projected errors normalize action lists by category and keep + bounded technical details. +- Focused race coverage passed: 1,212 tests across worktree, task models, + status summary, and executor packages. The 11 targeted task-launch recovery + tests also passed. +- The broader orchestrator package run was attempted but exceeded its timeout + in an existing SQLite/concurrency test. diff --git a/docs/plans/task-launch-error-consolidation/task-03-consolidate-launch-error-presentation.md b/docs/plans/task-launch-error-consolidation/task-03-consolidate-launch-error-presentation.md new file mode 100644 index 0000000000..fe50a79de8 --- /dev/null +++ b/docs/plans/task-launch-error-consolidation/task-03-consolidate-launch-error-presentation.md @@ -0,0 +1,108 @@ +--- +id: "03-consolidate-launch-error-presentation" +title: "Consolidate launch-error presentation" +status: completed +wave: 3 +depends_on: + - "02-classify-launch-recovery-actions" +plan: "plan.md" +requirements: + - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 +acceptance_criteria: + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.2 + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.7 + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.8 +system_design: + - ../../specs/tasks/system-design/task-launch-failure-recovery.md +--- + +# Task 03: Consolidate launch-error presentation + +## Summary + +Make the task launch card the only primary presentation for its matching error. +Give desktop and phone users the same cause, details, and valid recovery. + +## In scope + +- Derive separate launch-card visibility and session-surface ownership from + session identity and error stamp. +- Add clear category text, a no-change statement, and a details disclosure. +- Suppress matching duplicate notices, rows, banners, empty-turn copy, and toasts. +- Preserve unrelated historical and runtime errors, including stale + launch-only transcript rows. +- Add all required locale values. +- Add desktop and mobile Playwright coverage. + +## Out of scope + +- Redesign completed-session UI. +- Change non-launch runtime-error recovery. +- Change the mobile branch picker. + +## Acceptance + +- One failed start renders one error card after all events converge. +- Recovery failure updates the same card without another error surface. +- Phone actions remain reachable, at least 44 pixels high, and free of horizontal overflow. + +## Verification + +```bash +cd apps && pnpm install --frozen-lockfile +cd apps/web && pnpm exec vitest run components/task/simple/components/task-launch-error-entry.test.tsx components/task/chat/chat-input-container.test.tsx components/task/chat/message-list-footer.test.tsx components/task/chat/session-stopped-banner.test.tsx components/task/chat/stopped-banner-props.test.ts components/task/chat/types.test.ts lib/ws/handlers/empty-turn-notice.test.ts +cd apps/web && pnpm run typecheck && pnpm run i18n:check && pnpm run i18n:ratchet +cd apps/web && pnpm e2e:run tests/task/launch-failure-recovery.spec.ts +cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-launch-failure-recovery.spec.ts +``` + +## Files likely touched + +- `apps/web/components/task/task-chat-panel.tsx` +- `apps/web/components/task/task-launch-error-context.tsx` +- `apps/web/components/task/simple/components/task-launch-error-entry.tsx` +- `apps/web/components/task/chat/message-list-shared.tsx` +- `apps/web/components/task/chat/message-list-footer.tsx` +- `apps/web/components/task/chat/chat-input-container.tsx` +- `apps/web/lib/ws/handlers/empty-turn-notice.ts` +- Focused tests beside these files. +- `apps/web/src/locales/*/task.json` +- `apps/web/e2e/tests/task/launch-failure-recovery.spec.ts` +- `apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts` + +## Dependencies + +Task 02. + +## Risks + +- Event order can briefly show a derived error before the typed record arrives. +- Broad suppression can hide a later agent runtime error. + +## Parallelism + +`sequential` + +## Inputs + +- Task launch failure recovery design, presentation ownership and responsive sections. +- Mobile exemplar `apps/web/components/task/mobile/mobile-picker-sheet.tsx`. +- Existing desktop and mobile launch-failure tests. + +## Results + +- The typed task launch card is the single owner for its matching session and + error stamp. Matching preparation, empty-turn, agent-error, failed-status, + stopped-session, and toast surfaces are suppressed without hiding unrelated + runtime errors. +- The card now shows category-specific copy, bounded technical details, a + no-change statement, inline recovery failures, and mobile-sized stacked + actions. +- Added complete locale values and generated the pseudo-locale catalog. +- Focused frontend coverage passed: 10 Vitest files and 152 tests. +- `pnpm run typecheck`, `pnpm run lint`, `pnpm run i18n:check`, and + `pnpm run i18n:ratchet` passed. +- Managed Playwright coverage passed: desktop launch-failure recovery 3/3 and + mobile launch-failure recovery 2/2. +- Review-remediation coverage passed: 7 focused Vitest files and 75 tests, + including composer ownership and launch-error recovery-state reset. diff --git a/docs/specs/tasks/system-design/task-launch-failure-recovery.md b/docs/specs/tasks/system-design/task-launch-failure-recovery.md index c16ad0dd31..2b2de7c933 100644 --- a/docs/specs/tasks/system-design/task-launch-failure-recovery.md +++ b/docs/specs/tasks/system-design/task-launch-failure-recovery.md @@ -4,7 +4,7 @@ system: tasks requirements: - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 created: 2026-08-24 -updated: 2026-09-03 +updated: 2026-09-06 owners: - cfl12 --- @@ -45,8 +45,20 @@ actions, exact task-repository identity when applicable, and an idempotency stamp. The supported categories are `base_branch_missing`, `pr_already_closed`, -`default_branch_unresolved`, and `generic_launch_failure`. The supported -actions are `retry_default`, `pick_base_branch`, and `mark_review_done`. +`default_branch_unresolved`, `workspace_checkout_failed`, and +`generic_launch_failure`. The supported actions are `retry_launch`, +`retry_default`, `pick_base_branch`, and `mark_review_done`. + +The failure category controls the available actions. A repository identity +alone does not make a base-branch action valid. + +| Category | Valid actions | +| --- | --- | +| `base_branch_missing` | `retry_default`, `pick_base_branch` | +| `default_branch_unresolved` | `pick_base_branch` | +| `workspace_checkout_failed` | `retry_launch` | +| `pr_already_closed` | `mark_review_done` when the workflow permits it | +| `generic_launch_failure` | `retry_launch` | The `TaskStatusSummary.active_error` projection selects the newest active record, limits strings and actions, removes duplicate actions without changing @@ -64,6 +76,8 @@ the current error stamp; stale stamps fail without mutation. relaunches. - `pick_base_branch` validates and persists one selected branch before relaunch. +- `retry_launch` keeps the task-repository settings unchanged and repeats the + failed launch. It still requires the current error stamp. - `mark_review_done` is allowed only for a valid terminal workflow step and when every relevant PR is terminal; it uses the normal task-move service. @@ -106,6 +120,32 @@ but `retry_default` and `pick_base_branch` must write the resolved base to the exact `task_repositories` row and that write must succeed before relaunch. No new table is required. +## Pull-request checkout isolation + +A pull-request head uses a Kandev-owned ref that includes the pull-request +number, specifically `refs/kandev/pull//head`. The fetch can force-update +this internal ref because users do not own it. Ordinary remote refresh and +pruning do not manage this namespace. The fetch never writes directly to a +user-named local branch. + +The worktree manager verifies the fetched ref before it selects a start point. +If the named local branch has unrelated history, the manager preserves that +branch. It creates a unique task branch from the verified pull-request ref. + +This rule also applies when two pull requests reuse the same source-branch +name. A local branch from the first pull request cannot block the second pull +request. A fallback branch uses a task-owned deterministic suffix and a +bounded retry sequence, so an existing fallback branch cannot make launch fail +because of one random-name collision. + +The manager sets `origin/` as upstream only when that ref points +to the verified pull-request start point. A remote branch with different +history is never attached as the worktree upstream. + +If Kandev cannot fetch or verify the pull-request ref, preparation fails with a +typed `workspace_checkout_failed` error. The error retains the existing +credential and path redaction rules. + ## Failure and security PR lookup failures launch normally. Remote-default timeout, authentication, @@ -117,12 +157,49 @@ Initial-prompt errors use a safe generic durable message and the same stale-event checks as other agent failures. Raw attachment paths and provider details remain in backend diagnostics and do not reach the durable projection. +## Launch-error presentation ownership + +The active `TaskStatusSummary.active_error` record owns the primary launch +error for its matching session and stamp. A task-wide record remains visible +while a prior session is selected, but it owns session surfaces only when no +session is selected. A session-owned record owns surfaces only for its exact +session and stamp. The task Chat surface renders one persistent card from this +record. + +The matching launch error suppresses these secondary presentations: + +- The previous-agent-error notice. +- The standalone failed preparation row. +- The empty-turn warning. +- The generic failed-agent status row. +- The stopped-session recovery banner. +- A launch-error toast while the user views the affected task. + +Only synthetic rows generated by the active launch failure are suppressed; +stale stamped or historical launch-only rows remain visible. An unrelated +historical error keeps its normal transcript presentation. A runtime error that +occurs after agent startup keeps the existing session +recovery surface. + +The card shows a category title, a short cause, a no-change statement, and only +valid recovery actions. One disclosure shows bounded technical details. The +disclosure does not show raw Git output, credentials, or local file paths. + +A failed recovery updates the same card. It does not add another banner or +toast. A successful recovery removes the card after launch succeeds. + ## Responsive presentation -The task Chat surface renders one persistent error card from the shared -projection. Desktop actions are inline; mobile actions wrap and branch -selection uses the existing mobile picker. Both surfaces use the same action -authorization and remain free of horizontal overflow. +The task Chat surface is the entry point on desktop and phone. The card remains +inline because the error and its actions belong to the current task. + +Desktop actions use one compact row. Phone actions use a vertical layout with +44-pixel touch targets. The existing mobile picker remains the branch-selection +surface for `base_branch_missing` and `default_branch_unresolved`. + +The transcript owns vertical scrolling. Expanded technical details wrap inside +the card and do not create a second scroll region. Both layouts prevent +document-level horizontal overflow. `useEnsureTaskSession` keeps one request latch after an error. Store updates and loader identity changes do not start another request. The Retry control starts