diff --git a/apps/backend/internal/backendapp/adapters.go b/apps/backend/internal/backendapp/adapters.go index e333e63815..4cc10cc268 100644 --- a/apps/backend/internal/backendapp/adapters.go +++ b/apps/backend/internal/backendapp/adapters.go @@ -852,6 +852,43 @@ func (a githubTaskIssueStoreAdapter) UpdateTaskMetadata(ctx context.Context, tas return task, nil } +func (a githubTaskIssueStoreAdapter) UpdateTaskRepositoryBaseBranch( + ctx context.Context, taskID, repositoryID, headBranch, baseBranch string, +) error { + taskRepos, err := a.ListTaskRepositories(ctx, taskID) + if err != nil { + return err + } + taskRepo, err := selectTaskRepositoryForPR(taskRepos, repositoryID, headBranch) + if err != nil { + return err + } + _, err = a.svc.UpdateRepositoryBaseBranch(ctx, taskservice.UpdateRepositoryBaseBranchRequest{ + TaskID: taskID, TaskRepositoryID: taskRepo.ID, BaseBranch: baseBranch, + }) + return err +} + +func selectTaskRepositoryForPR( + taskRepos []*models.TaskRepository, repositoryID, headBranch string, +) (*models.TaskRepository, error) { + matches := make([]*models.TaskRepository, 0, 1) + for _, taskRepo := range taskRepos { + if taskRepo != nil && taskRepo.RepositoryID == repositoryID { + matches = append(matches, taskRepo) + } + } + if len(matches) == 1 { + return matches[0], nil + } + for _, taskRepo := range matches { + if taskRepo.CheckoutBranch == headBranch { + return taskRepo, nil + } + } + return nil, fmt.Errorf("task repository for repository %q and branch %q not found", repositoryID, headBranch) +} + func wrapGitHubTaskIssueStoreError(err error) error { if errors.Is(err, taskrepo.ErrTaskNotFound) { return fmt.Errorf("%w: %w", githubsvc.ErrTaskNotFound, err) diff --git a/apps/backend/internal/backendapp/adapters_github_pr_base_test.go b/apps/backend/internal/backendapp/adapters_github_pr_base_test.go new file mode 100644 index 0000000000..343f0d3b21 --- /dev/null +++ b/apps/backend/internal/backendapp/adapters_github_pr_base_test.go @@ -0,0 +1,23 @@ +package backendapp + +import ( + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestSelectTaskRepositoryForPRDisambiguatesByHeadBranch(t *testing.T) { + taskRepos := []*models.TaskRepository{ + {ID: "task-repo-parent", RepositoryID: "repo-1", CheckoutBranch: "feature/parent"}, + {ID: "task-repo-child", RepositoryID: "repo-1", CheckoutBranch: "feature/child"}, + {ID: "task-repo-other", RepositoryID: "repo-2", CheckoutBranch: "feature/child"}, + } + + got, err := selectTaskRepositoryForPR(taskRepos, "repo-1", "feature/child") + if err != nil { + t.Fatalf("selectTaskRepositoryForPR() error: %v", err) + } + if got.ID != "task-repo-child" { + t.Fatalf("task repository ID = %q, want task-repo-child", got.ID) + } +} diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index e01b401f7b..a376a77951 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -669,7 +669,9 @@ func startAgentInfrastructure( if services.GitHub != nil { orchestratorSvc.SetGitHubService(services.GitHub) services.GitHub.SetTaskDeleter(&taskDeleterAdapter{svc: services.Task}) - services.GitHub.SetTaskIssueStore(githubTaskIssueStoreAdapter{svc: services.Task}) + taskStoreAdapter := githubTaskIssueStoreAdapter{svc: services.Task} + services.GitHub.SetTaskIssueStore(taskStoreAdapter) + services.GitHub.SetTaskRepositoryBaseBranchUpdater(taskStoreAdapter) services.GitHub.SetTaskSessionChecker(&taskSessionCheckerAdapter{repo: repos.Task}) log.Info("GitHub service configured for orchestrator (PR auto-detection enabled)") diff --git a/apps/backend/internal/backendapp/orchestrator.go b/apps/backend/internal/backendapp/orchestrator.go index 9f5a6ccdee..7ee4686ea3 100644 --- a/apps/backend/internal/backendapp/orchestrator.go +++ b/apps/backend/internal/backendapp/orchestrator.go @@ -131,6 +131,7 @@ func provideOrchestrator( orchestratorSvc.SetTitleBranchRuntime(lifecycleMgr) if githubSvc != nil { orchestratorSvc.SetTaskGitCredentialPolicyResolver(githubExecutorCredentialPolicyAdapter{service: githubSvc}) + orchestratorSvc.SetPRBaseResolver(githubPRBaseResolver{service: githubSvc}) } taskSvc.SetExecutionStopper(orchestratorSvc) // Runtime-aware liveness lets durable cleanup treat a not-found stop for a @@ -258,6 +259,20 @@ type githubExecutorCredentialPolicyAdapter struct { service githubCredentialPolicyService } +type githubPRBaseResolver struct { + service *githubpkg.Service +} + +func (r githubPRBaseResolver) ResolvePRBaseBranch( + ctx context.Context, workspaceID, owner, repo string, number int, +) (string, error) { + pr, err := r.service.GetPRForAutomation(ctx, workspaceID, owner, repo, number) + if err != nil || pr == nil { + return "", err + } + return pr.BaseBranch, nil +} + func (a githubExecutorCredentialPolicyAdapter) ResolveTaskGitCredentialPolicy( ctx context.Context, workspaceID string, diff --git a/apps/backend/internal/github/service.go b/apps/backend/internal/github/service.go index dd987ac46a..7995671ae1 100644 --- a/apps/backend/internal/github/service.go +++ b/apps/backend/internal/github/service.go @@ -94,6 +94,13 @@ type PromptResolver interface { ResolvePromptContent(ctx context.Context, name, fallback string) string } +// TaskRepositoryBaseBranchUpdater projects provider base changes into tasks. +type TaskRepositoryBaseBranchUpdater interface { + UpdateTaskRepositoryBaseBranch( + ctx context.Context, taskID, repositoryID, headBranch, baseBranch string, + ) error +} + // Service coordinates GitHub integration operations. type Service struct { mu sync.Mutex @@ -112,6 +119,7 @@ type Service struct { eventBus bus.EventBus logger *logger.Logger taskDeleter TaskDeleter + taskRepositoryUpdater TaskRepositoryBaseBranchUpdater comparisonTargetObserver ComparisonTargetObserver taskIssueStore TaskIssueStore // cascadeTaskDeleter is the cascade-delete entry point used by the @@ -277,6 +285,11 @@ func (s *Service) newPATClient(token string) *PATClient { // SetTaskDeleter sets the task deletion dependency for cleanup operations. func (s *Service) SetTaskDeleter(d TaskDeleter) { s.taskDeleter = d } +// SetTaskRepositoryBaseBranchUpdater wires best-effort task base projection. +func (s *Service) SetTaskRepositoryBaseBranchUpdater(updater TaskRepositoryBaseBranchUpdater) { + s.taskRepositoryUpdater = updater +} + // SetCascadeTaskDeleter wires the cascade-delete dependency used by the // watch reset flow (ResetIssueWatch / ResetReviewWatch). Optional — when // unset, reset returns an error so the missing wiring is surfaced instead diff --git a/apps/backend/internal/github/service_pr.go b/apps/backend/internal/github/service_pr.go index 9ddec37360..7a570b7341 100644 --- a/apps/backend/internal/github/service_pr.go +++ b/apps/backend/internal/github/service_pr.go @@ -275,6 +275,22 @@ func (s *Service) GetPRForWorkspace( return resolved.Client.GetPR(ctx, owner, repo, number) } +// GetPRForAutomation fetches pull-request details through the workspace-owned +// automation credential. It is used by background launch paths that have a +// repository workspace but no interactive user identity. +func (s *Service) GetPRForAutomation( + ctx context.Context, workspaceID, owner, repo string, number int, +) (*PR, error) { + if err := s.ensureRepositoryInWorkspaceScope(ctx, workspaceID, owner, repo); err != nil { + return nil, err + } + resolved, err := s.resolveAutomationClient(ctx, workspaceID, owner, repo) + if err != nil { + return nil, err + } + return resolved.Client.GetPR(ctx, owner, repo, number) +} + // GetIssue fetches basic issue details from GitHub. The create-task dialog is // currently the only caller and dedupes requests per URL on the frontend. func (s *Service) GetIssue(ctx context.Context, owner, repo string, number int) (*Issue, error) { diff --git a/apps/backend/internal/github/service_pr_base_propagation_test.go b/apps/backend/internal/github/service_pr_base_propagation_test.go new file mode 100644 index 0000000000..e37d9df0e6 --- /dev/null +++ b/apps/backend/internal/github/service_pr_base_propagation_test.go @@ -0,0 +1,116 @@ +package github + +import ( + "context" + "errors" + "testing" +) + +type recordingTaskRepositoryUpdater struct { + err error + calls []taskRepositoryUpdateCall +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.12 +func TestSyncTaskPR_DoesNotPropagateUnchangedBase(t *testing.T) { + svc, store, _ := setupSyncTest(t) + ctx := context.Background() + updater := &recordingTaskRepositoryUpdater{} + svc.SetTaskRepositoryBaseBranchUpdater(updater) + if err := store.CreateTaskPR(ctx, &TaskPR{ + TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "widgets", PRNumber: 42, + HeadBranch: "feature/child", BaseBranch: "main", State: "open", + }); err != nil { + t.Fatalf("CreateTaskPR() error: %v", err) + } + + if err := svc.SyncTaskPR(ctx, "task-1", &PRStatus{PR: &PR{ + Number: 42, RepoOwner: "acme", RepoName: "widgets", HeadBranch: "feature/child", BaseBranch: "main", State: "open", + }}); err != nil { + t.Fatalf("SyncTaskPR() error: %v", err) + } + if len(updater.calls) != 0 { + t.Fatalf("updater calls = %#v, want none", updater.calls) + } +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.12 +func TestSyncTaskPR_TaskRepositoryUpdateFailureIsBestEffort(t *testing.T) { + svc, store, _ := setupSyncTest(t) + ctx := context.Background() + updater := &recordingTaskRepositoryUpdater{err: errors.New("task store unavailable")} + svc.SetTaskRepositoryBaseBranchUpdater(updater) + if err := store.CreateTaskPR(ctx, &TaskPR{ + TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "widgets", PRNumber: 42, + HeadBranch: "feature/child", BaseBranch: "feature/parent", State: "open", + }); err != nil { + t.Fatalf("CreateTaskPR() error: %v", err) + } + + if err := svc.SyncTaskPR(ctx, "task-1", &PRStatus{PR: &PR{ + Number: 42, RepoOwner: "acme", RepoName: "widgets", HeadBranch: "feature/child", BaseBranch: "main", State: "open", + }}); err != nil { + t.Fatalf("SyncTaskPR() error: %v", err) + } + if len(updater.calls) != 1 { + t.Fatalf("updater calls = %#v, want one best-effort attempt", updater.calls) + } + got, err := svc.GetTaskPRByOwnerRepoNumber(ctx, "task-1", "acme", "widgets", 42) + if err != nil { + t.Fatalf("GetTaskPRByOwnerRepoNumber() error: %v", err) + } + if got == nil || got.BaseBranch != "main" { + t.Fatalf("persisted TaskPR = %#v, want base main", got) + } +} + +type taskRepositoryUpdateCall struct { + taskID, repositoryID, headBranch, baseBranch string +} + +func (u *recordingTaskRepositoryUpdater) UpdateTaskRepositoryBaseBranch( + _ context.Context, taskID, repositoryID, headBranch, baseBranch string, +) error { + u.calls = append(u.calls, taskRepositoryUpdateCall{ + taskID: taskID, repositoryID: repositoryID, headBranch: headBranch, baseBranch: baseBranch, + }) + return u.err +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.12 +func TestSyncTaskPR_PropagatesChangedBaseToTaskRepository(t *testing.T) { + svc, store, _ := setupSyncTest(t) + ctx := context.Background() + updater := &recordingTaskRepositoryUpdater{} + svc.SetTaskRepositoryBaseBranchUpdater(updater) + if err := store.CreateTaskPR(ctx, &TaskPR{ + TaskID: "task-1", + RepositoryID: "repo-1", + Owner: "acme", + Repo: "widgets", + PRNumber: 42, + HeadBranch: "feature/stacked-child", + BaseBranch: "feature/deleted-parent", + State: "open", + }); err != nil { + t.Fatalf("CreateTaskPR() error: %v", err) + } + + err := svc.SyncTaskPR(ctx, "task-1", &PRStatus{PR: &PR{ + Number: 42, + RepoOwner: "acme", + RepoName: "widgets", + HeadBranch: "feature/stacked-child", + BaseBranch: "main", + State: "open", + }}) + if err != nil { + t.Fatalf("SyncTaskPR() error: %v", err) + } + want := taskRepositoryUpdateCall{ + taskID: "task-1", repositoryID: "repo-1", headBranch: "feature/stacked-child", baseBranch: "main", + } + if len(updater.calls) != 1 || updater.calls[0] != want { + t.Fatalf("updater calls = %#v, want %#v", updater.calls, want) + } +} diff --git a/apps/backend/internal/github/service_pr_outcome_sync_test.go b/apps/backend/internal/github/service_pr_outcome_sync_test.go index 3ecfc89d61..fca9677c5f 100644 --- a/apps/backend/internal/github/service_pr_outcome_sync_test.go +++ b/apps/backend/internal/github/service_pr_outcome_sync_test.go @@ -338,7 +338,7 @@ func TestPersistAndPublishTaskPRSync_PublishesReReadValueNotStaleInMemoryOne(t * staleObservedAt := persistedAt.Add(-time.Minute) staleTP.AutoMergeObservedAt = &staleObservedAt - if err := svc.persistAndPublishTaskPRSync(ctx, staleTP.TaskID, nil, &staleTP, true, true); err != nil { + if err := svc.persistAndPublishTaskPRSync(ctx, staleTP.TaskID, nil, &staleTP, true, true, false); err != nil { t.Fatalf("persistAndPublishTaskPRSync: %v", err) } diff --git a/apps/backend/internal/github/service_pr_unwatched.go b/apps/backend/internal/github/service_pr_unwatched.go index 4accd0078b..7ef11cd268 100644 --- a/apps/backend/internal/github/service_pr_unwatched.go +++ b/apps/backend/internal/github/service_pr_unwatched.go @@ -158,7 +158,7 @@ func (s *Service) reconcileTaskPRLifecycle(ctx context.Context, tp *TaskPR, stat // this call's own resolved value. persistAndPublishTaskPRSync already // re-reads before publishing for exactly this reason (codex [P2] on the // SyncTaskPR path); reuse it here instead of duplicating the write. - return s.persistAndPublishTaskPRSync(ctx, tp.TaskID, status.PR, tp, changed, status.OutcomeFieldsPopulated) + return s.persistAndPublishTaskPRSync(ctx, tp.TaskID, status.PR, tp, changed, status.OutcomeFieldsPopulated, false) } // fetchUnwatchedTaskPRs prefers the batched GraphQL query — one call for the diff --git a/apps/backend/internal/github/service_pr_watch.go b/apps/backend/internal/github/service_pr_watch.go index 0b10b2106e..26ade20c19 100644 --- a/apps/backend/internal/github/service_pr_watch.go +++ b/apps/backend/internal/github/service_pr_watch.go @@ -1046,6 +1046,7 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu return err } next := s.prepareTaskPRSyncState(ctx, tp, status) + baseBranchChanged := tp.BaseBranch != next.baseBranch && strings.TrimSpace(next.baseBranch) != "" changedFields := taskPRChangedFields(tp, status, next) changed := len(changedFields) > 0 @@ -1094,7 +1095,25 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu ) } - return s.persistAndPublishTaskPRSync(ctx, taskID, status.PR, tp, changed, status.OutcomeFieldsPopulated) + return s.persistAndPublishTaskPRSync( + ctx, taskID, status.PR, tp, changed, status.OutcomeFieldsPopulated, baseBranchChanged, + ) +} + +func (s *Service) propagateTaskRepositoryBaseBranch(ctx context.Context, tp *TaskPR, changed bool) { + if !changed || s.taskRepositoryUpdater == nil || tp == nil { + return + } + if err := s.taskRepositoryUpdater.UpdateTaskRepositoryBaseBranch( + ctx, tp.TaskID, tp.RepositoryID, tp.HeadBranch, tp.BaseBranch, + ); err != nil && s.logger != nil { + s.logger.Warn("failed to propagate pull request base branch to task repository", + zap.String("task_id", tp.TaskID), + zap.String("repository_id", tp.RepositoryID), + zap.Int("pr_number", tp.PRNumber), + zap.String("base_branch", tp.BaseBranch), + zap.Error(err)) + } } // persistAndPublishTaskPRSync writes the reconciled sync state and, on @@ -1110,7 +1129,8 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu // SyncTaskPR to keep that function within the repo's complexity limits and // to make the re-read-before-publish behavior directly testable. func (s *Service) persistAndPublishTaskPRSync( - ctx context.Context, taskID string, pr *PR, tp *TaskPR, changed, outcomeFieldsPopulated bool, + ctx context.Context, taskID string, pr *PR, tp *TaskPR, + changed, outcomeFieldsPopulated, baseBranchChanged bool, ) error { // AC-38/AC-18c: the counter fires at the populated-ness decision point, // before the write is attempted, and survives write failure — it @@ -1120,6 +1140,7 @@ func (s *Service) persistAndPublishTaskPRSync( if err := s.store.UpdateTaskPR(ctx, tp); err != nil { return fmt.Errorf("update task PR: %w", err) } + s.propagateTaskRepositoryBaseBranch(ctx, tp, baseBranchChanged) // Provider payloads carry the authoritative head/base repository identity // and branch. Reconcile after the TaskPR write so a malformed or // unmatchable payload never prevents the review association from persisting. diff --git a/apps/backend/internal/github/service_workspace_authorization_test.go b/apps/backend/internal/github/service_workspace_authorization_test.go index 27f883136f..0bacf6add7 100644 --- a/apps/backend/internal/github/service_workspace_authorization_test.go +++ b/apps/backend/internal/github/service_workspace_authorization_test.go @@ -190,6 +190,25 @@ func TestGetPRForWorkspaceWithoutConnectionUsesAnonymousClient(t *testing.T) { } } +func TestGetPRForAutomationUsesWorkspaceCredential(t *testing.T) { + ambient := &stubClient{getPRFunc: func(context.Context, string, string, int) (*PR, error) { + return &PR{Title: "ambient credential"}, nil + }} + workspace := &stubClient{getPRFunc: func(context.Context, string, string, int) (*PR, error) { + return &PR{Title: "workspace credential", BaseBranch: "main"}, nil + }} + svc := NewService(ambient, AuthMethodPAT, nil, nil, nil, testLogger(t)) + configureTestWorkspaceAuth(t, svc, workspace, "workspace-1") + + pr, err := svc.GetPRForAutomation(context.Background(), "workspace-1", "acme", "widgets", 42) + if err != nil { + t.Fatalf("GetPRForAutomation() error: %v", err) + } + if pr.Title != "workspace credential" { + t.Fatalf("PR title = %q, want workspace credential", pr.Title) + } +} + func TestListAllReviewWatchesRetainsIdentitylessInternalUse(t *testing.T) { store := newTestStore(t) svc := NewService(nil, AuthMethodNone, nil, store, nil, testLogger(t)) diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 1f18b69818..e2f7fb9659 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -854,6 +854,7 @@ type Executor struct { repoCloner RepoCloner repoUpdater RepoUpdater taskRepositoryBaseBranchUpdater TaskRepositoryBaseBranchUpdater + prBaseResolver PRBaseResolver } // taskEnvLock returns the per-task mutex for env persistence, creating one on @@ -1012,6 +1013,11 @@ type TaskRepositoryBaseBranchUpdater interface { UpdateTaskRepositoryBaseBranch(ctx context.Context, taskID, taskRepositoryID, baseBranch string) error } +// PRBaseResolver returns the current base branch for one provider pull request. +type PRBaseResolver interface { + ResolvePRBaseBranch(ctx context.Context, workspaceID, owner, repo string, number int) (string, error) +} + // ExecutorConfig holds configuration for the Executor type ExecutorConfig struct { ShellPrefs ShellPreferenceProvider @@ -1106,6 +1112,11 @@ func (e *Executor) SetTaskRepositoryBaseBranchUpdater(updater TaskRepositoryBase e.taskRepositoryBaseBranchUpdater = updater } +// SetPRBaseResolver wires best-effort pull-request base resolution at launch. +func (e *Executor) SetPRBaseResolver(resolver PRBaseResolver) { + e.prBaseResolver = resolver +} + // SetOnAgentStartFailed sets a callback for agent process start failures. // This allows the orchestrator to intercept auth errors and treat them as // recoverable instead of terminal failures. diff --git a/apps/backend/internal/orchestrator/executor/executor_pr_base_resolver_test.go b/apps/backend/internal/orchestrator/executor/executor_pr_base_resolver_test.go new file mode 100644 index 0000000000..c0503cc986 --- /dev/null +++ b/apps/backend/internal/orchestrator/executor/executor_pr_base_resolver_test.go @@ -0,0 +1,197 @@ +package executor + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +type recordingPRBaseResolver struct { + base string + err error + calls []prBaseResolveCall +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11 +func TestResolveTaskRepoInfo_PRBaseLookupFailureKeepsStoredBase(t *testing.T) { + repo := newMockRepository() + repo.repositories["repo-1"] = &models.Repository{ + ID: "repo-1", + WorkspaceID: "workspace-1", + SourceType: sourceTypeLocal, + LocalPath: t.TempDir(), + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + DefaultBranch: "main", + } + resolver := &recordingPRBaseResolver{err: errors.New("GitHub unavailable")} + exec := newTestExecutor(t, &mockAgentManager{}, repo) + exec.SetPRBaseResolver(resolver) + + info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{ + ID: "task-repo-1", + TaskID: "task-1", + RepositoryID: "repo-1", + BaseBranch: "feature/stored-base", + Metadata: map[string]interface{}{"pr_number": 42}, + }) + if err != nil { + t.Fatalf("resolveTaskRepoInfo() error: %v", err) + } + if info.BaseBranch != "feature/stored-base" { + t.Fatalf("BaseBranch = %q, want stored base", info.BaseBranch) + } +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11 +func TestResolveTaskRepoInfo_SkipsPRBaseLookupWithoutGitHubPR(t *testing.T) { + tests := []struct { + name string + provider string + metadata map[string]interface{} + }{ + {name: "no PR number", provider: "github"}, + {name: "non GitHub provider", provider: "gitlab", metadata: map[string]interface{}{"pr_number": 42}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := newMockRepository() + repo.repositories["repo-1"] = &models.Repository{ + ID: "repo-1", + WorkspaceID: "workspace-1", + SourceType: sourceTypeLocal, + LocalPath: t.TempDir(), + Provider: tt.provider, + ProviderOwner: "acme", + ProviderName: "widgets", + DefaultBranch: "main", + } + resolver := &recordingPRBaseResolver{base: "main"} + exec := newTestExecutor(t, &mockAgentManager{}, repo) + exec.SetPRBaseResolver(resolver) + + info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{ + ID: "task-repo-1", + TaskID: "task-1", + RepositoryID: "repo-1", + BaseBranch: "feature/stored-base", + Metadata: tt.metadata, + }) + if err != nil { + t.Fatalf("resolveTaskRepoInfo() error: %v", err) + } + if info.BaseBranch != "feature/stored-base" { + t.Fatalf("BaseBranch = %q, want stored base", info.BaseBranch) + } + if len(resolver.calls) != 0 { + t.Fatalf("resolver calls = %#v, want none", resolver.calls) + } + }) + } +} + +type prBaseResolveCall struct { + workspaceID, owner, repo string + number int +} + +func (r *recordingPRBaseResolver) ResolvePRBaseBranch( + _ context.Context, workspaceID, owner, repo string, number int, +) (string, error) { + r.calls = append(r.calls, prBaseResolveCall{workspaceID: workspaceID, owner: owner, repo: repo, number: number}) + return r.base, r.err +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11 +func TestResolveTaskRepoInfo_UsesLivePRBase(t *testing.T) { + repo := newMockRepository() + repo.repositories["repo-1"] = &models.Repository{ + ID: "repo-1", + WorkspaceID: "workspace-1", + SourceType: sourceTypeLocal, + LocalPath: t.TempDir(), + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + DefaultBranch: "main", + } + resolver := &recordingPRBaseResolver{base: "main"} + exec := newTestExecutor(t, &mockAgentManager{}, repo) + exec.SetPRBaseResolver(resolver) + + info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{ + ID: "task-repo-1", + TaskID: "task-1", + RepositoryID: "repo-1", + BaseBranch: "feature/deleted-parent", + CheckoutBranch: "feature/stacked-child", + Metadata: map[string]interface{}{"pr_number": float64(42)}, + }) + if err != nil { + t.Fatalf("resolveTaskRepoInfo() error: %v", err) + } + if info.BaseBranch != "main" { + t.Fatalf("BaseBranch = %q, want live base main", info.BaseBranch) + } + if len(resolver.calls) != 1 || resolver.calls[0] != (prBaseResolveCall{workspaceID: "workspace-1", owner: "acme", repo: "widgets", number: 42}) { + t.Fatalf("resolver calls = %#v, want acme/widgets#42", resolver.calls) + } +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11 +func TestResolveTaskRepoInfo_ReconcilesRetargetedRemoteContributionBase(t *testing.T) { + repo := newMockRepository() + repo.repositories["repo-1"] = &models.Repository{ + ID: "repo-1", + WorkspaceID: "workspace-1", + SourceType: sourceTypeLocal, + LocalPath: t.TempDir(), + Provider: "github", + ProviderOwner: "acme", + ProviderName: "widgets", + DefaultBranch: "main", + } + binding := &models.RemoteContribution{ + Version: models.RemoteContributionVersion, + Provider: models.RemoteContributionProviderGitHub, + Kind: models.RemoteContributionKindPullRequest, + CanonicalURL: "https://github.com/acme/widgets/pull/42", + Number: 42, + State: models.RemoteContributionStateOpen, + BaseBranch: "feature/deleted-parent", + HeadBranch: "feature/stacked-child", + HeadSHA: strings.Repeat("a", 40), + SourceRepository: models.RemoteContributionRepository{ + Host: "github.com", + Path: "acme/widgets", + RemoteURL: "https://github.com/acme/widgets.git", + }, + } + metadata := map[string]interface{}{} + if err := models.PutRemoteContribution(metadata, binding); err != nil { + t.Fatalf("PutRemoteContribution() error: %v", err) + } + exec := newTestExecutor(t, &mockAgentManager{}, repo) + + info, err := exec.resolveTaskRepoInfo(context.Background(), &models.TaskRepository{ + ID: "task-repo-1", + TaskID: "task-1", + RepositoryID: "repo-1", + BaseBranch: "main", + CheckoutBranch: "feature/stacked-child", + Metadata: metadata, + }) + if err != nil { + t.Fatalf("resolveTaskRepoInfo() error: %v", err) + } + if info.BaseBranch != "main" { + t.Fatalf("BaseBranch = %q, want task repository base", info.BaseBranch) + } + if info.RemoteContribution == nil || info.RemoteContribution.BaseBranch != "main" { + t.Fatalf("remote contribution base = %#v, want main", info.RemoteContribution) + } +} diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index 51091cf543..45c5c6e9af 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -177,7 +177,11 @@ func (e *Executor) resolveTaskRepoInfoForSession( return nil, fmt.Errorf("load remote contribution for task repository %q: %w", tr.ID, err) } else if found { if tr.BaseBranch != "" && tr.BaseBranch != binding.BaseBranch { - return nil, fmt.Errorf("task repository %q base branch does not match remote contribution", tr.ID) + e.logger.Debug("reconciling remote contribution base branch with task repository", + zap.String("task_repository_id", tr.ID), + zap.String("old_base_branch", binding.BaseBranch), + zap.String("new_base_branch", tr.BaseBranch)) + binding.BaseBranch = tr.BaseBranch } if tr.CheckoutBranch != "" && tr.CheckoutBranch != binding.HeadBranch { return nil, fmt.Errorf("task repository %q checkout branch does not match remote contribution", tr.ID) @@ -206,6 +210,7 @@ func (e *Executor) resolveTaskRepoInfoForSession( zap.Error(err)) return nil, err } + e.resolvePRBaseForLaunch(ctx, tr, repo, info) remoteRefState, err := e.ensureRepoLocalPathForSessionAndState(ctx, tr.TaskID, sessionID, repo) if err != nil { @@ -256,6 +261,36 @@ func (e *Executor) resolveTaskRepoInfoForSession( return info, nil } +func (e *Executor) resolvePRBaseForLaunch( + ctx context.Context, tr *models.TaskRepository, repo *models.Repository, info *repoInfo, +) { + if e.prBaseResolver == nil || info.PRNumber <= 0 || !isGitHubRepository(repo) { + return + } + baseBranch, err := e.prBaseResolver.ResolvePRBaseBranch( + ctx, repo.WorkspaceID, repo.ProviderOwner, repo.ProviderName, info.PRNumber, + ) + baseBranch = strings.TrimSpace(baseBranch) + if err != nil || baseBranch == "" { + e.logger.Debug("could not resolve live pull request base branch", + zap.String("task_id", tr.TaskID), + zap.Int("pr_number", info.PRNumber), + zap.Error(err)) + return + } + if baseBranch != tr.BaseBranch { + e.logger.Info("pull request base branch changed since task creation", + zap.String("task_id", tr.TaskID), + zap.Int("pr_number", info.PRNumber), + zap.String("old_base_branch", tr.BaseBranch), + zap.String("new_base_branch", baseBranch)) + } + info.BaseBranch = baseBranch + if info.RemoteContribution != nil { + info.RemoteContribution.BaseBranch = baseBranch + } +} + func hasProviderRepositoryIdentity(repo *models.Repository) bool { if repo == nil { return false diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 66c75941c4..d3b05fdeda 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -1401,6 +1401,11 @@ func (s *Service) SetTaskRepositoryBaseBranchUpdater(updater executor.TaskReposi s.executor.SetTaskRepositoryBaseBranchUpdater(updater) } +// SetPRBaseResolver wires best-effort pull-request base resolution at launch. +func (s *Service) SetPRBaseResolver(resolver executor.PRBaseResolver) { + s.executor.SetPRBaseResolver(resolver) +} + // RepositoryHostCloner is the narrow host-materialization contract. It returns // only the persisted local path; clone credentials remain private to the // orchestrator executor pipeline. diff --git a/apps/backend/internal/worktree/manager_git.go b/apps/backend/internal/worktree/manager_git.go index a59f16a09b..4b938bed3f 100644 --- a/apps/backend/internal/worktree/manager_git.go +++ b/apps/backend/internal/worktree/manager_git.go @@ -121,26 +121,34 @@ func (m *Manager) preferRefreshedRemoteRef(ctx context.Context, repoPath, branch } func (m *Manager) resolveRefreshedBaseRefWithFallback( - ctx context.Context, repoPath, baseBranch string, -) (string, string, string, error) { + ctx context.Context, repoPath, baseBranch, fallbackBaseBranch string, +) (string, string, string, string, error) { resolved, err := m.preferRefreshedRemoteRef(ctx, repoPath, baseBranch) if err == nil { - return resolved, "", "", nil - } - if isRemoteOnlyBaseRef(baseBranch) { - return "", "", "", err + return resolved, "", "", "", nil } if ctxErr := ctx.Err(); ctxErr != nil { - return "", "", "", ctxErr + return "", "", "", "", ctxErr + } + reason := classifyBaseRefSelectionFailure(err) + fallback := strings.TrimSpace(fallbackBaseBranch) + if reason == gitFallbackReasonMissingRemoteRef && fallback != "" && fallback != baseBranch { + resolvedFallback, _, _, _, fallbackErr := m.resolveRefreshedBaseRefWithFallback(ctx, repoPath, fallback, "") + if fallbackErr == nil { + return resolvedFallback, "", "", fallback, nil + } + return "", "", "", "", fallbackErr + } + if isRemoteOnlyBaseRef(baseBranch) { + return "", "", "", "", err } localExists, localErr := m.branchExists(ctx, repoPath, baseBranch) if localErr != nil { - return "", "", "", fmt.Errorf("could not verify local base ref %q: %w", baseBranch, localErr) + return "", "", "", "", fmt.Errorf("could not verify local base ref %q: %w", baseBranch, localErr) } if !localExists { - return "", "", "", err + return "", "", "", "", err } - reason := classifyBaseRefSelectionFailure(err) warning, detail := localBaseRefreshWarning(reason, baseBranch) m.logger.Warn("refreshed remote base was incomplete; using local base", zap.String("branch", baseBranch), @@ -148,7 +156,7 @@ func (m *Manager) resolveRefreshedBaseRefWithFallback( zap.String("fallback_ref", baseBranch), zap.Error(err), ) - return baseBranch, warning, detail, nil + return baseBranch, warning, detail, "", nil } // prepareCheckoutFromRefreshedOrigin verifies a refreshed remote branch and @@ -304,6 +312,8 @@ func (m *Manager) newNonInteractiveGitCmd(ctx context.Context, repoPath string, return cmd } +const gitFallbackReasonMissingRemoteRef = "missing_remote_ref" + func classifyGitFallbackReason(cmdErr error, cmdOutput string, ctxErr error) string { if errors.Is(ctxErr, context.DeadlineExceeded) || errors.Is(cmdErr, context.DeadlineExceeded) { return "timeout" @@ -312,11 +322,9 @@ func classifyGitFallbackReason(cmdErr error, cmdOutput string, ctxErr error) str if containsAuthFailure(strings.ToLower(cmdOutput)) { return "non_interactive_auth_failed" } - if isRemoteBranchMissingError(cmdOutput) { - return "missing_remote_ref" + return gitFallbackReasonMissingRemoteRef } - return "git_command_failed" } @@ -332,6 +340,20 @@ func classifyGitFallbackReason(cmdErr error, cmdOutput string, ctxErr error) str func (m *Manager) pullBaseBranch( ctx context.Context, repoPath, baseBranch string, onProgress SyncProgressCallback, ) (string, error) { + ref, _, err := m.pullBaseBranchWithPolicy(ctx, repoPath, baseBranch, "", false, onProgress) + return ref, err +} + +func (m *Manager) pullBaseBranchWithFallback( + ctx context.Context, repoPath, baseBranch, fallbackBaseBranch string, onProgress SyncProgressCallback, +) (string, string, error) { + return m.pullBaseBranchWithPolicy(ctx, repoPath, baseBranch, fallbackBaseBranch, true, onProgress) +} + +func (m *Manager) pullBaseBranchWithPolicy( + ctx context.Context, repoPath, baseBranch, fallbackBaseBranch string, required bool, + onProgress SyncProgressCallback, +) (string, string, error) { localBranch := strings.TrimPrefix(baseBranch, "origin/") isRemoteRef := localBranch != baseBranch stepName := "Sync base branch" @@ -347,12 +369,7 @@ func (m *Manager) pullBaseBranch( var err error localBaseExists, err = m.branchExists(ctx, repoPath, baseBranch) if err != nil { - reason := "base_ref_unverified" - if ctxErr := ctx.Err(); ctxErr != nil { - reason = syncContextFailureReason(ctxErr) - return "", m.failRequiredSync(stepName, baseBranch, onProgress, reason, ctxErr) - } - return "", m.failRequiredSync(stepName, baseBranch, onProgress, reason, err) + return "", "", m.failBaseBranchProbe(ctx, stepName, baseBranch, onProgress, err) } } @@ -367,39 +384,97 @@ func (m *Manager) pullBaseBranch( } output, err, execCtxErr := m.runGitCombinedAfterAcquire(ctx, m.fetchTimeout, repoPath, fetchArgs...) if err != nil { - reason := classifyGitFallbackReason(err, string(output), execCtxErr) - if localBaseExists { - return m.completeSyncWithWarning( - ctx, stepName, baseBranch, "Fetch", reason, baseBranch, onProgress, - syncFailureCause(reason, err, execCtxErr), - ) + return m.handleBaseFetchFailure( + ctx, repoPath, stepName, baseBranch, fallbackBaseBranch, required, + localBaseExists, output, err, execCtxErr, onProgress, + ) + } + + if isRemoteRef { + return m.resolveFetchedRemoteBase(ctx, repoPath, stepName, baseBranch, localBranch, required, onProgress) + } + + resolved, resolveErr := m.resolveLocalBaseRef(ctx, repoPath, baseBranch, localBranch, stepName, onProgress, localBaseExists && !required) + return resolved, "", resolveErr +} + +func (m *Manager) failBaseBranchProbe( + ctx context.Context, stepName, baseBranch string, onProgress SyncProgressCallback, err error, +) error { + reason := "base_ref_unverified" + if ctxErr := ctx.Err(); ctxErr != nil { + reason = syncContextFailureReason(ctxErr) + err = ctxErr + } + return m.failRequiredSync(stepName, baseBranch, onProgress, reason, err) +} + +func (m *Manager) handleBaseFetchFailure( + ctx context.Context, + repoPath, stepName, baseBranch, fallbackBaseBranch string, + required, localBaseExists bool, + output []byte, + err, execCtxErr error, + onProgress SyncProgressCallback, +) (string, string, error) { + reason := classifyGitFallbackReason(err, string(output), execCtxErr) + if required { + fallback := strings.TrimSpace(fallbackBaseBranch) + if reason == gitFallbackReasonMissingRemoteRef && fallback != "" && fallback != baseBranch { + resolved, _, fallbackErr := m.pullBaseBranchWithPolicy(ctx, repoPath, fallback, "", true, onProgress) + if fallbackErr == nil { + return resolved, fallback, nil + } + return "", "", fallbackErr } failureCause := syncFailureCause(reason, err, execCtxErr) if ctxErr := ctx.Err(); ctxErr != nil { reason = syncContextFailureReason(ctxErr) failureCause = ctxErr } - return "", m.failRequiredSync( + return "", "", m.failRequiredSync( stepName, baseBranch, onProgress, reason, fmt.Errorf("required refresh of %q failed (%s): %w", baseBranch, reason, failureCause), ) } - - if isRemoteRef { - resolved := "origin/" + localBranch - if exists, branchErr := m.branchExists(ctx, repoPath, resolved); branchErr != nil { - return "", m.failRequiredSync(stepName, baseBranch, onProgress, "base_ref_unverified", branchErr) - } else if !exists { - return "", m.failRequiredSync( - stepName, baseBranch, onProgress, "missing_remote_ref", - fmt.Errorf("required fetched remote ref %q is missing", resolved), - ) - } - m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", resolved), "") - return resolved, nil + if localBaseExists { + resolved, warningErr := m.completeSyncWithWarning( + ctx, stepName, baseBranch, "Fetch", reason, baseBranch, onProgress, + syncFailureCause(reason, err, execCtxErr), + ) + return resolved, "", warningErr } + failureCause := syncFailureCause(reason, err, execCtxErr) + if ctxErr := ctx.Err(); ctxErr != nil { + reason = syncContextFailureReason(ctxErr) + failureCause = ctxErr + } + return "", "", m.failRequiredSync( + stepName, baseBranch, onProgress, reason, + fmt.Errorf("required refresh of %q failed (%s): %w", baseBranch, reason, failureCause), + ) +} - return m.resolveLocalBaseRef(ctx, repoPath, baseBranch, localBranch, stepName, onProgress, localBaseExists) +func (m *Manager) resolveFetchedRemoteBase( + ctx context.Context, + repoPath, stepName, baseBranch, localBranch string, + required bool, + onProgress SyncProgressCallback, +) (string, string, error) { + resolved := "origin/" + localBranch + if exists, branchErr := m.branchExists(ctx, repoPath, resolved); branchErr != nil { + if required { + return "", "", m.failRequiredSync(stepName, baseBranch, onProgress, "base_ref_unverified", branchErr) + } + return "", "", branchErr + } else if !exists { + return "", "", m.failRequiredSync( + stepName, baseBranch, onProgress, gitFallbackReasonMissingRemoteRef, + fmt.Errorf("required fetched remote ref %q is missing", resolved), + ) + } + m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", resolved), "") + return resolved, "", nil } func (m *Manager) reportSyncProgress(cb SyncProgressCallback, event SyncProgressEvent) { diff --git a/apps/backend/internal/worktree/manager_lifecycle.go b/apps/backend/internal/worktree/manager_lifecycle.go index 01eff08359..5b1945b483 100644 --- a/apps/backend/internal/worktree/manager_lifecycle.go +++ b/apps/backend/internal/worktree/manager_lifecycle.go @@ -205,6 +205,9 @@ func (m *Manager) handleProviderRefreshFailure(ctx context.Context, req *CreateR } return refreshErr } + if req.PRNumber > 0 { + return refreshErr + } if isRemoteOnlyBaseRef(req.BaseBranch) { return refreshErr } @@ -553,6 +556,7 @@ func (m *Manager) resolveBaseRefWithFallback(ctx context.Context, req *CreateReq baseRef = req.BaseBranch warning = req.baseRefreshFallbackWarning detail = req.baseRefreshFallbackDetail + resolvedFallback := "" switch { case req.baseRefreshFallback: // The provider refresh already failed after verifying the local base. @@ -568,18 +572,37 @@ func (m *Manager) resolveBaseRefWithFallback(ctx context.Context, req *CreateReq case baselineRef != "": baseRef = baselineRef case req.RemoteSyncHandled: - baseRef, warning, detail, err = m.resolveRefreshedBaseRefWithFallback(ctx, req.RepositoryPath, req.BaseBranch) + baseRef, warning, detail, resolvedFallback, err = m.resolveRefreshedBaseRefWithFallback( + ctx, req.RepositoryPath, req.BaseBranch, req.FallbackBaseBranch, + ) case req.PullBeforeWorktree: - baseRef, err = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) + if req.PRNumber > 0 { + baseRef, resolvedFallback, err = m.pullBaseBranchWithFallback( + ctx, req.RepositoryPath, req.BaseBranch, req.FallbackBaseBranch, req.OnSyncProgress, + ) + } else { + baseRef, err = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) + } } case req.RemoteSyncHandled: - baseRef, warning, detail, err = m.resolveRefreshedBaseRefWithFallback(ctx, req.RepositoryPath, req.BaseBranch) + baseRef, warning, detail, resolvedFallback, err = m.resolveRefreshedBaseRefWithFallback( + ctx, req.RepositoryPath, req.BaseBranch, req.FallbackBaseBranch, + ) case req.PullBeforeWorktree: - baseRef, err = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) + if req.PRNumber > 0 { + baseRef, resolvedFallback, err = m.pullBaseBranchWithFallback( + ctx, req.RepositoryPath, req.BaseBranch, req.FallbackBaseBranch, req.OnSyncProgress, + ) + } else { + baseRef, err = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) + } } if err != nil { return "", "", "", err } + if resolvedFallback != "" { + return m.finishMissingRemoteBaseFallback(req, resolvedFallback, baseRef) + } baseExists, baseErr := m.branchExists(ctx, req.RepositoryPath, baseRef) if baseErr != nil { @@ -592,7 +615,6 @@ func (m *Manager) resolveBaseRefWithFallback(ctx context.Context, req *CreateReq } fallback := strings.TrimSpace(req.FallbackBaseBranch) - resolvedFallback := "" if fallback != "" && fallback != baseRef { resolvedFallback, err = m.resolveFallbackRef(ctx, req, fallback) if err != nil { @@ -637,7 +659,7 @@ func (m *Manager) resolveBaseRefWithFallback(ctx context.Context, req *CreateReq func (m *Manager) resolveFallbackRef(ctx context.Context, req *CreateRequest, fallback string) (string, error) { if req.RemoteSyncHandled { - resolved, _, _, err := m.resolveRefreshedBaseRefWithFallback(ctx, req.RepositoryPath, fallback) + resolved, _, _, _, err := m.resolveRefreshedBaseRefWithFallback(ctx, req.RepositoryPath, fallback, "") return resolved, err } if req.PullBeforeWorktree { @@ -663,6 +685,19 @@ func (m *Manager) finishBaseFallback(req *CreateRequest, baseRef, fallback, reso return resolvedFallback, warning, detail, nil } +func (m *Manager) finishMissingRemoteBaseFallback( + req *CreateRequest, fallback, resolvedFallback string, +) (string, string, string, error) { + m.logger.Warn("requested base branch no longer exists on origin, falling back", + zap.String("repository_path", req.RepositoryPath), + zap.String("requested_branch", req.BaseBranch), + zap.String("fallback_branch", fallback)) + warning := fmt.Sprintf("Base branch %q no longer exists on origin; using %q instead.", req.BaseBranch, fallback) + detail := fmt.Sprintf("Required refresh could not find remote ref %q; fallback branch %q refreshed successfully", req.BaseBranch, fallback) + req.BaseBranch = fallback + return resolvedFallback, warning, detail, nil +} + // createInTaskDir creates a worktree inside the task directory structure: // ~/.kandev/tasks/{taskDirName}/{repoName}/ // diff --git a/apps/backend/internal/worktree/manager_pull_fallback_test.go b/apps/backend/internal/worktree/manager_pull_fallback_test.go index 5b998e4a9b..56b51a9935 100644 --- a/apps/backend/internal/worktree/manager_pull_fallback_test.go +++ b/apps/backend/internal/worktree/manager_pull_fallback_test.go @@ -225,6 +225,36 @@ func TestCreateWorktree_ManagedRefreshDivergenceUsesLocalBaseWarning(t *testing. } } +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13 +func TestCreateWorktree_ManagedRefreshMissingBaseUsesConfiguredFallback(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + runGit(t, repoPath, "branch", "feature/deleted-parent") + mgr := newRecreateTestManager(t) + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-managed-missing-base", + SessionID: "session-managed-missing-base", + RepositoryID: "repo-managed-missing-base", + RepositoryPath: repoPath, + BaseBranch: "feature/deleted-parent", + FallbackBaseBranch: "main", + PullBeforeWorktree: true, + RemoteSyncHandled: true, + TaskDirName: "task-managed-missing-base", + RepoName: "repo", + }) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if wt.BaseBranch != "main" { + t.Fatalf("worktree BaseBranch = %q, want main", wt.BaseBranch) + } + if !strings.Contains(wt.BaseBranchFallbackWarning, "feature/deleted-parent") || + !strings.Contains(wt.BaseBranchFallbackWarning, "main") { + t.Fatalf("fallback warning = %q, want old and new branches", wt.BaseBranchFallbackWarning) + } +} + func TestCreateWorktree_ManagedRefreshFailureUsesLocalBaseWarning(t *testing.T) { repoPath, localHead := initGitRepoWithOriginAheadLocal(t) mgr := newRecreateTestManager(t) @@ -520,7 +550,7 @@ esac t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) req := CreateRequest{ - RepositoryPath: repoPath, BaseBranch: "main", PullBeforeWorktree: true, + RepositoryPath: repoPath, BaseBranch: "main", FallbackBaseBranch: "develop", PullBeforeWorktree: true, } var events []SyncProgressEvent req.OnSyncProgress = captureSyncProgress(&events) diff --git a/apps/backend/internal/worktree/manager_stacked_pr_base_fallback_test.go b/apps/backend/internal/worktree/manager_stacked_pr_base_fallback_test.go new file mode 100644 index 0000000000..a0f74adfa5 --- /dev/null +++ b/apps/backend/internal/worktree/manager_stacked_pr_base_fallback_test.go @@ -0,0 +1,169 @@ +package worktree + +import ( + "context" + "errors" + "os" + "strings" + "testing" +) + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13 +func TestCreateWorktree_MissingRemoteBaseUsesRefreshedFallback(t *testing.T) { + repoPath, wantHead := initManagedPRCheckoutBranch(t, 42, "feature/pr-branch") + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + var events []SyncProgressEvent + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-stacked-pr", + SessionID: "session-stacked-pr", + TaskTitle: "Review stacked PR", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "feature/deleted-parent", + FallbackBaseBranch: "main", + CheckoutBranch: "feature/pr-branch", + PRNumber: 42, + PullBeforeWorktree: true, + TaskDirName: "task-stacked-pr", + RepoName: "repo-1", + OnSyncProgress: captureSyncProgress(&events), + }) + if err != nil { + t.Fatalf("Create() unexpected error: %v", err) + } + if wt.BaseBranch != "main" { + t.Fatalf("worktree BaseBranch = %q, want main", wt.BaseBranch) + } + if !strings.Contains(wt.BaseBranchFallbackWarning, "feature/deleted-parent") || + !strings.Contains(wt.BaseBranchFallbackWarning, "main") { + t.Fatalf("fallback warning = %q, want old and new branches", wt.BaseBranchFallbackWarning) + } + if !strings.Contains(wt.BaseBranchFallbackWarning, "no longer exists on origin") { + t.Fatalf("fallback warning = %q, want missing-origin explanation", wt.BaseBranchFallbackWarning) + } + gotHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + if gotHead != wantHead { + t.Fatalf("worktree HEAD = %s, want checkout branch head %s", gotHead, wantHead) + } + for _, event := range events { + if event.Status == SyncProgressFailed { + t.Fatalf("sync progress contains failed event: %#v", events) + } + } + if len(events) == 0 || events[len(events)-1].Status != SyncProgressCompleted { + t.Fatalf("final sync progress = %#v, want completed", events) + } +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13 +func TestCreateWorktree_MissingRemoteBaseWithoutFallbackReportsMissingRef(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + var events []SyncProgressEvent + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-stacked-pr", + SessionID: "session-stacked-pr", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "feature/deleted-parent", + CheckoutBranch: "feature/pr-branch", + PRNumber: 42, + PullBeforeWorktree: true, + TaskDirName: "task-stacked-pr", + RepoName: "repo-1", + OnSyncProgress: captureSyncProgress(&events), + }) + if err == nil { + t.Fatal("Create() succeeded without a fallback") + } + if !strings.Contains(err.Error(), "missing_remote_ref") { + t.Fatalf("Create() error = %q, want missing_remote_ref", err) + } + if len(events) == 0 || events[len(events)-1].Status != SyncProgressFailed || + events[len(events)-1].Error != "missing_remote_ref" { + t.Fatalf("final sync progress = %#v, want missing_remote_ref failure", events) + } +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13 +func TestCreateWorktree_MissingRemoteBaseAndFallbackReportsMissingRef(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-stacked-pr", + SessionID: "session-stacked-pr", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "feature/deleted-parent", + FallbackBaseBranch: "feature/deleted-fallback", + CheckoutBranch: "feature/pr-branch", + PRNumber: 42, + PullBeforeWorktree: true, + TaskDirName: "task-stacked-pr", + RepoName: "repo-1", + }) + if err == nil { + t.Fatal("Create() succeeded with a missing fallback") + } + if !strings.Contains(err.Error(), "missing_remote_ref") { + t.Fatalf("Create() error = %q, want missing_remote_ref", err) + } + if errors.Is(err, ErrInvalidBaseBranch) { + t.Fatalf("Create() error = %v, want required-refresh failure", err) + } +} + +// @covers AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13 +func TestCreateWorktree_PRBaseTransientRefreshFailureRemainsFatal(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + fetch) + echo "fatal: Authentication failed" >&2 + exit 1 + ;; + *) + exit 0 + ;; +esac +`) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + var events []SyncProgressEvent + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-stacked-pr", + SessionID: "session-stacked-pr", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "main", + FallbackBaseBranch: "develop", + CheckoutBranch: "feature/pr-branch", + PRNumber: 42, + PullBeforeWorktree: true, + TaskDirName: "task-stacked-pr", + RepoName: "repo-1", + OnSyncProgress: captureSyncProgress(&events), + }) + if err == nil || !strings.Contains(err.Error(), "non_interactive_auth_failed") { + t.Fatalf("Create() error = %v, want fatal auth classification", err) + } + if len(events) == 0 || events[len(events)-1].Status != SyncProgressFailed { + t.Fatalf("final sync progress = %#v, want failed", events) + } +} diff --git a/docs/decisions/2026-08-25-required-worktree-refresh-fails-closed.md b/docs/decisions/2026-08-25-required-worktree-refresh-fails-closed.md index 7543b0a930..65460b0bef 100644 --- a/docs/decisions/2026-08-25-required-worktree-refresh-fails-closed.md +++ b/docs/decisions/2026-08-25-required-worktree-refresh-fails-closed.md @@ -8,6 +8,11 @@ > [ADR-2026-08-30-empty-remote-bootstrap-publication](2026-08-30-empty-remote-bootstrap-publication.md): > an authenticated remote that advertises zero refs uses a marked local baseline. > Launch still performs no remote mutation. +> +> Amended on 2026-08-31: a pull-request base fetch that proves the requested +> remote base was deleted can use a different configured fallback only after +> that fallback is refreshed successfully. Other required PR refresh failures +> remain fail-closed. **Status:** accepted (local-worktree boundary superseded by 2026-08-31-local-worktree-refresh-best-effort) **Date:** 2026-08-25 @@ -15,12 +20,10 @@ ## Context -A repository can require Kandev to pull before it creates a worktree. The -current worktree manager treats fetch and pull failures as warnings. It then -uses a local fallback ref and starts the agent. An authentication or network -failure can therefore produce a successful-looking task that edits an old copy -of the repository. The later push fails, after the agent has already spent time -on work based on stale state. +A repository can require Kandev to pull before it creates a worktree. Required +remote materialization and pull-request base refresh must not use an unverified +local fallback after a failed fetch. A successful-looking task could otherwise +edit an old copy of the repository and fail later when it pushes. Kandev also supports an explicit offline mode. A repository with pull-before-worktree disabled can use local state without contacting a remote. @@ -32,20 +35,25 @@ It cannot force a managed HTTPS route over an executor-inherited SSH origin. ## Decision -Pull-before-worktree is an admission gate for every new or recreated worktree. -When the setting is enabled, Kandev starts no agent until the configured remote -refresh succeeds and the selected base ref is proven to include the fetched -remote base. +Pull-before-worktree remains best effort for a host worktree with a usable local +base, as defined by the superseding local-worktree decision. Required remote +materialization and pull-request base refresh are admission gates. In those +paths, Kandev starts no agent until the configured remote refresh succeeds and +the selected base ref is verified. A provider-managed checkout uses the backend's exact-scope credential refresh. A host or executor checkout uses its reconciled origin and non-interactive Git environment. A successful provider refresh marks remote sync as handled so the worktree manager does not run a second unauthenticated fetch. -Fetch failure stops preparation. After a successful fetch, Kandev preserves a -local ref only when it contains the fetched remote ref. It uses the remote ref -when the remote contains the local ref. Diverged refs and failed ancestry -checks stop preparation without changing either ref. +For a required pull-request base, fetch failure stops preparation except when +Git explicitly proves the requested remote base is absent and a different +configured fallback can be refreshed successfully. This exception records the +substituted base as a warning and never authorizes an unverified local ref. +After a successful fetch, Kandev preserves a local ref only when it contains the +fetched remote ref. It uses the remote ref when the remote contains the local +ref. Diverged refs and failed ancestry checks stop required PR preparation +without changing either ref. Pull-before-worktree disabled remains the explicit offline opt-out. This path can use available local refs and makes no freshness guarantee. @@ -57,23 +65,32 @@ credential material. ## Consequences - Authentication, SSH, network, and timeout failures become visible before an - agent starts. + agent starts when remote materialization or PR refresh is required. - Users do not lose agent time on a checkout that Kandev knows it failed to refresh. - Existing offline workflows continue when pull-before-worktree is disabled. - Local-only commits remain available when the local branch contains current remote state. - Diverged branches require a user to reconcile history before retrying launch. +- Stacked pull requests can continue after GitHub retargets them away from a + deleted parent branch, provided the replacement base is refreshed. - The executor and worktree APIs become fallible at the base-refresh boundary. - Multi-repository launch stops before runtime startup when one required repository cannot refresh. ## Alternatives Considered -### Continue with a local ref and show a warning +### Continue with a local ref and show a warning for required PR refresh Rejected. A warning does not prevent the agent from producing changes against -known stale state. The failure can remain hidden until push. +known stale state. Host local-only worktrees use the separate best-effort +decision, but required PR refresh remains fail-closed. + +### Fail every missing remote base without consulting provider state + +Rejected. GitHub can retarget a stacked pull request after its parent branch is +merged and deleted. The pull-request head remains fetchable, and a separately +refreshed current or default base preserves the freshness gate. ### Require remote access for every worktree diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 2cd7ea258d..8d3ecec765 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -209,7 +209,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-08-24-opt-in-ssh-task-directory-reclamation | [Opt-in reclamation of remote SSH task directories](2026-08-24-opt-in-ssh-task-directory-reclamation.md) | accepted | backend, frontend, operations | 2026-08-24 | | 2026-08-24-agentctl-local-managed-runtime-cache-repair | [Run cache repair where npm runs](2026-08-24-agentctl-local-managed-runtime-cache-repair.md) | accepted | backend, agentctl, protocol, security | 2026-08-24 | | 2026-08-24-unified-fork-approval-label | [Use One Maintainer Approval Label for Contributor PR Automation](2026-08-24-unified-fork-approval-label.md) | accepted | infra, workflow, security | 2026-08-24 | -| 2026-08-25-required-worktree-refresh-fails-closed | [Required Worktree Refresh Fails Closed](2026-08-25-required-worktree-refresh-fails-closed.md) | accepted (local-worktree boundary superseded by 2026-08-31-local-worktree-refresh-best-effort) | backend, security, operations | 2026-08-25 | +| 2026-08-25-required-worktree-refresh-fails-closed | [Required Worktree Refresh Fails Closed](2026-08-25-required-worktree-refresh-fails-closed.md) | accepted (amended 2026-08-31; local-worktree boundary superseded) | backend, security, operations | 2026-08-25 | | 2026-08-26-quick-chat-tab-order | [Store Quick Chat Tab Order as a User Preference](2026-08-26-quick-chat-tab-order.md) | accepted | backend, frontend, protocol | 2026-08-26 | | 2026-08-26-quick-chat-agent-titles | [Apply Agent-Generated Titles to Quick Chat](2026-08-26-quick-chat-agent-titles.md) | accepted | backend, frontend, protocol | 2026-08-26 | | 2026-08-27-bounded-agent-profile-recency | [Store Agent Profile Recency in Bounded Context Rows](2026-08-27-bounded-agent-profile-recency.md) | accepted | backend, frontend, protocol | 2026-08-27 | diff --git a/docs/plans/worktree-refresh-fail-closed/plan.md b/docs/plans/worktree-refresh-fail-closed/plan.md index 0d35bb35eb..970e69dbc3 100644 --- a/docs/plans/worktree-refresh-fail-closed/plan.md +++ b/docs/plans/worktree-refresh-fail-closed/plan.md @@ -48,6 +48,9 @@ and updates the public Git guidance. - Preserve a local base that contains the fetched remote base. - Stop multi-repository launch before agent startup when one required refresh fails. +- Reconcile a GitHub pull request's current base at launch and during polling. +- Recover a proven deleted remote base through a separately refreshed default + base while retaining fail-closed behavior for all other fetch failures. - Project a credential-safe, repository-specific launch error through the existing task error surface. - Document SSH setup and the fail-closed behavior in the public Git guidance. @@ -87,11 +90,12 @@ No database or WebSocket schema change is planned. - [completed] [Task 02: Reject Stale Worktree Fallbacks](task-02-reject-stale-fallbacks.md) - [completed] [Task 03: Project Refresh Launch Failures](task-03-project-refresh-launch-failures.md) - [completed] [Task 04: Document Required Repository Refresh](task-04-document-required-refresh.md) +- [completed] [Task 05: Support Stacked PR Base Retargeting](task-05-support-stacked-pr-base-retargeting.md) ## Dependency order ```text -Task 01 -> Task 02 -> Task 03 -> Task 04 +Task 01 -> Task 02 -> Task 03 -> Task 04 -> Task 05 ``` The package is sequential. Task 02 consumes the refresh-route contract from @@ -147,6 +151,16 @@ verified behavior. affected mobile command passed 2/2 tests, and the complete Git Changes Panel file passed 21/21 tests. Prettier and `git diff --check` also passed. +### Stacked PR base retargeting results + +- Added launch-time live GitHub base resolution and polling-time persistence + for PR-backed task repositories, with best-effort failure handling. +- Added a verified default-base fallback only for missing remote base refs; + authentication, transport, timeout, and other refresh failures remain fatal. +- The affected backend package tests, complete backend test suite, backend + lint, backend build, specification validators, and public-documentation + validators passed. + ## Risks - A managed refresh that ignores task Git policy can replace an @@ -160,6 +174,8 @@ verified behavior. route helper narrow and run fresh-launch and resume suites together. - Error wrapping can leak raw Git output. Use existing redaction and bounded launch-error helpers. +- Live GitHub base resolution is best-effort. The polling projection and + refreshed default fallback must independently cover unavailable lookups. - A multi-repository preparation attempt can refresh earlier repositories before a later one fails. This is safe on-disk cache progress, but agent startup must remain atomic. diff --git a/docs/plans/worktree-refresh-fail-closed/task-05-support-stacked-pr-base-retargeting.md b/docs/plans/worktree-refresh-fail-closed/task-05-support-stacked-pr-base-retargeting.md new file mode 100644 index 0000000000..3374ab55df --- /dev/null +++ b/docs/plans/worktree-refresh-fail-closed/task-05-support-stacked-pr-base-retargeting.md @@ -0,0 +1,103 @@ +--- +id: "05-support-stacked-pr-base-retargeting" +title: "Support stacked PR base retargeting" +status: completed +wave: 5 +depends_on: + - "02-reject-stale-fallbacks" +plan: "plan.md" +requirements: + - REQ-WORKSPACES-WORKTREE-BASE-REFRESH-001 +acceptance_criteria: + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.12 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13 +system_design: + - ../../specs/workspaces/system-design/worktree-base-refresh.md +--- + +# Task 05: Support Stacked PR Base Retargeting + +## Summary + +Keep PR-backed task bases aligned with GitHub and recover a deleted stacked-PR +parent branch through a verified fallback. Preserve fail-closed behavior for +transport, authentication, timeout, and other unproven fetch failures. + +## In scope + +- Resolve a GitHub pull request's current base during task repository launch. +- Propagate observed base changes from GitHub polling to the matching task + repository. +- Fetch and verify a configured fallback only for a proven missing remote base. +- Surface the branch substitution and classify unrecoverable missing refs. +- Add focused executor, GitHub service, and worktree tests. + +## Out of scope + +- Pull-request head fetching or checkout changes. +- Credential, SSH, or remote-origin behavior changes. +- Event-driven worktree recreation after provider updates. + +## Acceptance + +- A retargeted stacked pull request materializes against its current live base. +- Polling updates the matching persisted task repository without making PR sync + depend on that best-effort update. +- A proven deleted base uses only a successfully refreshed fallback and records + a warning; missing fallback and all transient failures remain fatal. + +## Verification + +```bash +cd apps/backend && go test ./internal/worktree/... ./internal/orchestrator/executor/... ./internal/github/... +make -C apps/backend test +make -C apps/backend lint +make -C apps/backend build +python3 scripts/lint-spec-files.py --all +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +``` + +## Files likely touched + +- `apps/backend/internal/worktree/manager_git.go` +- `apps/backend/internal/worktree/manager_lifecycle.go` +- `apps/backend/internal/orchestrator/executor/` +- `apps/backend/internal/github/` +- `apps/backend/internal/backendapp/` +- `docs/specs/workspaces/` +- `docs/public/git-operations.md` + +## Dependencies + +Task 02 established the required-refresh and verified-ref contracts extended by +this work. + +## Risks + +- A broad fallback could hide auth or transport failures; eligibility must use + only the explicit missing-remote-ref classifier. +- Multi-repository tasks require exact repository association matching. + +## Parallelism + +`sequential` + +## Inputs + +- The pull-request base reconciliation and missing-base fallback sections in + the workspace system design. +- Existing worktree fallback, executor resolver, and GitHub task-PR sync tests. + +## Results + +- Launch materialization now resolves a GitHub pull request's live base without + making GitHub availability a launch dependency. +- PR polling projects a changed base to the matching task repository through + the existing task-service update contract. +- Required base sync classifies missing remote refs explicitly and uses a + verified fallback with a user-visible warning when one is available. +- Focused tests, the complete backend suite, backend lint and build, and all + specification and public-documentation validators passed. diff --git a/docs/public/executors.md b/docs/public/executors.md index 23cf0d3294..0ade16c11d 100644 --- a/docs/public/executors.md +++ b/docs/public/executors.md @@ -200,7 +200,7 @@ equivalent GitHub SSH access, known-hosts entry, and agent or key on that execut Worktree creates a dedicated host Git worktree and runs the standalone `agentctl` service against it. It separates branches and files between tasks, but the process still has the Kandev user's host permissions, network access, and readable credentials. -Repository settings control base branch, branch naming, pull-before-create, repository setup/cleanup scripts, and optional copies of ignored files. With **Always pull before creating a new worktree** enabled, a host Worktree refresh is best effort when the selected local base exists. An authentication, network, timeout, missing-ref, divergent-ref, or uncertain-ancestry error produces a credential-safe warning and the host worktree uses the verified local base. The warning states that remote changes may be missing. If no usable local base exists, or if the executor must materialize the repository remotely, refresh and checkout remain required and a failure stops the launch. Disable this setting only for an intentional offline local workflow. Copy ignored files narrowly: `.env` and similar files often contain production secrets. Multi-repository tasks receive one materialized worktree per attachment; use the per-repository setup scripts because the profile-level prepare script is currently skipped for that path. +Repository settings control base branch, branch naming, pull-before-create, repository setup/cleanup scripts, and optional copies of ignored files. With **Always pull before creating a new worktree** enabled, a host Worktree refresh is best effort when the selected local base exists. An authentication, network, timeout, missing-ref, divergent-ref, or uncertain-ancestry error produces a credential-safe warning and the host worktree uses the verified local base. The warning states that remote changes may be missing. For a numbered GitHub PR, the current PR base is used when available. A proven deleted remote base can use a separately refreshed configured fallback branch, often the repository default, and produces a warning; authentication, network, timeout, and other unproven PR refresh failures remain fatal. If no usable local base exists, or if the executor must materialize the repository remotely, refresh and checkout remain required and a failure stops the launch. Disable this setting only for an intentional offline local workflow. Copy ignored files narrowly: `.env` and similar files often contain production secrets. Multi-repository tasks receive one materialized worktree per attachment; use the per-repository setup scripts because the profile-level prepare script is currently skipped for that path. Normal stop keeps the task environment available. Task deletion or **Reset Environment** removes the tracked worktree when configured to clean worktrees. Preserve or push valuable changes first; see [Git Operations](git-operations.md). diff --git a/docs/public/git-operations.md b/docs/public/git-operations.md index d7c6184da9..c052b0497b 100644 --- a/docs/public/git-operations.md +++ b/docs/public/git-operations.md @@ -80,6 +80,7 @@ feature/{title}-{suffix} ``` `{title}` is an ASCII-safe, lower-case task-title slug and `{suffix}` is a short collision-avoidance value. Repository settings can change the template. When `pull_before_worktree` is omitted it defaults to `true`: Kandev attempts to refresh and verify the base branch before creating or recreating the worktree. The public configuration defaults both fetch and fast-forward pull timeouts to 60 seconds. When a usable local base exists, authentication, network, timeout, missing-ref, divergent-ref, and uncertain-ancestry errors produce a credential-safe warning and Kandev creates the worktree from that local base. The warning states that remote changes may be missing. When no usable local base exists, Kandev must materialize the requested branch from the remote; a failed refresh or missing remote ref stops task preparation with a repository-specific launch error. Explicit remote-only refs and remote executors keep this strict materialization behavior. +For a numbered GitHub PR, Kandev uses the current PR base when available. If Git proves that the requested PR base was deleted, Kandev can refresh and use a configured fallback branch, often the repository default, with a warning that names both branches; unproven PR refresh failures remain fatal. Kandev does not create the worktree from an unverified local or remote-tracking fallback. If the repository is intentionally offline, open its workspace repository settings and disable **Always pull before creating a new worktree**. This skips the refresh attempt for host worktrees, @@ -116,7 +117,7 @@ target. Policies are not available in **Quick Chat**, **Remote**, **Add Sources**, or **Add Branch** flows. -When a task opens an existing branch or GitHub PR, Kandev fetches that branch; for a numbered GitHub PR it can fetch `refs/pull/NUMBER/head`, including fork PRs. If the intended branch is already checked out in another worktree, the new worktree uses a suffixed local branch and tracks the original `origin` branch when available. The required-refresh rule still applies before that new worktree is created. +When a task opens an existing branch or GitHub PR, Kandev fetches that branch; for a numbered GitHub PR it can fetch `refs/pull/NUMBER/head`, including fork PRs. At materialization, Kandev uses the PR's current GitHub base when available. Polling also keeps the task's stored comparison base aligned after GitHub retargets a stacked PR. If the intended branch is already checked out in another worktree, the new worktree uses a suffixed local branch and tracks the original `origin` branch when available. The required-refresh rule still applies before that new worktree is created. Tasks created without an initial title can expose the one-shot `set_task_title_kandev` handoff when **Settings → General → Task Actions → Agent-generated task titles** is enabled. After the owning diff --git a/docs/specs/workspaces/requirements/worktree-base-refresh.md b/docs/specs/workspaces/requirements/worktree-base-refresh.md index 291babdb40..b75ff7dabc 100644 --- a/docs/specs/workspaces/requirements/worktree-base-refresh.md +++ b/docs/specs/workspaces/requirements/worktree-base-refresh.md @@ -66,6 +66,22 @@ local branches without publishing them first. - **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.10:** When the caller cancels preparation, Kandev shall stop without creating a fallback worktree. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11:** When a task repository is + linked to a GitHub pull request, Kandev shall use the pull request's current + base branch for materialization when live provider state is available, and + shall retain the stored base when the provider lookup is unavailable. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.12:** When polling observes that a + linked pull request's non-empty base branch changed, Kandev shall update the + matching task repository base without failing the pull-request sync if that + secondary update is unavailable. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13:** When required refresh for a + pull-request task proves that the requested remote base no longer exists, + Kandev shall use a different configured fallback only after that fallback + refresh succeeds, and shall identify both branches in a warning. Without a + usable fallback, preparation shall fail with a missing-remote-ref + classification. Authentication, network, timeout, cancellation, divergent, + uncertain, and other unproven failures remain fatal for pull-request tasks. + ## Compatibility This requirement replaces the universal fail-closed refresh behavior from diff --git a/docs/specs/workspaces/system-design/worktree-base-refresh.md b/docs/specs/workspaces/system-design/worktree-base-refresh.md index fcf82c2ec7..5b9b751b68 100644 --- a/docs/specs/workspaces/system-design/worktree-base-refresh.md +++ b/docs/specs/workspaces/system-design/worktree-base-refresh.md @@ -34,15 +34,23 @@ change Git commands that an agent runs after launch. | `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.8` | [Required materialization](#required-materialization) | | `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.9` | [Empty remote](#empty-remote) | | `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.10` | [Failure and recovery](#failure-and-recovery) | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.11` | [Pull-request base reconciliation](#pull-request-base-reconciliation) | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.12` | [Pull-request base reconciliation](#pull-request-base-reconciliation) | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.13` | [Missing-base fallback](#missing-base-fallback) | ## Components and responsibilities - `internal/orchestrator/executor` resolves repository paths, refresh routes, - credentials, and executor capabilities. + credentials, and executor capabilities. It also resolves live GitHub pull- + request bases through an injected provider seam. - `internal/worktree.Manager` verifies local refs, attempts host refresh, and - selects the worktree base. + selects the worktree base. Pull-request base refresh remains strict except + for the verified missing-remote-ref fallback. - `internal/repoclone.Cloner` materializes provider-managed repositories when no user-owned local checkout supplies the branch. +- `internal/github.Service` reconciles changed pull-request bases through + narrow injected interfaces. Provider lookup and task-repository propagation + are best-effort. - `internal/agent/runtime/lifecycle` stops launch only when no usable base remains. - The task projection shows bounded warnings and required-materialization @@ -64,6 +72,29 @@ The preparation fields have these contracts: `PullBeforeWorktree` is not a universal admission gate. Local-ref availability determines whether a failed refresh is recoverable. +The task repository's stored base remains the durable comparison target and +offline launch fallback. The GitHub task-PR row tracks the provider observation; +polling propagates a changed non-empty base to the matching task repository. + +## Pull-request base reconciliation + +The executor reads the pull-request number from task-repository metadata and +the owner and repository name from the repository entity. For GitHub repository +rows with a positive pull-request number, it asks an injected resolver for the +current base before materialization. A successful non-empty result overrides +the launch request only. A missing resolver, lookup error, or empty result keeps +the stored base and does not stop launch. + +Once the pull-request task reaches Git refresh, its base remains strict for +unproven failures. A proven missing remote base can use only the fallback +described below. + +The GitHub polling service compares the previous `TaskPR.BaseBranch` with the +incoming non-empty base. On change, it updates the task repository whose task +and repository IDs match. When more than one association matches, the checkout +branch must also match the pull-request head. Update failures are logged and do +not fail the authoritative task-PR sync. + ## Refresh policy If `PullBeforeWorktree` is false, Kandev uses the selected local base without a @@ -89,6 +120,9 @@ raw credential output. The fallback does not change local or remote refs. It does not push the local branch. +Pull-request base refresh does not use this local fallback. It remains strict +for unproven failures and uses only the verified missing-base fallback below. + ## Base-ref selection After a successful fetch, the worktree manager compares local base `L` and @@ -116,6 +150,20 @@ repository and uses a bounded failure class. Provider selection still follows the existing Git credential policy. A managed HTTPS route must not replace an executor-owned SSH route. +## Missing-base fallback + +A fetch error that explicitly reports a missing remote ref is the only failed +fetch eligible for fallback. When the request carries a different non-empty +fallback base, the manager fetches that branch through the same non-interactive +route and applies the normal containing-ref checks. Success completes sync, +uses the fallback ref, and records a warning naming the requested and fallback +branches. + +If the fallback is absent or its refresh fails, preparation remains failed. A +missing requested or fallback remote ref uses `missing_remote_ref`; auth, +network, timeout, cancellation, and other Git failures keep their existing +fail-closed classifications. + ## Empty remote An authenticated remote with zero refs uses the marked local baseline from the @@ -139,6 +187,10 @@ whole task stops before agent startup. The error names that repository. - Caller cancellation stops preparation without a fallback worktree. - A retry repeats remote materialization only when the task still needs it. - A valid reused worktree does not refresh or materialize its base again. +- A pull-request base refresh failure stops preparation unless the missing + remote ref has a separately refreshed configured fallback. +- A pull-request fallback reports the requested and selected branches and uses + `missing_remote_ref` when no usable fallback exists. ## Security @@ -150,8 +202,11 @@ not contain tokens, credential-helper output, or secret URLs. ## Observability -Sync progress reports the refresh attempt and its result. Local fallback -reports completion with a warning, not a launch error. +The existing sync progress callback reports a running event and then either a +completed event or a failed event. A failed event contains the bounded failure +class and repository identity. A successful missing-base fallback reports a +completed event and records the branch substitution warning on the worktree. +Local fallback reports completion with a warning, not a launch error. Structured logs record repository identity, refresh route, failure class, and selected fallback ref. Logs exclude credential material and raw remote URLs.