diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go index 270e9d80de..efd67b7a5f 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go @@ -608,6 +608,8 @@ func applySyncProgressEvent(step *PrepareStep, event worktree.SyncProgressEvent) } step.Output = event.Output step.Error = event.Error + step.Warning = event.Warning + step.WarningDetail = event.WarningDetail switch event.Status { case worktree.SyncProgressRunning: step.Status = PrepareStepRunning diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go index 05bcc52ad0..6cb8ce7f1a 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go @@ -307,7 +307,9 @@ func TestWorktreePreparer_MultiRepo_RequiredRefreshIdentifiesFailingRepository(t TaskDirName: "refresh-fail_eee", Repositories: []RepoPrepareSpec{ {TaskRepositoryID: "tr-good", RepositoryID: "repo-good", RepositoryPath: repoA, RepoName: "good", BaseBranch: "main"}, - {TaskRepositoryID: "tr-bad", RepositoryID: "repo-bad", RepositoryPath: repoB, RepoName: "backend", BaseBranch: "main", PullBeforeWorktree: true}, + // The absent local and remote base keeps this repository strict so + // the test can identify which attachment caused preparation to fail. + {TaskRepositoryID: "tr-bad", RepositoryID: "repo-bad", RepositoryPath: repoB, RepoName: "backend", BaseBranch: "remote-only", PullBeforeWorktree: true}, }, } @@ -339,6 +341,49 @@ func TestWorktreePreparer_MultiRepo_RequiredRefreshIdentifiesFailingRepository(t } } +func TestWorktreePreparer_MultiRepo_LocalRefreshFallbackKeepsTaskRunning(t *testing.T) { + repoA := initBareGitRepo(t, "local-fallback") + repoB := initBareGitRepo(t, "unchanged") + + preparer, mgr := newPreparerForTest(t) + req := &EnvPrepareRequest{ + TaskID: "task-multi-local-fallback", + SessionID: "sess-multi-local-fallback", + TaskTitle: "Local refresh fallback", + ExecutorType: executor.NameStandalone, + TaskDirName: "local-fallback_fff", + Repositories: []RepoPrepareSpec{ + {TaskRepositoryID: "tr-local", RepositoryID: "repo-local", RepositoryPath: repoA, RepoName: "local-fallback", BaseBranch: "main", PullBeforeWorktree: true}, + {TaskRepositoryID: "tr-unchanged", RepositoryID: "repo-unchanged", RepositoryPath: repoB, RepoName: "unchanged", BaseBranch: "main"}, + }, + } + + res, err := preparer.Prepare(context.Background(), req, nil) + if err != nil { + t.Fatalf("prepare returned hard error: %v", err) + } + if !res.Success { + t.Fatalf("expected local fallback to keep preparation running: %s", res.ErrorMessage) + } + if len(res.Worktrees) != 2 { + t.Fatalf("worktrees = %d, want 2", len(res.Worktrees)) + } + var syncWarnings []PrepareStep + for _, step := range res.Steps { + if step.Name == "Sync base branch" && step.Warning != "" { + syncWarnings = append(syncWarnings, step) + } + } + if len(syncWarnings) != 1 { + t.Fatalf("sync warning steps = %#v, want one local fallback warning", res.Steps) + } + if all, listErr := mgr.GetAllByTaskID(context.Background(), req.TaskID); listErr != nil { + t.Fatalf("list worktrees: %v", listErr) + } else if len(all) != 2 { + t.Fatalf("stored worktrees = %d, want 2", len(all)) + } +} + func TestWorktreePreparer_MultiRepo_RollbackKeepsReusedWorktrees(t *testing.T) { repoExisting := initBareGitRepo(t, "existing") repoNew := initBareGitRepo(t, "new") diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_test.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_test.go index 357fccb388..875b0472db 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_test.go @@ -63,3 +63,24 @@ func TestCopyFilesStep_WarningsOnlyNoCopies(t *testing.T) { t.Error("expected warning to be set") } } + +func TestApplySyncProgressEvent_PropagatesWarning(t *testing.T) { + step := beginStep("Sync base branch") + applySyncProgressEvent(&step, worktree.SyncProgressEvent{ + StepName: "Sync base branch", + Status: worktree.SyncProgressCompleted, + Output: "Fetch timeout; using main", + Warning: "Remote refresh was incomplete; using local base main.", + WarningDetail: "The selected local base was verified before refresh.", + }) + + if step.Status != PrepareStepCompleted { + t.Fatalf("status = %q, want completed", step.Status) + } + if step.Warning != "Remote refresh was incomplete; using local base main." { + t.Fatalf("warning = %q, want sync warning", step.Warning) + } + if step.WarningDetail != "The selected local base was verified before refresh." { + t.Fatalf("warning detail = %q, want sync warning detail", step.WarningDetail) + } +} diff --git a/apps/backend/internal/worktree/manager_git.go b/apps/backend/internal/worktree/manager_git.go index 0cb346811e..a59f16a09b 100644 --- a/apps/backend/internal/worktree/manager_git.go +++ b/apps/backend/internal/worktree/manager_git.go @@ -120,6 +120,37 @@ func (m *Manager) preferRefreshedRemoteRef(ctx context.Context, repoPath, branch return m.selectContainingRef(ctx, repoPath, branch, remoteRef) } +func (m *Manager) resolveRefreshedBaseRefWithFallback( + ctx context.Context, repoPath, baseBranch string, +) (string, string, string, error) { + resolved, err := m.preferRefreshedRemoteRef(ctx, repoPath, baseBranch) + if err == nil { + return resolved, "", "", nil + } + if isRemoteOnlyBaseRef(baseBranch) { + return "", "", "", err + } + if ctxErr := ctx.Err(); ctxErr != nil { + return "", "", "", ctxErr + } + localExists, localErr := m.branchExists(ctx, repoPath, baseBranch) + if localErr != nil { + return "", "", "", fmt.Errorf("could not verify local base ref %q: %w", baseBranch, localErr) + } + if !localExists { + 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), + zap.String("reason", reason), + zap.String("fallback_ref", baseBranch), + zap.Error(err), + ) + return baseBranch, warning, detail, nil +} + // prepareCheckoutFromRefreshedOrigin verifies a refreshed remote branch and // returns whether a usable local or remote ref exists without contacting // origin. It returns false when neither ref exists, preserving the existing @@ -282,23 +313,29 @@ func classifyGitFallbackReason(cmdErr error, cmdOutput string, ctxErr error) str return "non_interactive_auth_failed" } + if isRemoteBranchMissingError(cmdOutput) { + return "missing_remote_ref" + } + return "git_command_failed" } -// pullBaseBranch fetches the latest changes from origin and returns the safest -// ref to use for creating a new worktree. The function handles three scenarios: +// pullBaseBranch attempts to refresh origin and returns the safest ref to use +// for creating a new worktree. The function handles three scenarios: // // 1. baseBranch is already a remote ref (e.g., "origin/main"): fetch and use it directly // 2. baseBranch is a local branch and we're currently on it: pull --ff-only to update // 3. baseBranch is a local branch but we're on a different branch: use origin/ instead // -// A failed required fetch or an unprovable base relationship returns an error. +// A local base makes refresh best effort. Remote-only refs still require a +// successful fetch and an independently verified remote ref. func (m *Manager) pullBaseBranch( ctx context.Context, repoPath, baseBranch string, onProgress SyncProgressCallback, ) (string, error) { localBranch := strings.TrimPrefix(baseBranch, "origin/") isRemoteRef := localBranch != baseBranch stepName := "Sync base branch" + localBaseExists := false m.reportSyncProgress(onProgress, SyncProgressEvent{ StepName: stepName, @@ -306,6 +343,19 @@ func (m *Manager) pullBaseBranch( Output: fmt.Sprintf("Fetching latest changes for %s", baseBranch), }) + if !isRemoteRef { + 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) + } + } + // Acquire the git throttle slot first, then start the fetch timer. // Order matters: the previous "build fetchCtx, then runGitCmd" shape // let throttle queue time burn the fetch budget while we waited for a @@ -318,9 +368,20 @@ 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), + ) + } + 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, syncFailureCause(reason, err, execCtxErr)), + fmt.Errorf("required refresh of %q failed (%s): %w", baseBranch, reason, failureCause), ) } @@ -338,7 +399,7 @@ func (m *Manager) pullBaseBranch( return resolved, nil } - return m.resolveLocalBaseRef(ctx, repoPath, baseBranch, localBranch, stepName, onProgress) + return m.resolveLocalBaseRef(ctx, repoPath, baseBranch, localBranch, stepName, onProgress, localBaseExists) } func (m *Manager) reportSyncProgress(cb SyncProgressCallback, event SyncProgressEvent) { @@ -356,6 +417,56 @@ func (m *Manager) reportSyncCompleted(stepName string, onProgress SyncProgressCa }) } +func (m *Manager) completeSyncWithWarning( + ctx context.Context, + stepName, baseBranch, operation, reason, selectedRef string, + onProgress SyncProgressCallback, cause error, +) (string, error) { + if ctxErr := ctx.Err(); ctxErr != nil { + return "", m.failRequiredSync(stepName, baseBranch, onProgress, syncContextFailureReason(ctxErr), ctxErr) + } + + m.logger.Warn("git refresh was incomplete; using selected base", + zap.String("branch", baseBranch), + zap.String("reason", reason), + zap.String("fallback_ref", selectedRef), + zap.Error(cause)) + warning, detail := localBaseRefreshWarning(reason, selectedRef) + m.reportSyncProgress(onProgress, SyncProgressEvent{ + StepName: stepName, + Status: SyncProgressCompleted, + Output: fmt.Sprintf("%s %s; using %s", operation, reason, selectedRef), + Warning: warning, + WarningDetail: detail, + }) + return selectedRef, nil +} + +func localBaseRefreshWarning(reason, selectedRef string) (string, string) { + baseKind := "local base" + if isRemoteOnlyBaseRef(selectedRef) { + baseKind = "remote-tracking base" + } + return fmt.Sprintf( + "Remote refresh was incomplete (%s); using %s %q. Remote changes may be missing.", + reason, baseKind, selectedRef, + ), fmt.Sprintf("The selected %s was verified before refresh. Kandev did not change Git refs.", baseKind) +} + +func localCheckoutBranchRefreshDetail(branch string) string { + return fmt.Sprintf("The local checkout branch %q was verified after refresh failed. Kandev did not change Git refs.", branch) +} + +func syncContextFailureReason(err error) string { + if errors.Is(err, context.DeadlineExceeded) { + return "timeout" + } + if errors.Is(err, context.Canceled) { + return "canceled" + } + return "git_command_failed" +} + func (m *Manager) failRequiredSync( stepName, branch string, onProgress SyncProgressCallback, reason string, err error, ) error { @@ -374,14 +485,20 @@ func (m *Manager) failRequiredSync( func (m *Manager) resolveLocalBaseRef( ctx context.Context, repoPath, baseBranch, localBranch, stepName string, - onProgress SyncProgressCallback, + onProgress SyncProgressCallback, localBaseExists bool, ) (string, error) { remoteRef := "origin/" + localBranch if m.currentBranch(ctx, repoPath) == baseBranch { - return m.pullCurrentBranchOrFallback(ctx, repoPath, baseBranch, remoteRef, stepName, onProgress) + return m.pullCurrentBranchOrFallback(ctx, repoPath, baseBranch, remoteRef, stepName, onProgress, localBaseExists) } resolved, err := m.selectContainingRef(ctx, repoPath, baseBranch, remoteRef) if err != nil { + if localBaseExists { + return m.completeSyncWithWarning( + ctx, stepName, baseBranch, "Base ref selection", classifyBaseRefSelectionFailure(err), + baseBranch, onProgress, err, + ) + } return "", m.failRequiredSync(stepName, baseBranch, onProgress, "base_ref_unverified", err) } m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", resolved), "") @@ -390,7 +507,7 @@ func (m *Manager) resolveLocalBaseRef( func (m *Manager) pullCurrentBranchOrFallback( ctx context.Context, repoPath, baseBranch, remoteRef, stepName string, - onProgress SyncProgressCallback, + onProgress SyncProgressCallback, localBaseExists bool, ) (string, error) { // Same Acquire-then-build-execCtx ordering as the fetch path. output, err, execCtxErr := m.runGitCombinedAfterAcquire(ctx, m.pullTimeout, repoPath, "pull", "--ff-only", "origin", baseBranch) @@ -398,15 +515,35 @@ func (m *Manager) pullCurrentBranchOrFallback( reason := classifyGitFallbackReason(err, string(output), execCtxErr) resolved, selectErr := m.selectContainingRef(ctx, repoPath, baseBranch, remoteRef) if selectErr != nil { - return "", m.failRequiredSync(stepName, baseBranch, onProgress, reason, selectErr) + if !localBaseExists { + return "", m.failRequiredSync(stepName, baseBranch, onProgress, reason, selectErr) + } + return m.completeSyncWithWarning( + ctx, stepName, baseBranch, "Pull", classifyBaseRefSelectionFailure(selectErr), + baseBranch, onProgress, selectErr, + ) } - m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Pull %s; using %s", reason, resolved), "") - return resolved, nil + if localBaseExists { + return m.completeSyncWithWarning(ctx, stepName, baseBranch, "Pull", reason, resolved, onProgress, err) + } + return "", m.failRequiredSync(stepName, baseBranch, onProgress, reason, err) } m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", baseBranch), "") return baseBranch, nil } +func classifyBaseRefSelectionFailure(err error) string { + lowerErr := strings.ToLower(err.Error()) + switch { + case strings.Contains(lowerErr, "diverged"): + return "diverged_refs" + case strings.Contains(lowerErr, "missing"): + return "missing_remote_ref" + default: + return "base_ref_unverified" + } +} + func (m *Manager) selectContainingRef( ctx context.Context, repoPath, localRef, remoteRef string, ) (string, error) { diff --git a/apps/backend/internal/worktree/manager_lifecycle.go b/apps/backend/internal/worktree/manager_lifecycle.go index a5046cda8f..01eff08359 100644 --- a/apps/backend/internal/worktree/manager_lifecycle.go +++ b/apps/backend/internal/worktree/manager_lifecycle.go @@ -176,7 +176,7 @@ func (m *Manager) refreshRepositoryForMaterialization(ctx context.Context, req * if req.RefreshRepositoryWithState != nil { state, err := req.RefreshRepositoryWithState(ctx) if err != nil { - return err + return m.handleProviderRefreshFailure(ctx, req, err) } req.RemoteRefState = state req.RemoteSyncHandled = true @@ -187,13 +187,60 @@ func (m *Manager) refreshRepositoryForMaterialization(ctx context.Context, req * return nil } if err := req.RefreshRepository(ctx); err != nil { - return err + return m.handleProviderRefreshFailure(ctx, req, err) } req.RemoteSyncHandled = true req.PullBeforeWorktree = false return nil } +// handleProviderRefreshFailure keeps host worktree creation local-first when +// a provider refresh fails after a usable local base has been verified. An +// explicit remote ref, a missing local base, and caller cancellation remain +// strict because they require remote materialization or a terminal stop. +func (m *Manager) handleProviderRefreshFailure(ctx context.Context, req *CreateRequest, refreshErr error) error { + if ctxErr := ctx.Err(); ctxErr != nil || errors.Is(refreshErr, context.Canceled) { + if ctxErr != nil { + return ctxErr + } + return refreshErr + } + if isRemoteOnlyBaseRef(req.BaseBranch) { + return refreshErr + } + localExists, err := m.branchExists(ctx, req.RepositoryPath, req.BaseBranch) + if err != nil || !localExists { + return refreshErr + } + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + + reason := classifyGitFallbackReason(refreshErr, refreshErr.Error(), nil) + warning, detail := localBaseRefreshWarning(reason, req.BaseBranch) + req.baseRefreshFallback = true + req.baseRefreshFallbackWarning = warning + req.baseRefreshFallbackDetail = detail + m.logger.Warn("provider refresh was incomplete; using local base", + zap.String("repository_path", req.RepositoryPath), + zap.String("branch", req.BaseBranch), + zap.String("reason", reason), + zap.String("fallback_ref", req.BaseBranch)) + m.reportSyncProgress(req.OnSyncProgress, SyncProgressEvent{ + StepName: "Sync base branch", + Status: SyncProgressCompleted, + Output: fmt.Sprintf("Provider refresh incomplete (%s); using local base %s", reason, req.BaseBranch), + Warning: warning, + WarningDetail: detail, + }) + return nil +} + +func isRemoteOnlyBaseRef(branch string) bool { + branch = strings.TrimSpace(branch) + return strings.HasPrefix(branch, "origin/") || strings.HasPrefix(branch, "refs/remotes/") +} + func (m *Manager) ensureEmptyRemoteBaseline(ctx context.Context, req *CreateRequest) (string, error) { if req == nil || req.RemoteRefState != repoclone.RemoteRefStateEmpty { return "", nil @@ -504,7 +551,14 @@ func requestBranchIdentitySlug(req CreateRequest) string { // returned for surfacing on the resulting worktree record. func (m *Manager) resolveBaseRefWithFallback(ctx context.Context, req *CreateRequest) (baseRef, warning, detail string, err error) { baseRef = req.BaseBranch + warning = req.baseRefreshFallbackWarning + detail = req.baseRefreshFallbackDetail switch { + case req.baseRefreshFallback: + // The provider refresh already failed after verifying the local base. + // Keep PullBeforeWorktree true so a requested checkout branch still + // follows its own materialization policy, but do not refresh the base a + // second time through the unauthenticated Git path. case req.RemoteRefState == repoclone.RemoteRefStateEmpty: baselineRef, baselineErr := m.ensureEmptyRemoteBaseline(ctx, req) if baselineErr != nil { @@ -514,12 +568,12 @@ func (m *Manager) resolveBaseRefWithFallback(ctx context.Context, req *CreateReq case baselineRef != "": baseRef = baselineRef case req.RemoteSyncHandled: - baseRef, err = m.preferRefreshedRemoteRef(ctx, req.RepositoryPath, req.BaseBranch) + baseRef, warning, detail, err = m.resolveRefreshedBaseRefWithFallback(ctx, req.RepositoryPath, req.BaseBranch) case req.PullBeforeWorktree: baseRef, err = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) } case req.RemoteSyncHandled: - baseRef, err = m.preferRefreshedRemoteRef(ctx, req.RepositoryPath, req.BaseBranch) + baseRef, warning, detail, err = m.resolveRefreshedBaseRefWithFallback(ctx, req.RepositoryPath, req.BaseBranch) case req.PullBeforeWorktree: baseRef, err = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) } @@ -534,7 +588,7 @@ func (m *Manager) resolveBaseRefWithFallback(ctx context.Context, req *CreateReq return "", "", "", fmt.Errorf("could not verify base branch %q: %w", baseRef, baseErr) } if baseExists { - return baseRef, "", "", nil + return baseRef, warning, detail, nil } fallback := strings.TrimSpace(req.FallbackBaseBranch) @@ -583,7 +637,8 @@ 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 { - return m.preferRefreshedRemoteRef(ctx, req.RepositoryPath, fallback) + resolved, _, _, err := m.resolveRefreshedBaseRefWithFallback(ctx, req.RepositoryPath, fallback) + return resolved, err } if req.PullBeforeWorktree { return m.pullBaseBranch(ctx, req.RepositoryPath, fallback, nil) @@ -660,10 +715,11 @@ func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRe // origin, the caller's intent is "create a new branch with this // name" rather than "fetch this existing ref" — the historical // fetch-then-check-out path errored ("not found locally or on - // remote") in that case and rolled back. We drop CheckoutBranch - // from the request copy and pass the desired name as the fallback - // (new) branch name so gitAddWorktree creates it from baseRef. - if req.PRNumber == 0 && !m.checkoutBranchExistsAnywhere(ctx, req.RepositoryPath, req.CheckoutBranch) { + // remote") in that case and rolled back. We keep that behavior for + // ordinary branch creation, but a provider refresh fallback must + // still materialize the requested checkout branch or fail. The + // provider may have been the only route to the remote ref. + if req.PRNumber == 0 && !req.baseRefreshFallback && !m.checkoutBranchExistsAnywhere(ctx, req.RepositoryPath, req.CheckoutBranch) { m.logger.Info("checkout branch missing locally and on origin; creating new branch with this name", zap.String("repository_path", req.RepositoryPath), zap.String("requested_branch", req.CheckoutBranch), @@ -671,8 +727,16 @@ func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRe branchName = req.CheckoutBranch checkoutMode.CheckoutBranch = "" } else { + checkoutRefreshRequired := req.PullBeforeWorktree + if req.baseRefreshFallback && req.PRNumber == 0 { + // A provider refresh failure does not make an existing local + // checkout branch remote-only. Let the fetch helper verify and + // reuse that local branch when its refresh also fails. If the + // branch is absent locally, the helper still returns an error. + checkoutRefreshRequired = false + } fetchResult, err = m.fetchBranchToLocalWithPolicy( - ctx, req.RepositoryPath, req.CheckoutBranch, req.PRNumber, req.PullBeforeWorktree, + ctx, req.RepositoryPath, req.CheckoutBranch, req.PRNumber, checkoutRefreshRequired, ) if err != nil { return nil, err @@ -682,6 +746,9 @@ func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRe } else { startPoint = req.CheckoutBranch } + if req.baseRefreshFallback && fetchResult.Warning != "" { + fetchResult.WarningDetail = localCheckoutBranchRefreshDetail(req.CheckoutBranch) + } } } } @@ -1696,14 +1763,21 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe if err := m.refreshRepositoryForMaterialization(ctx, &req); err != nil { return nil, err } + fallbackWarning := req.baseRefreshFallbackWarning + fallbackDetail := req.baseRefreshFallbackDetail // Recreate bypasses the new-worktree path, so perform the same required // base refresh before touching the existing worktree path. A failed refresh // must leave the retryable on-disk state intact. if req.PullBeforeWorktree && !req.RemoteSyncHandled { refreshReq := req - if _, _, _, err := m.resolveBaseRefWithFallback(ctx, &refreshReq); err != nil { + _, warning, detail, err := m.resolveBaseRefWithFallback(ctx, &refreshReq) + if err != nil { return nil, err } + if warning != "" { + fallbackWarning = warning + fallbackDetail = detail + } } // Clean up the recorded directory through no-follow descriptors. Reuse @@ -1855,6 +1929,8 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe existing.Status = StatusActive existing.DeletedAt = nil existing.UpdatedAt = now + existing.BaseBranchFallbackWarning = fallbackWarning + existing.BaseBranchFallbackDetail = fallbackDetail if m.store != nil { if err := m.store.UpdateWorktree(ctx, existing); err != nil { diff --git a/apps/backend/internal/worktree/manager_pull_fallback_test.go b/apps/backend/internal/worktree/manager_pull_fallback_test.go index 18d2baf797..5b998e4a9b 100644 --- a/apps/backend/internal/worktree/manager_pull_fallback_test.go +++ b/apps/backend/internal/worktree/manager_pull_fallback_test.go @@ -2,10 +2,14 @@ package worktree import ( "context" + "errors" + "fmt" "os" "path/filepath" "strings" "testing" + + "github.com/kandev/kandev/internal/repoclone" ) // initGitRepoWithOriginAheadLocal builds the shape that matters here: a normal @@ -43,6 +47,44 @@ func initGitRepoWithOriginAheadLocal(t *testing.T) (string, string) { return repoPath, localHead } +func initGitRepoWithDivergedOrigin(t *testing.T) (string, string, string) { + t.Helper() + + base := t.TempDir() + originPath := filepath.Join(base, "origin.git") + repoPath := filepath.Join(base, "repo") + otherPath := filepath.Join(base, "other") + + runGit(t, base, "init", "--bare", "-b", "main", originPath) + runGit(t, base, "init", "-b", "main", repoPath) + runGit(t, repoPath, "config", "user.email", "test@example.com") + runGit(t, repoPath, "config", "user.name", "Test User") + runGit(t, repoPath, "config", "commit.gpgsign", "false") + writeRepoFile(t, repoPath, "README.md", "initial\n") + runGit(t, repoPath, "add", "README.md") + runGit(t, repoPath, "commit", "-m", "initial commit") + runGit(t, repoPath, "remote", "add", "origin", originPath) + runGit(t, repoPath, "push", "origin", "main") + + writeRepoFile(t, repoPath, "local-only.txt", "local\n") + runGit(t, repoPath, "add", "local-only.txt") + runGit(t, repoPath, "commit", "-m", "local-only commit") + localHead := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "main")) + + runGit(t, base, "clone", originPath, otherPath) + runGit(t, otherPath, "config", "user.email", "other@example.com") + runGit(t, otherPath, "config", "user.name", "Other User") + runGit(t, otherPath, "config", "commit.gpgsign", "false") + writeRepoFile(t, otherPath, "remote-only.txt", "remote\n") + runGit(t, otherPath, "add", "remote-only.txt") + runGit(t, otherPath, "commit", "-m", "remote-only commit") + runGit(t, otherPath, "push", "origin", "main") + runGit(t, repoPath, "fetch", "origin", "main") + remoteHead := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "origin/main")) + + return repoPath, localHead, remoteHead +} + // captureSyncProgress records every SyncProgressEvent so a test can assert // which branch of the sync actually ran, not merely what it returned. func captureSyncProgress(events *[]SyncProgressEvent) SyncProgressCallback { @@ -58,6 +100,345 @@ func syncOutputs(events []SyncProgressEvent) string { return strings.Join(parts, " | ") } +func TestCreateWorktree_PullEnabledUsesLocalOnlyBase(t *testing.T) { + repoPath, localHead := initGitRepoWithOriginAheadLocal(t) + // Keep the local branch usable while making the refresh itself fail. The + // warning assertions below ensure Git's credential-bearing failure output + // does not cross the sync-progress boundary. + runGit(t, repoPath, "remote", "set-url", "origin", "https://user:super-secret@127.0.0.1:1/missing.git") + + 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-local-base", + SessionID: "session-local-base", + RepositoryID: "repo-local-base", + RepositoryPath: repoPath, + BaseBranch: "main", + PullBeforeWorktree: true, + TaskDirName: "task-local-base", + RepoName: "repo", + OnSyncProgress: captureSyncProgress(&events), + }) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if wt == nil { + t.Fatal("Create() returned a nil worktree") + } + + gotHead := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")) + if gotHead != localHead { + t.Fatalf("worktree HEAD = %s, want local-only base %s", gotHead, localHead) + } + if len(events) != 2 || events[0].Status != SyncProgressRunning || events[1].Status != SyncProgressCompleted { + t.Fatalf("sync progress = %#v, want running then completed", events) + } + if events[1].Warning == "" || events[1].WarningDetail == "" { + t.Fatalf("sync progress = %#v, want bounded fallback warning and detail", events) + } + if strings.Contains(syncOutputs(events), "super-secret") { + t.Fatalf("sync progress exposed credential-bearing Git output: %#v", events) + } +} + +func TestPullBaseBranch_MissingRemoteRefUsesLocalBase(t *testing.T) { + repoPath, localHead := initGitRepoWithOriginAheadLocal(t) + runGit(t, repoPath, "branch", "local-only-base") + + mgr := newRecreateTestManager(t) + var events []SyncProgressEvent + resolved, err := mgr.pullBaseBranch(context.Background(), repoPath, "local-only-base", captureSyncProgress(&events)) + if err != nil { + t.Fatalf("pullBaseBranch() error = %v", err) + } + if resolved != "local-only-base" { + t.Fatalf("resolved ref = %q, want local-only-base", resolved) + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", resolved)); got != localHead { + t.Fatalf("resolved SHA = %q, want local-only commit %q", got, localHead) + } + if len(events) != 2 || events[1].Status != SyncProgressCompleted || events[1].Warning == "" { + t.Fatalf("sync progress = %#v, want completed warning", events) + } + if !strings.Contains(events[1].Output, "missing_remote_ref") { + t.Fatalf("sync output = %q, want missing_remote_ref classification", events[1].Output) + } +} + +func TestPullBaseBranch_DivergedRefsUseLocalBaseWithWarning(t *testing.T) { + repoPath, localHead, remoteHead := initGitRepoWithDivergedOrigin(t) + mgr := newRecreateTestManager(t) + + var events []SyncProgressEvent + resolved, err := mgr.pullBaseBranch(context.Background(), repoPath, "main", captureSyncProgress(&events)) + if err != nil { + t.Fatalf("pullBaseBranch() error = %v", err) + } + if resolved != "main" { + t.Fatalf("resolved ref = %q, want local main", resolved) + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "main")); got != localHead { + t.Fatalf("local main moved to %q, want %q", got, localHead) + } + if got := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "origin/main")); got != remoteHead { + t.Fatalf("origin/main moved to %q, want %q", got, remoteHead) + } + if len(events) != 2 || events[1].Status != SyncProgressCompleted || events[1].Warning == "" { + t.Fatalf("sync progress = %#v, want completed warning", events) + } + if !strings.Contains(events[1].Output, "diverged_refs") { + t.Fatalf("sync output = %q, want diverged_refs classification", events[1].Output) + } +} + +func TestCreateWorktree_ManagedRefreshDivergenceUsesLocalBaseWarning(t *testing.T) { + repoPath, localHead, _ := initGitRepoWithDivergedOrigin(t) + mgr := newRecreateTestManager(t) + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-managed-divergence", + SessionID: "session-managed-divergence", + RepositoryID: "repo-managed-divergence", + RepositoryPath: repoPath, + BaseBranch: "main", + PullBeforeWorktree: true, + RemoteSyncHandled: true, + TaskDirName: "task-managed-divergence", + RepoName: "repo", + }) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if wt.BaseBranchFallbackWarning == "" || wt.BaseBranchFallbackDetail == "" { + t.Fatalf("worktree fallback warning = %#v, want warning and detail", wt) + } + if strings.Contains(wt.BaseBranchFallbackWarning, "fatal") || strings.Contains(wt.BaseBranchFallbackDetail, "fatal") { + t.Fatalf("worktree fallback warning exposed raw Git output: %#v", wt) + } + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")); got != localHead { + t.Fatalf("managed-refresh worktree HEAD = %q, want local base %q", got, localHead) + } +} + +func TestCreateWorktree_ManagedRefreshFailureUsesLocalBaseWarning(t *testing.T) { + repoPath, localHead := initGitRepoWithOriginAheadLocal(t) + mgr := newRecreateTestManager(t) + refreshCalls := 0 + var events []SyncProgressEvent + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-managed-refresh-failure", + SessionID: "session-managed-refresh-failure", + RepositoryID: "repo-managed-refresh-failure", + RepositoryPath: repoPath, + BaseBranch: "main", + PullBeforeWorktree: true, + RefreshRepositoryWithState: func(context.Context) (repoclone.RemoteRefState, error) { + refreshCalls++ + return repoclone.RemoteRefStateUnknown, errors.New("authentication failed for https://user:super-secret@example.invalid/repo.git") + }, + TaskDirName: "task-managed-refresh-failure", + RepoName: "repo", + OnSyncProgress: captureSyncProgress(&events), + }) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if refreshCalls != 1 { + t.Fatalf("refresh calls = %d, want one attempted provider refresh", refreshCalls) + } + if wt.BaseBranchFallbackWarning == "" || wt.BaseBranchFallbackDetail == "" { + t.Fatalf("worktree fallback warning = %#v, want warning and detail", wt) + } + if strings.Contains(wt.BaseBranchFallbackWarning, "super-secret") || strings.Contains(wt.BaseBranchFallbackDetail, "super-secret") { + t.Fatalf("worktree fallback warning exposed provider credentials: %#v", wt) + } + if len(events) != 1 || events[0].Status != SyncProgressCompleted || events[0].Warning == "" { + t.Fatalf("provider refresh progress = %#v, want one completed warning", events) + } + if strings.Contains(syncOutputs(events), "super-secret") { + t.Fatalf("provider refresh progress exposed provider credentials: %#v", events) + } + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")); got != localHead { + t.Fatalf("managed-refresh fallback HEAD = %q, want local base %q", got, localHead) + } +} + +func TestCreateWorktree_ManagedRefreshFailureKeepsMissingCheckoutBranchStrict(t *testing.T) { + repoPath, _ := initGitRepoWithOriginAheadLocal(t) + mgr := newRecreateTestManager(t) + + _, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-managed-missing-checkout", + SessionID: "session-managed-missing-checkout", + RepositoryID: "repo-managed-missing-checkout", + RepositoryPath: repoPath, + BaseBranch: "main", + CheckoutBranch: "feature/remote-only", + PullBeforeWorktree: true, + RefreshRepository: func(context.Context) error { + return errors.New("provider refresh unavailable") + }, + TaskDirName: "task-managed-missing-checkout", + RepoName: "repo", + }) + if !errors.Is(err, ErrInvalidBaseBranch) { + t.Fatalf("Create() error = %v, want missing checkout branch to remain strict", err) + } +} + +func TestCreateWorktree_ManagedRefreshFailureUsesLocalCheckoutBranch(t *testing.T) { + repoPath, _ := initGitRepoWithOriginAheadLocal(t) + checkoutBranch := "feature/local-only" + runGit(t, repoPath, "checkout", "-b", checkoutBranch) + writeRepoFile(t, repoPath, "local-checkout-only.txt", "unpushed checkout work\n") + runGit(t, repoPath, "add", "local-checkout-only.txt") + runGit(t, repoPath, "commit", "-m", "local-only checkout commit") + localCheckoutHead := strings.TrimSpace(runGit(t, repoPath, "rev-parse", checkoutBranch)) + runGit(t, repoPath, "checkout", "main") + runGit(t, repoPath, "remote", "set-url", "origin", "https://user:super-secret@127.0.0.1:1/missing.git") + + mgr := newRecreateTestManager(t) + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-managed-local-checkout", + SessionID: "session-managed-local-checkout", + RepositoryID: "repo-managed-local-checkout", + RepositoryPath: repoPath, + BaseBranch: "main", + CheckoutBranch: checkoutBranch, + PullBeforeWorktree: true, + RefreshRepository: func(context.Context) error { + return errors.New("provider refresh unavailable") + }, + TaskDirName: "task-managed-local-checkout", + RepoName: "repo", + }) + if err != nil { + t.Fatalf("Create() error = %v, want the existing local checkout branch", err) + } + if wt.Branch != checkoutBranch { + t.Fatalf("worktree branch = %q, want %q", wt.Branch, checkoutBranch) + } + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")); got != localCheckoutHead { + t.Fatalf("worktree HEAD = %q, want local checkout HEAD %q", got, localCheckoutHead) + } + if wt.FetchWarning == "" { + t.Fatal("local checkout fallback did not surface a fetch warning") + } + wantDetail := fmt.Sprintf("The local checkout branch %q was verified after refresh failed. Kandev did not change Git refs.", checkoutBranch) + if wt.FetchWarningDetail != wantDetail { + t.Fatalf("fetch warning detail = %q, want bounded detail %q", wt.FetchWarningDetail, wantDetail) + } + if strings.Contains(wt.FetchWarning, "super-secret") || strings.Contains(wt.FetchWarningDetail, "super-secret") { + t.Fatalf("checkout fallback warning exposed provider credentials: warning=%q detail=%q", wt.FetchWarning, wt.FetchWarningDetail) + } +} + +func TestCreateWorktree_ManagedRefreshFailureKeepsRemoteOnlyStrict(t *testing.T) { + repoPath, _ := initGitRepoWithOriginAheadLocal(t) + mgr := newRecreateTestManager(t) + refreshErr := errors.New("authentication failed for https://user:super-secret@example.invalid/repo.git") + + _, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-managed-remote-only", + SessionID: "session-managed-remote-only", + RepositoryID: "repo-managed-remote-only", + RepositoryPath: repoPath, + BaseBranch: "origin/main", + PullBeforeWorktree: true, + RefreshRepository: func(context.Context) error { + return refreshErr + }, + TaskDirName: "task-managed-remote-only", + RepoName: "repo", + }) + if !errors.Is(err, refreshErr) { + t.Fatalf("Create() error = %v, want the provider refresh error", err) + } +} + +func TestPullBaseBranch_UncertainAncestryUsesLocalBaseWithWarning(t *testing.T) { + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + fetch) + exit 0 + ;; + pull) + exit 1 + ;; + rev-parse) + if [ "${2:-}" = "--abbrev-ref" ]; then + echo "main" + fi + exit 0 + ;; + merge-base) + echo "fatal: unable to inspect ancestry" >&2 + exit 128 + ;; + *) + exit 0 + ;; +esac +`) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + mgr := newRecreateTestManager(t) + var events []SyncProgressEvent + resolved, err := mgr.pullBaseBranch(context.Background(), t.TempDir(), "main", captureSyncProgress(&events)) + if err != nil { + t.Fatalf("pullBaseBranch() error = %v", err) + } + if resolved != "main" { + t.Fatalf("resolved ref = %q, want local main", resolved) + } + if len(events) != 2 || events[1].Status != SyncProgressCompleted || events[1].Warning == "" { + t.Fatalf("sync progress = %#v, want completed warning", events) + } + if !strings.Contains(events[1].Output, "base_ref_unverified") { + t.Fatalf("sync output = %q, want base_ref_unverified classification", events[1].Output) + } + if strings.Contains(events[1].WarningDetail, "unable to inspect") { + t.Fatalf("warning detail exposed raw ancestry output: %q", events[1].WarningDetail) + } +} + +func TestCreateWorktree_CancelledRefreshDoesNotCreateFallback(t *testing.T) { + repoPath, _ := initGitRepoWithOriginAheadLocal(t) + mgr := newRecreateTestManager(t) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + wt, err := mgr.Create(ctx, CreateRequest{ + TaskID: "task-cancelled-refresh", + SessionID: "session-cancelled-refresh", + RepositoryID: "repo-cancelled-refresh", + RepositoryPath: repoPath, + BaseBranch: "main", + PullBeforeWorktree: true, + TaskDirName: "task-cancelled-refresh", + RepoName: "repo", + }) + if wt != nil { + t.Fatalf("Create() returned a worktree after cancellation: %#v", wt) + } + if !errors.Is(err, context.Canceled) { + t.Fatalf("Create() error = %v, want context.Canceled", err) + } + path, pathErr := mgr.config.TaskWorktreePath("task-cancelled-refresh", "repo", "") + if pathErr != nil { + t.Fatalf("TaskWorktreePath() error = %v", pathErr) + } + if _, statErr := os.Stat(path); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("cancelled Create() left fallback worktree path: %v", statErr) + } +} + // TestPullBaseBranch_PullFailureKeepsLocalCommits pins the behaviour that a // transient `git pull` failure must not silently rebase the worktree's starting // point onto origin/. @@ -119,7 +500,7 @@ func TestPullBaseBranch_PullFailureKeepsLocalCommits(t *testing.T) { } } -func TestResolveBaseRefWithFallback_RequiredRefreshFailureStopsPreparation(t *testing.T) { +func TestResolveBaseRefWithFallback_RefreshFailureUsesLocalBase(t *testing.T) { repoPath, _ := initGitRepoWithOriginAheadLocal(t) mgr, err := NewManager(newTestConfig(t), newMockStore(), newTestLogger()) if err != nil { @@ -143,15 +524,21 @@ esac } var events []SyncProgressEvent req.OnSyncProgress = captureSyncProgress(&events) - _, _, _, err = mgr.resolveBaseRefWithFallback(context.Background(), &req) - if err == nil { - t.Fatal("resolveBaseRefWithFallback() succeeded after required refresh failure") + baseRef, _, _, err := mgr.resolveBaseRefWithFallback(context.Background(), &req) + if err != nil { + t.Fatalf("resolveBaseRefWithFallback() error = %v", err) } - if len(events) != 2 || events[0].Status != SyncProgressRunning || events[1].Status != SyncProgressFailed { - t.Fatalf("required refresh progress = %#v, want running then failed", events) + if baseRef != "main" { + t.Fatalf("resolved base ref = %q, want local main", baseRef) } - if events[1].Error == "" || strings.Contains(events[1].Error, "Authentication") { - t.Fatalf("required refresh progress exposed unsafe error detail: %#v", events[1]) + if len(events) != 2 || events[0].Status != SyncProgressRunning || events[1].Status != SyncProgressCompleted { + t.Fatalf("refresh progress = %#v, want running then completed", events) + } + if events[1].Warning == "" || events[1].WarningDetail == "" { + t.Fatalf("refresh fallback warning = %#v, want warning and detail", events[1]) + } + if strings.Contains(events[1].Warning, "Authentication") || strings.Contains(events[1].WarningDetail, "Authentication") { + t.Fatalf("refresh progress exposed unsafe error detail: %#v", events[1]) } } diff --git a/apps/backend/internal/worktree/manager_recreate_recovery_test.go b/apps/backend/internal/worktree/manager_recreate_recovery_test.go index aa3fb8a9bd..d3d1125bac 100644 --- a/apps/backend/internal/worktree/manager_recreate_recovery_test.go +++ b/apps/backend/internal/worktree/manager_recreate_recovery_test.go @@ -67,25 +67,14 @@ func TestRecreate_FetchesBranchFromOriginWhenLocalDeleted(t *testing.T) { } } -func TestRecreate_RequiredBaseRefreshFailureStopsPreparation(t *testing.T) { +func TestRecreate_RefreshFailureUsesLocalBase(t *testing.T) { repoPath := initGitRepoWithRemote(t) worktreePath := filepath.Join(t.TempDir(), "task-required", "repo-1") if err := os.MkdirAll(worktreePath, 0755); err != nil { t.Fatalf("create worktree placeholder: %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")) + branchSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/pr-branch")) + runGit(t, repoPath, "remote", "set-url", "origin", "https://user:super-secret@127.0.0.1:1/missing.git") mgr := newRecreateTestManager(t) existing := &Worktree{ @@ -107,11 +96,15 @@ esac BaseBranch: "main", PullBeforeWorktree: true, }) - if err == nil { - t.Fatal("recreate() succeeded after required base refresh failure") + if err != nil { + t.Fatalf("recreate() error = %v", err) } if _, statErr := os.Stat(worktreePath); statErr != nil { - t.Fatalf("required refresh failure removed the existing worktree path: %v", statErr) + t.Fatalf("recreate() did not restore the worktree path: %v", statErr) + } + gotSHA := strings.TrimSpace(runGit(t, worktreePath, "rev-parse", "HEAD")) + if gotSHA != branchSHA { + t.Fatalf("recreated worktree HEAD = %q, want local branch %q", gotSHA, branchSHA) } } diff --git a/apps/backend/internal/worktree/manager_test.go b/apps/backend/internal/worktree/manager_test.go index 165a081c31..3fb345b9af 100644 --- a/apps/backend/internal/worktree/manager_test.go +++ b/apps/backend/internal/worktree/manager_test.go @@ -799,6 +799,9 @@ case "${1:-}" in exit 0 ;; rev-parse) + if [ "${2:-}" = "--verify" ]; then + exit 1 + fi if [ "${2:-}" = "--abbrev-ref" ]; then echo "master" fi diff --git a/apps/backend/internal/worktree/worktree.go b/apps/backend/internal/worktree/worktree.go index cf840f4045..47a6cb6c43 100644 --- a/apps/backend/internal/worktree/worktree.go +++ b/apps/backend/internal/worktree/worktree.go @@ -20,10 +20,12 @@ const ( // SyncProgressEvent reports pre-worktree base-branch synchronization progress. type SyncProgressEvent struct { - StepName string - Status SyncProgressStatus - Output string - Error string + StepName string + Status SyncProgressStatus + Output string + Error string + Warning string + WarningDetail string } // SyncProgressCallback is called when base-branch sync status changes. @@ -103,10 +105,9 @@ type Worktree struct { // Shown as collapsible content alongside the user-friendly FetchWarning. FetchWarningDetail string `json:"fetch_warning_detail,omitempty"` - // BaseBranchFallbackWarning is set when the requested BaseBranch did not - // exist in the repository and the worktree was created from a fallback - // branch (typically the repository's default_branch) instead. Empty when - // the original BaseBranch was used. + // BaseBranchFallbackWarning is set when the requested base was unavailable + // or could not be refreshed and the worktree used a verified local fallback. + // Empty when the requested base was used after a successful refresh. BaseBranchFallbackWarning string `json:"base_branch_fallback_warning,omitempty"` // BaseBranchFallbackDetail mirrors FetchWarningDetail: a longer message @@ -233,6 +234,14 @@ type CreateRequest struct { // used for this materialization. RemoteRefState repoclone.RemoteRefState + // These fields are manager-internal state used when a provider refresh + // fails after a local base was verified. They keep the original refresh + // policy for checkout-branch materialization while preventing a second + // unauthenticated base refresh. + baseRefreshFallback bool + baseRefreshFallbackWarning string + baseRefreshFallbackDetail string + // WorktreeID is the ID of an existing worktree to reuse (optional). // If provided and valid, the existing worktree is returned instead of creating a new one. WorktreeID string diff --git a/apps/web/components/integrations/change-request-detail.test.tsx b/apps/web/components/integrations/change-request-detail.test.tsx index 9236a3e080..3a89364dc7 100644 --- a/apps/web/components/integrations/change-request-detail.test.tsx +++ b/apps/web/components/integrations/change-request-detail.test.tsx @@ -211,9 +211,9 @@ describe("ChangeRequestDetail copy actions", () => { 3, "https://bitbucket.org/workspace/repo/pull-requests/42#comment-2", ); + expect(requestCopy.getAttribute(ARIA_LABEL_ATTRIBUTE)).toBe(REQUEST_COPIED_LABEL); + expect(rootCopy.getAttribute(ARIA_LABEL_ATTRIBUTE)).toBe(COMMENT_COPIED_LABEL); }); - expect(requestCopy.getAttribute(ARIA_LABEL_ATTRIBUTE)).toBe(REQUEST_COPIED_LABEL); - expect(rootCopy.getAttribute(ARIA_LABEL_ATTRIBUTE)).toBe(COMMENT_COPIED_LABEL); }); it("omits copy actions when a request or comment URL is unavailable", () => { 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 1937a9f368..e2e09d48bf 100644 --- a/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts +++ b/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts @@ -282,7 +282,7 @@ test.describe("task launch failure recovery", () => { } }); - test("persists a required refresh failure and retries after the origin recovers", async ({ + test("starts from the local base when origin refresh fails", async ({ testPage, apiClient, seedData, @@ -293,11 +293,11 @@ test.describe("task launch failure recovery", () => { const { workflow, waiting, review } = await recoveryWorkflow( apiClient, seedData.workspaceId, - `Required refresh recovery ${Date.now()}`, + "Local base refresh", ); const task = await apiClient.createTask( seedData.workspaceId, - "Required refresh failure recovery fixture", + "Local base refresh fallback fixture", { description: "/e2e:simple-message", workflow_id: workflow.id, @@ -307,9 +307,6 @@ test.describe("task launch failure recovery", () => { repositories: [{ repository_id: seedData.repositoryId, base_branch: "main" }], }, ); - const storedTask = await apiClient.getTask(task.id); - const taskRepository = storedTask.repositories?.[0]; - if (!taskRepository) throw new Error("refresh fixture did not create a task repository row"); pointSeedRepositoryAtFailingOrigin(seedData, backend.tmpDir); try { @@ -318,28 +315,6 @@ test.describe("task launch failure recovery", () => { const session = new SessionPage(testPage); await session.waitForLoad(); - const launchError = await waitForTaskLaunchError(apiClient, seedData.workspaceId, task.id); - expect(launchError.category).toBe("generic_launch_failure"); - expect(launchError.task_repository_id).toBe(taskRepository.id); - expect(launchError.recovery_actions).toEqual(["retry_default", "pick_base_branch"]); - - const card = testPage.getByTestId("task-launch-error-entry"); - await expect(card).toHaveCount(1, { timeout: 30_000 }); - await expect(card).toContainText("The task could not start."); - - await testPage.reload(); - await session.waitForLoad(); - await expect(testPage.getByTestId("task-launch-error-entry")).toHaveCount(1); - - restoreSeedRepositoryOrigin(seedData); - await testPage.getByTestId("task-launch-pick_base_branch-button").click(); - await expect(testPage.getByTestId("task-launch-branch-picker-option-main")).toBeVisible({ - timeout: 30_000, - }); - await testPage.getByTestId("task-launch-branch-picker-option-main").click(); - await expect(testPage.getByTestId("task-launch-branch-picker-option-main")).toHaveCount(0); - await waitForLaunchErrorCleared(apiClient, seedData.workspaceId, task.id); - await expect .poll( async () => { @@ -348,13 +323,21 @@ test.describe("task launch failure recovery", () => { ["RUNNING", "WAITING_FOR_INPUT", "IDLE", "COMPLETED"].includes(item.state), ); }, - { timeout: 60_000, message: "waiting for the recovered refresh session to launch" }, + { timeout: 60_000, message: "waiting for the local-base session to launch" }, ) .toBe(true); - await assertNoDocumentHorizontalOverflow(testPage, "desktop required refresh recovery"); + await expect + .poll(() => taskLaunchError(apiClient, seedData.workspaceId, task.id), { + timeout: 30_000, + message: "waiting for the local-base launch error to remain clear", + }) + .toBeNull(); + await expect(testPage.getByTestId("task-launch-error-entry")).toHaveCount(0); + + await assertNoDocumentHorizontalOverflow(testPage, "desktop local-base recovery"); await testPage.screenshot({ - path: testInfo.outputPath("required-refresh-recovery-desktop.png"), + path: testInfo.outputPath("local-base-refresh-desktop.png"), fullPage: true, }); } finally { 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 238e66a495..f3f9f4145c 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 @@ -152,7 +152,7 @@ test.describe("mobile task launch failure recovery", () => { } }); - test("keeps a required refresh failure visible and retries on the phone", async ({ + test("starts from the local base when origin refresh fails on the phone", async ({ testPage, apiClient, seedData, @@ -163,7 +163,7 @@ test.describe("mobile task launch failure recovery", () => { const workflow = await apiClient.createWorkflow( seedData.workspaceId, - `Mobile required refresh recovery ${Date.now()}`, + "Mobile local base refresh", ); const waiting = await apiClient.createWorkflowStep(workflow.id, "Waiting", 0); const review = await apiClient.createWorkflowStep(workflow.id, "Review", 1); @@ -173,7 +173,7 @@ test.describe("mobile task launch failure recovery", () => { const task = await apiClient.createTask( seedData.workspaceId, - "Mobile required refresh failure recovery fixture", + "Mobile local base refresh fallback fixture", { description: "/e2e:simple-message", workflow_id: workflow.id, @@ -183,10 +183,6 @@ test.describe("mobile task launch failure recovery", () => { repositories: [{ repository_id: seedData.repositoryId, base_branch: "main" }], }, ); - const storedTask = await apiClient.getTask(task.id); - const taskRepository = storedTask.repositories?.[0]; - if (!taskRepository) - throw new Error("mobile refresh fixture did not create a task repository row"); pointSeedRepositoryAtFailingOrigin(seedData, backend.tmpDir); try { @@ -197,39 +193,15 @@ test.describe("mobile task launch failure recovery", () => { await expect .poll( async () => { - const { tasks } = await apiClient.listTasks(seedData.workspaceId); - const error = tasks.find((candidate) => candidate.id === task.id)?.status_summary - ?.active_error; - return Boolean(error?.stamp && error.category === "generic_launch_failure"); + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some((item) => + ["RUNNING", "WAITING_FOR_INPUT", "IDLE", "COMPLETED"].includes(item.state), + ); }, - { timeout: 60_000, message: "waiting for the mobile required refresh launch error" }, + { timeout: 60_000, message: "waiting for the mobile local-base session to launch" }, ) .toBe(true); - const launchError = (await apiClient.listTasks(seedData.workspaceId)).tasks.find( - (candidate) => candidate.id === task.id, - )?.status_summary?.active_error; - expect(launchError?.task_repository_id).toBe(taskRepository.id); - - const card = testPage.getByTestId("task-launch-error-entry"); - await expect(card).toHaveCount(1, { timeout: 30_000 }); - await expect(card).toContainText("The task could not start."); - await testPage.reload(); - await session.waitForLoad(); - await expect(testPage.getByTestId("task-launch-error-entry")).toHaveCount(1); - - restoreSeedRepositoryOrigin(seedData); - const pickBranch = testPage.getByTestId("task-launch-pick_base_branch-button"); - await pickBranch.tap(); - await expect(testPage.getByTestId("task-launch-branch-picker-mobile")).toBeVisible({ - timeout: 30_000, - }); - const branchOption = testPage.getByTestId("task-launch-branch-picker-option-main"); - await expect(branchOption).toBeVisible({ timeout: 30_000 }); - await expect(branchOption).toBeInViewport(); - await branchOption.tap(); - await expect(testPage.getByTestId("task-launch-branch-picker-mobile")).toHaveCount(0); - await expect .poll( async () => { @@ -239,24 +211,17 @@ test.describe("mobile task launch failure recovery", () => { null ); }, - { timeout: 60_000, message: "waiting for the mobile required refresh error to clear" }, + { + timeout: 30_000, + message: "waiting for the mobile local-base launch error to remain clear", + }, ) .toBeNull(); - await expect - .poll( - async () => { - const { sessions } = await apiClient.listTaskSessions(task.id); - return sessions.some((item) => - ["RUNNING", "WAITING_FOR_INPUT", "IDLE", "COMPLETED"].includes(item.state), - ); - }, - { timeout: 60_000, message: "waiting for the mobile refresh retry to launch" }, - ) - .toBe(true); + await expect(testPage.getByTestId("task-launch-error-entry")).toHaveCount(0); - await assertNoDocumentHorizontalOverflow(testPage, "mobile required refresh recovery"); + await assertNoDocumentHorizontalOverflow(testPage, "mobile local-base recovery"); await testPage.screenshot({ - path: testInfo.outputPath("required-refresh-recovery-mobile.png"), + path: testInfo.outputPath("local-base-refresh-mobile.png"), fullPage: true, }); } finally { 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 fa56d43793..7543b0a930 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 @@ -1,11 +1,15 @@ # ADR-2026-08-25-required-worktree-refresh-fails-closed: Required Worktree Refresh Fails Closed +> The local-worktree admission boundary is superseded by +> [ADR-2026-08-31-local-worktree-refresh-best-effort](2026-08-31-local-worktree-refresh-best-effort.md). +> Required remote materialization remains fail-closed when no usable local base exists. +> > Amended by > [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. -**Status:** accepted +**Status:** accepted (local-worktree boundary superseded by 2026-08-31-local-worktree-refresh-best-effort) **Date:** 2026-08-25 **Area:** backend, security, operations diff --git a/docs/decisions/2026-08-31-local-worktree-refresh-best-effort.md b/docs/decisions/2026-08-31-local-worktree-refresh-best-effort.md new file mode 100644 index 0000000000..1c2ad090da --- /dev/null +++ b/docs/decisions/2026-08-31-local-worktree-refresh-best-effort.md @@ -0,0 +1,68 @@ +# ADR-2026-08-31-local-worktree-refresh-best-effort: Local Worktree Refresh Is Best Effort + +**Status:** accepted +**Date:** 2026-08-31 +**Area:** backend, security, operations + +## Context + +Kandev v0.92.1 made pull-before-worktree a universal admission gate. This rule +blocked host worktrees when the selected branch existed only locally. + +A local Git worktree does not need remote publication. Users can create local +branches for parallel agents and keep all work off the remote. + +Remote-only executors have a different constraint. They must fetch or clone a +branch when no usable local ref is available. + +## Decision + +Host worktree refresh is best effort when the selected local base exists. +Kandev attempts the configured fetch, then uses the local ref if refresh cannot +improve it. + +Fetch errors, an absent remote branch, divergent refs, and uncertain ancestry +produce credential-safe warnings. These states do not block a host worktree +that has a valid local base. + +Caller cancellation still stops preparation. Kandev does not create a fallback +worktree after a cancellation request. + +Remote materialization remains strict when no usable local base exists. This +rule includes explicit remote-only refs and executors that must clone the +repository. + +Worktree preparation never pushes a branch. It does not reset, rebase, merge, +or remove local history. + +This decision supersedes the local-worktree admission boundary in +[ADR-2026-08-25-required-worktree-refresh-fails-closed](2026-08-25-required-worktree-refresh-fails-closed.md). + +## Consequences + +- Users can create worktrees from unpublished local branches. +- Origin outages do not block host work when a valid local base exists. +- Warnings show that the worktree can lack remote changes. +- Remote-only launch still stops when Kandev cannot materialize the branch. +- Local history remains authoritative when local and remote refs diverge. +- The `pull_before_worktree` setting controls a refresh attempt, not a universal + freshness guarantee. + +## Alternatives Considered + +### Keep universal fail-closed refresh + +Rejected. This rule makes remote publication a condition for local Git work. + +### Require users to disable pull-before-worktree + +Rejected. An intentional local branch is normal development, not an offline +repository configuration. + +### Push the local branch before worktree creation + +Rejected. Worktree preparation must not publish user history. + +### Use the local ref without a warning + +Rejected. The warning tells the user that remote changes can be absent. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 9714ee5332..2cd7ea258d 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 | 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 (local-worktree boundary superseded by 2026-08-31-local-worktree-refresh-best-effort) | 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 | @@ -222,4 +222,5 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-08-30-bound-untracked-dependency-enumeration | [Bound Untracked Dependency Enumeration](2026-08-30-bound-untracked-dependency-enumeration.md) | accepted | backend | 2026-08-30 | | 2026-08-30-e2e-browser-cache | [Cache host-runner E2E browser provisioning](2026-08-30-e2e-browser-cache.md) | accepted | infra, workflow | 2026-08-30 | | 2026-08-30-environment-owned-git-status | [Project Current Git Status From the Task Environment](2026-08-30-environment-owned-git-status.md) | proposed | backend, frontend, protocol | 2026-08-30 | +| 2026-08-31-local-worktree-refresh-best-effort | [Local Worktree Refresh Is Best Effort](2026-08-31-local-worktree-refresh-best-effort.md) | accepted | backend, security, operations | 2026-08-31 | | 2026-08-31-passthrough-running-publication | [Defer Passthrough Running Publication Until Guard Release](2026-08-31-passthrough-running-publication.md) | accepted | backend | 2026-08-31 | diff --git a/docs/plans/local-worktree-best-effort-refresh/plan.md b/docs/plans/local-worktree-best-effort-refresh/plan.md new file mode 100644 index 0000000000..5263fb2bc7 --- /dev/null +++ b/docs/plans/local-worktree-best-effort-refresh/plan.md @@ -0,0 +1,152 @@ +--- +created: 2026-08-31 +status: completed +requirements: + - REQ-WORKSPACES-WORKTREE-BASE-REFRESH-001 +system_design: + - ../../specs/workspaces/system-design/worktree-base-refresh.md +legacy_specs: [] +--- + +# Implementation Plan: Restore Best-Effort Local Worktree Refresh + +## Overview + +Restore local-first worktree creation without weakening remote-only +materialization. The implementation first corrects base selection and warning +behavior. Then it updates desktop, mobile, and public documentation evidence. + +## Confirmed root cause + +Commit `527cf1e9fa` changed `pullBaseBranch` from best-effort fetch to a required +refresh gate. The gate runs before Kandev uses an existing local base. + +A branch that exists only locally now fails at `git fetch origin `. +Kandev never reaches the valid `git worktree add` path. + +The focused reproduction uses one unpublished local branch. It fails with +`PullBeforeWorktree=true` and succeeds with the same setting disabled. + +The permanent regression test is +`TestCreateWorktree_PullEnabledUsesLocalOnlyBase` in +`apps/backend/internal/worktree/manager_pull_fallback_test.go`. + +## Scope + +### In scope + +- Use a verified local base after host refresh errors. +- Stop preparation when the caller cancels the operation. +- Preserve local history after missing remote refs or divergent refs. +- Keep warnings bounded and credential-safe. +- Keep remote materialization strict when no usable local base exists. +- Preserve the empty-remote bootstrap contract. +- Replace desktop and mobile E2E expectations that encode universal fail-closed refresh. +- Update public Git and executor documentation. + +### Out of scope + +- Pushing local branches during worktree preparation. +- Changing Docker, SSH, Sprites, or provider clone requirements. +- Changing the repository setting schema or default. +- Changing worktree settings UI composition or labels. +- Changing Git commands after agent startup. + +## Technical approach + +### Local base selection + +Update `internal/worktree.Manager` in +`apps/backend/internal/worktree/manager_git.go`. Verify the local base before +remote refresh. + +If the local base exists, convert fetch, pull, missing-ref, divergence, and +ancestry errors into bounded warnings. Return the local base without changing +Git refs. + +If no local base exists, keep the current error path. Remote materialization +must provide the requested base before worktree creation. + +Keep the successful-refresh selection rule. Use the remote ref when it contains +the local ref. Use the local ref when it contains the remote ref. + +### Warning propagation + +Use the existing `SyncProgressEvent` path for refresh warnings. Do not include +raw Git output in user-visible warnings or generic task errors. + +Keep structured logs limited to repository identity, failure class, branch, +and selected fallback ref. + +### User-facing evidence + +Replace the required-refresh failure cases in the desktop and mobile launch +recovery specs. The new cases keep a local base, make origin unavailable, and +prove that the task starts without an active launch error. + +The change does not alter layout, navigation, touch behavior, or scroll +ownership. Desktop and mobile use their existing task-session surfaces. + +### Public documentation + +Update the Git operations reference and executor explanation. State that host +refresh is best effort when a local base exists. Keep strict remote +materialization limits explicit. + +## Tests + +| Acceptance criteria | Evidence | +| --- | --- | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.1` | Existing disabled-policy worktree tests remain green. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.2` | A focused manager test proves that the fetch attempt occurs. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3` | `TestCreateWorktree_PullEnabledUsesLocalOnlyBase` proves fallback and bounded warning output. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.4` | Existing containing-ref tests remain green. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.5` | A manager test proves that divergence preserves the local ref with a warning. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.6` | A manager test proves that missing local and remote refs stop preparation. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.7` | Existing multi-repository preparation tests gain a local-fallback case if manager coverage cannot prove propagation. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.8` | Existing repository-specific launch-error tests remain green. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.9` | Existing empty-remote worktree tests remain green. | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.10` | A manager test proves that caller cancellation does not create a fallback worktree. | + +## E2E tests + +- Desktop: `apps/web/e2e/tests/task/launch-failure-recovery.spec.ts` proves that + an origin outage does not block a task with a valid local base. +- Mobile: `apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts` + proves the same task outcome in the `mobile-chrome` project. + +## Work orders + +- [x] [Task 01: Restore Local Base Fallback](task-01-restore-local-base-fallback.md) +- [x] [Task 02: Prove Desktop and Mobile Launch](task-02-prove-local-fallback-launch.md) +- [x] [Task 03: Update Public Refresh Documentation](task-03-update-refresh-documentation.md) + +## Dependency order + +```text +Task 01 -> Task 02 -> Task 03 +``` + +The work orders are sequential. No work order authorizes subagents. + +## Verification results + +- `go test ./internal/worktree ./internal/agent/runtime/lifecycle ./internal/orchestrator/executor -run 'Test(CreateWorktree|PullBaseBranch|PreferRefreshedRemoteRef|ResolveBaseRefWithFallback|Prepare.*Worktree|.*Refresh)' -count=1` passed (69 tests). +- `make -C apps/backend test` passed with the task-session configuration overrides removed. +- Changed-file `golangci-lint` passed with no issues. +- Web typecheck and focused E2E checks passed for Chromium and `mobile-chrome`. +- Public documentation validation and specification lint passed. + +## Risks + +- A broad fallback can hide a missing remote-only branch. The code must verify + the local base before fallback. +- A broad fallback can ignore user cancellation. Cancellation must remain fatal. +- A warning can expose Git credential output. The warning must use bounded + failure classes. +- Provider-managed refresh can fail before local selection. The implementation + must not weaken remote-only materialization. +- Existing E2E cases encode the superseded invariant. Replacement cases must + keep unrelated launch-error recovery coverage intact. +- Empty remotes use a separate marked baseline. The local fallback must not + bypass that marker contract. diff --git a/docs/plans/local-worktree-best-effort-refresh/task-01-restore-local-base-fallback.md b/docs/plans/local-worktree-best-effort-refresh/task-01-restore-local-base-fallback.md new file mode 100644 index 0000000000..72b94c1ce0 --- /dev/null +++ b/docs/plans/local-worktree-best-effort-refresh/task-01-restore-local-base-fallback.md @@ -0,0 +1,102 @@ +--- +id: "01-restore-local-base-fallback" +title: "Restore Local Base Fallback" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-WORKSPACES-WORKTREE-BASE-REFRESH-001 +acceptance_criteria: + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.1 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.2 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.4 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.5 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.6 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.7 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.8 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.9 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.10 +system_design: + - ../../specs/workspaces/system-design/worktree-base-refresh.md +--- + +# Task 01: Restore Local Base Fallback + +## Summary + +Restore best-effort refresh for host worktrees with a valid local base. Keep +strict remote materialization when Kandev has no usable local ref. + +## In scope + +- Add the failing `TestCreateWorktree_PullEnabledUsesLocalOnlyBase` regression. +- Add fetch-error, missing-ref, divergence, and uncertain-ancestry fallback + coverage. +- Preserve successful remote-ahead and local-ahead selection. +- Preserve strict failure when both local and remote bases are unavailable. +- Preserve caller cancellation as a terminal preparation result. +- Emit bounded warning progress without raw Git credential output. +- Keep empty-remote and multi-repository behavior compatible. + +## Out of scope + +- E2E changes. +- Public documentation changes. +- Repository setting or API changes. +- Automatic remote publication. + +## Acceptance + +- A valid local base creates a worktree after a non-cancellation refresh error. +- A missing local base still needs successful remote materialization. +- Caller cancellation never creates a fallback worktree. +- Local fallback never changes refs and never exposes raw Git output. + +## Verification + +```bash +cd apps/backend && go test ./internal/worktree ./internal/agent/runtime/lifecycle ./internal/orchestrator/executor -run 'Test(CreateWorktree|PullBaseBranch|PreferRefreshedRemoteRef|ResolveBaseRefWithFallback|Prepare.*Worktree|.*Refresh)' -count=1 +``` + +## Files likely touched + +- `apps/backend/internal/worktree/manager_git.go` +- `apps/backend/internal/worktree/manager_lifecycle.go` +- `apps/backend/internal/worktree/manager_pull_fallback_test.go` +- `apps/backend/internal/worktree/manager_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_multi_test.go` + +## Dependencies + +None. + +## Risks + +- The fallback can incorrectly admit remote-only branches if local verification + is omitted. +- The fallback can ignore cancellation if it classifies every command error as + recoverable. +- Warning details can leak credentials if they reuse raw Git output. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-WORKSPACES-WORKTREE-BASE-REFRESH-001` +- Worktree Base Refresh System Design sections for local fallback and required materialization. +- ADR `2026-08-31-local-worktree-refresh-best-effort`. +- Pre-`527cf1e9fa` best-effort manager behavior. + +## Results + +- Host worktree refresh now verifies the local base first and treats fetch, + pull, missing-remote-ref, divergence, and uncertain-ancestry failures as + credential-safe warnings when that local base remains usable. +- Remote-only refs, missing local bases, and cancellation remain strict. +- Worktree and lifecycle tests cover local preservation, warning projection, + cancellation, provider-managed refresh, recreate, and multi-repository + continuation. diff --git a/docs/plans/local-worktree-best-effort-refresh/task-02-prove-local-fallback-launch.md b/docs/plans/local-worktree-best-effort-refresh/task-02-prove-local-fallback-launch.md new file mode 100644 index 0000000000..5fcf0b7154 --- /dev/null +++ b/docs/plans/local-worktree-best-effort-refresh/task-02-prove-local-fallback-launch.md @@ -0,0 +1,86 @@ +--- +id: "02-prove-local-fallback-launch" +title: "Prove Desktop and Mobile Launch" +status: completed +wave: 2 +depends_on: + - "01-restore-local-base-fallback" +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.7 +system_design: + - ../../specs/workspaces/system-design/worktree-base-refresh.md +--- + +# Task 02: Prove Desktop and Mobile Launch + +## Summary + +Replace E2E expectations that treat every refresh error as fatal. Prove that a +task starts from a valid local base on desktop and mobile when origin is +unavailable. + +## In scope + +- Convert the desktop required-refresh case into a local-fallback launch case. +- Convert the mobile required-refresh case into the same local-fallback outcome. +- Assert that the session starts and no active launch error remains. +- Restore all modified seed repository state after each case. +- Preserve unrelated missing-base and launch-recovery scenarios. + +## Out of scope + +- UI layout or component changes. +- New mobile navigation or touch interactions. +- Backend manager implementation. + +## Acceptance + +- The desktop task reaches a running, idle, or completed session from the local base. +- The mobile task reaches the same state without an active launch error. +- Both specs restore origin and repository settings after completion. + +## Verification + +```bash +cd apps/web && pnpm e2e:run tests/task/launch-failure-recovery.spec.ts -- --grep 'local base' +cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-launch-failure-recovery.spec.ts -- --grep 'local base' +``` + +## Files likely touched + +- `apps/web/e2e/tests/task/launch-failure-recovery.spec.ts` +- `apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts` +- `apps/web/e2e/helpers/api-client.ts` only if existing seed methods are insufficient. + +## Dependencies + +- Task 01 must be complete. + +## Risks + +- The tests can leak a broken origin into later worker-scoped cases. +- A backend-only assertion can miss the user-visible session outcome. + +## Parallelism + +`sequential` + +## Inputs + +- `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3` +- `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.7` +- Existing desktop and mobile launch-failure recovery fixtures. + +## Results + +- The desktop launch-recovery case now proves a valid local base starts a + session when origin refresh fails and leaves no active launch error. +- The mobile launch-recovery case proves the same outcome in the + `mobile-chrome` project while retaining the existing native mobile recovery + coverage for missing bases. +- Both focused E2E commands passed and restore the seed origin and checkout + state in cleanup. diff --git a/docs/plans/local-worktree-best-effort-refresh/task-03-update-refresh-documentation.md b/docs/plans/local-worktree-best-effort-refresh/task-03-update-refresh-documentation.md new file mode 100644 index 0000000000..f00a53f783 --- /dev/null +++ b/docs/plans/local-worktree-best-effort-refresh/task-03-update-refresh-documentation.md @@ -0,0 +1,83 @@ +--- +id: "03-update-refresh-documentation" +title: "Update Public Refresh Documentation" +status: completed +wave: 3 +depends_on: + - "02-prove-local-fallback-launch" +plan: "plan.md" +requirements: + - REQ-WORKSPACES-WORKTREE-BASE-REFRESH-001 +acceptance_criteria: + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.2 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3 + - AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.6 +system_design: + - ../../specs/workspaces/system-design/worktree-base-refresh.md +--- + +# Task 03: Update Public Refresh Documentation + +## Summary + +Update public Git and executor documentation for local-first worktree refresh. +Keep remote-only materialization limits clear. + +## In scope + +- Describe pull-before-worktree as best effort when a local base exists. +- Describe credential-safe warnings for local fallback. +- Describe strict failure when no local or remote base is available. +- Remove guidance that users must disable refresh for normal local branches. +- Keep empty-remote and remote executor guidance accurate. + +## Out of scope + +- New public pages or navigation entries. +- UI copy or localization changes. +- Release notes. + +## Acceptance + +- Public docs do not claim that all refresh errors stop host worktrees. +- Public docs state when remote materialization remains required. +- Public-doc validators and link checks pass. + +## Verification + +```bash +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +git diff --check -- docs/public +``` + +## Files likely touched + +- `docs/public/git-operations.md` +- `docs/public/executors.md` + +## Dependencies + +- Task 02 must be complete. + +## Risks + +- Broad wording can imply that Docker, SSH, or Sprites can use host-only refs. + +## Parallelism + +`sequential` + +## Inputs + +- Worktree Base Refresh requirements and system design. +- ADR `2026-08-31-local-worktree-refresh-best-effort`. +- Existing Git operations and executor documentation. + +## Results + +- Git operations now describe best-effort host refresh with credential-safe + local fallback warnings. +- Executor guidance distinguishes host local bases from strict remote-only + refresh and checkout requirements. +- Public documentation tests, link validation, and whitespace checks passed. diff --git a/docs/public/executors.md b/docs/public/executors.md index 8f1ed4159b..23cf0d3294 100644 --- a/docs/public/executors.md +++ b/docs/public/executors.md @@ -200,15 +200,15 @@ 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, the base refresh is required for a new or recreated worktree. An authentication, network, timeout, missing-ref, divergent-ref, or uncertain-ancestry failure stops the launch; Kandev does not use a stale local fallback. 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. 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). Typical failures: - dirty or conflicting source repository state; -- base branch missing locally or remotely; -- required refresh credentials, network access, or remote ancestry cannot be verified; +- a base branch is missing locally and remote materialization cannot provide it; +- a remote-only executor cannot verify its refresh or checkout; - worktree path already registered in Git metadata; - setup dependencies absent on the host; - repository cleanup failure leaving a stale worktree. diff --git a/docs/public/git-operations.md b/docs/public/git-operations.md index 296853e23a..d7c6184da9 100644 --- a/docs/public/git-operations.md +++ b/docs/public/git-operations.md @@ -79,12 +79,12 @@ For a new task branch, the repository default template is: 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 must 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. An authentication, network, timeout, missing-ref, divergent-ref, or uncertain-ancestry failure stops task preparation and records a repository-specific launch error. Kandev does not create the worktree from a stale local or remote-tracking fallback. +`{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. If the repository is intentionally offline, open its workspace repository settings and disable -**Always pull before creating a new worktree**. This preserves the local workflow, but it also -opts out of the freshness guarantee and allows the task to use the local base state. Re-enable the -setting before relying on remote changes for later task launches. +**Always pull before creating a new worktree**. This skips the refresh attempt for host worktrees, +but it is not required for a normal local-only base. Keep the setting enabled when remote freshness +is important for later task launches. ### Named branch policies diff --git a/docs/specs/workspaces/requirements/worktree-base-refresh.md b/docs/specs/workspaces/requirements/worktree-base-refresh.md index 667a218967..291babdb40 100644 --- a/docs/specs/workspaces/requirements/worktree-base-refresh.md +++ b/docs/specs/workspaces/requirements/worktree-base-refresh.md @@ -10,73 +10,75 @@ owners: ## Overview -The workspace system owns repository state and the base ref used to create a -task worktree. When a workspace enables pull-before-worktree behavior, users -expect each new or recreated worktree to start from verified remote state. -Kandev must not start an agent on an unverified local copy after a required -refresh fails. +The workspace system owns the base ref for a task worktree. A host worktree can +use a local base without remote publication. Remote refresh improves freshness, +but it must not prevent intentional local development. + +Remote materialization has a different contract. If Kandev has no usable local +base, it must obtain the selected branch before it creates the worktree. ## Terminology -- **Required refresh:** The remote synchronization gate enabled by a - repository's pull-before-worktree setting. -- **Fetched remote base:** The remote-tracking ref produced by the successful - fetch for the requested base branch. -- **Comparable refs:** Two refs where either ref contains every commit in the - other ref. +- **Local base:** A valid local ref that Kandev can use to create the worktree. +- **Best-effort refresh:** A remote refresh attempt that can fall back to a + local base. +- **Required materialization:** A remote fetch that is necessary because no + usable local base exists. +- **Fetched remote base:** The remote-tracking ref from a successful fetch. ## Requirements -### REQ-WORKSPACES-WORKTREE-BASE-REFRESH-001: Verified Worktree Base +### REQ-WORKSPACES-WORKTREE-BASE-REFRESH-001: Local-First Worktree Base -**Intent:** Prevent an agent from starting on stale repository state when the -workspace requires a remote refresh before worktree creation. +**Intent:** Preserve local development while Kandev attempts to refresh remote +state before worktree creation. -**User story:** As a workspace user, I want a required repository refresh to -stop task launch when it fails, so that an agent cannot edit and push from an -old copy without my knowledge. +**User story:** As a workspace user, I want Kandev to create worktrees from my +local branches without publishing them first. #### Acceptance criteria - **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.1:** When pull-before-worktree is - disabled, Kandev shall allow a new or recreated worktree to use available - local refs without requiring remote access. + disabled, Kandev shall use an available local base without remote access. - **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.2:** When pull-before-worktree is - enabled, Kandev shall complete the remote refresh through the repository's - configured transport and credential route before it starts an agent in a new - or recreated worktree. -- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3:** When a required fetch fails - because of authentication, network access, timeout, cancellation, or another - Git error, Kandev shall stop worktree preparation and shall not use a stale - local fallback ref. An authenticated zero-ref advertisement is the distinct - empty-remote state defined by `REQ-WORKSPACES-EMPTY-REMOTE-REPOSITORIES-001`. -- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.4:** When the required fetch - succeeds, Kandev shall choose a start ref that contains the fetched remote - base. It can preserve local-only commits only when the local base also - contains the fetched remote base. -- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.5:** When the local base and fetched - remote base have diverged, or Kandev cannot prove their ancestry, Kandev - shall stop worktree preparation without resetting, deleting, or hiding either - ref. -- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.6:** When required refresh fails - for any repository in a multi-repository task, Kandev shall not start the - task agent and shall identify the affected repository in a credential-safe - launch error. -- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.7:** When required refresh fails - during initial launch or a resume that creates or recreates a worktree, - Kandev shall expose the failure through the existing durable task launch-error - projection. -- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.8:** When authenticated refresh - proves that a remote advertises zero refs, Kandev shall use only the marked - local baseline defined by the empty-remote repository contract. + enabled and a local base exists, Kandev shall attempt a remote refresh before + it creates or recreates the worktree. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3:** When refresh has an + authentication, network, timeout, missing-branch, or non-cancellation Git + error, Kandev shall use the local base and show a credential-safe warning. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.4:** When the refresh succeeds, + Kandev shall preserve local-only commits and include compatible remote + commits when one ref contains the other. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.5:** When local and remote refs + diverge, Kandev shall preserve the selected local base and show a warning. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.6:** When no local base exists, + Kandev shall stop preparation if required materialization cannot provide the + selected branch. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.7:** When one repository in a + multi-repository task has a usable local fallback, its refresh error shall not + stop the task agent. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.8:** When required materialization + fails for any repository, Kandev shall stop the task agent and identify that + repository in a credential-safe error. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.9:** When an authenticated remote + advertises zero refs, Kandev shall use the marked local baseline from + `REQ-WORKSPACES-EMPTY-REMOTE-REPOSITORIES-001`. +- **AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.10:** When the caller cancels + preparation, Kandev shall stop without creating a fallback worktree. + +## Compatibility + +This requirement replaces the universal fail-closed refresh behavior from +Kandev v0.92.1. Remote-only executors and branches keep strict materialization. ## Out of scope -- Making setup-script failures fatal. -- Refreshing a worktree that Kandev reuses without creating or recreating it. +- Making setup-script errors fatal. +- Refreshing a valid worktree that Kandev reuses. - Adding a background repository-refresh service. -- Automatically resetting, rebasing, merging, or deleting local refs. +- Resetting, rebasing, merging, or removing local refs. - Changing agent Git transport after the agent starts. +- Pushing a local branch during worktree preparation. ## System design diff --git a/docs/specs/workspaces/system-design/worktree-base-refresh.md b/docs/specs/workspaces/system-design/worktree-base-refresh.md index 4c68ca231f..fcf82c2ec7 100644 --- a/docs/specs/workspaces/system-design/worktree-base-refresh.md +++ b/docs/specs/workspaces/system-design/worktree-base-refresh.md @@ -9,198 +9,156 @@ requirements: ## Purpose and boundaries -The workspace system owns the repository checkout, remote state, and base ref -used to create a worktree. This design makes `repositories.pull_before_worktree` -an admission gate for new and recreated worktrees. +The workspace system owns the repository checkout and the worktree base ref. +This design keeps host worktrees local-first while remote materialization stays +strict. -The task system continues to own session launch state and error presentation. -It consumes preparation errors through the existing launch-failure contract in -[Task Launch Failure Recovery](../../tasks/system-design/task-launch-failure-recovery.md). -The integration system continues to own provider credentials. The executor -continues to own credentials and SSH configuration inherited from its runtime. +The task system owns session launch state and error presentation. The +integration system owns provider credentials. Executors own their Git transport +and remote workspace preparation. -This design does not refresh a worktree that Kandev can reuse without -recreation. It also does not change Git commands that an agent runs after -launch. +This design does not refresh a valid worktree that Kandev reuses. It does not +change Git commands that an agent runs after launch. ## Requirement mapping -| Requirement | Design section | +| Acceptance criterion | Design section | | --- | --- | | `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.1` | [Refresh policy](#refresh-policy) | -| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.2` | [Refresh routing](#refresh-routing) | -| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3` | [Failure and recovery](#failure-and-recovery) | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.2` | [Refresh policy](#refresh-policy) | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.3` | [Local fallback](#local-fallback) | | `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.4` | [Base-ref selection](#base-ref-selection) | | `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.5` | [Base-ref selection](#base-ref-selection) | -| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.6` | [Multi-repository launch](#multi-repository-launch) | -| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.7` | [Launch-error projection](#launch-error-projection) | -| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.8` | [Empty remote exception](#empty-remote-exception) | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.6` | [Required materialization](#required-materialization) | +| `AC-WORKSPACES-WORKTREE-BASE-REFRESH-001.7` | [Multi-repository launch](#multi-repository-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) | ## Components and responsibilities -- `internal/orchestrator/executor` resolves every task repository and selects - the refresh route that matches its provider and task Git credential policy. - It passes `PullBeforeWorktree`, `RemoteSyncHandled`, and a provider-authenticated - refresh callback to the lifecycle layer. The callback is not invoked until a - worktree must be materialized or recreated. -- `internal/repoclone.Cloner` performs a strict refresh for a Kandev-managed - provider checkout when provider credentials belong to the backend refresh - boundary. `RefreshWorkspaceRepositoryWithCredentialRequest` binds the fetch - to the exact workspace, task, session, and repository scope. -- `internal/worktree.Manager` performs strict non-interactive fetch and base-ref - selection when the checkout uses the host or executor Git route. It creates - no worktree until the required sync and ancestry checks succeed. -- `internal/agent/runtime/lifecycle` propagates repository preparation errors - without starting the agent runtime. -- The task launch-failure projection stores and renders the resulting bounded, - credential-safe error. This design does not add a parallel workspace error - store. +- `internal/orchestrator/executor` resolves repository paths, refresh routes, + credentials, and executor capabilities. +- `internal/worktree.Manager` verifies local refs, attempts host refresh, and + selects the worktree base. +- `internal/repoclone.Cloner` materializes provider-managed repositories when + no user-owned local checkout supplies the branch. +- `internal/agent/runtime/lifecycle` stops launch only when no usable base + remains. +- The task projection shows bounded warnings and required-materialization + errors. ## Data and contracts -`repositories.pull_before_worktree` remains the persisted user policy. No -schema change is required. +`repositories.pull_before_worktree` remains a persisted user policy. No schema +change is necessary. -The existing worktree preparation fields have these contracts: +The preparation fields have these contracts: | Field | Contract | | --- | --- | -| `PullBeforeWorktree` | A required remote refresh remains to be completed before base-ref selection. | -| `RemoteSyncHandled` | A strict provider-authenticated refresh completed for the exact checkout. The worktree manager must not run a second unauthenticated fetch. | -| `RefreshRepository` | An optional exact-scope refresh callback. Lifecycle invokes it only when creating or recreating a worktree; valid reuse bypasses it. | +| `PullBeforeWorktree` | Kandev attempts remote refresh before local worktree creation. | +| `RemoteSyncHandled` | The configured remote route completed a refresh for this checkout. | +| `RefreshRepository` | An optional provider refresh for worktree materialization. | -`RemoteSyncHandled` is valid only after a successful strict refresh. A -best-effort fetch or a failed refresh cannot set it. +`PullBeforeWorktree` is not a universal admission gate. Local-ref availability +determines whether a failed refresh is recoverable. ## Refresh policy -When `PullBeforeWorktree` is false, base-ref resolution keeps its offline local -behavior. This path does not claim that the checkout contains current remote -state. +If `PullBeforeWorktree` is false, Kandev uses the selected local base without a +remote request. -When `PullBeforeWorktree` is true, worktree materialization requires one -successful refresh. A caller cannot convert a refresh error into a warning and -local ref. The rule applies to initial launch and to resume whenever Kandev -creates or recreates a worktree. A valid reusable worktree is returned before -the callback is invoked. +If `PullBeforeWorktree` is true, Kandev attempts the configured refresh before +new or recreated host worktrees. A valid reusable worktree bypasses this +attempt. -## Refresh routing +The refresh route still follows the task Git credential policy. A host checkout +keeps its configured origin and transport. A provider-managed checkout keeps +its exact credential scope. -The executor resolves one of two refresh routes before lifecycle preparation: +## Local fallback -1. A Kandev-managed provider route supplies a callback that calls the strict - `repoclone` refresh with the exact credential request. The lifecycle layer - invokes it only when it must materialize or recreate a worktree. On success, - worktree creation clears `PullBeforeWorktree` and sets `RemoteSyncHandled`. -2. A host or executor route leaves `PullBeforeWorktree` set. The worktree - manager uses the reconciled `origin` URL and the non-interactive host Git - environment. This includes an executor-inherited GitHub checkout whose - origin uses the protocol selected by the host `gh` configuration. +Before refresh, the worktree manager verifies the selected local ref. If that +ref exists, refresh is best effort. -When a managed checkout carries a pull request number, the strict refresh also -fetches `refs/pull//head` into a local `origin/pr/` remote-tracking ref. -Worktree creation uses that ref for the checkout branch, including fork pull -requests, without a second unauthenticated fetch. +Authentication, network, timeout, Git, and missing-remote-ref errors return the +selected local ref. The manager emits a bounded warning and does not include +raw credential output. -Provider selection must follow the existing task Git credential policy. A -managed GitHub workspace uses the backend-managed credential route. An -executor-inherited GitHub workspace must not replace the reconciled SSH origin -with a managed HTTPS origin. - -The plugin-provider strict-refresh behavior remains the model for exact -credential scope. GitLab and Azure DevOps keep their existing provider-specific -credential resolution. +The fallback does not change local or remote refs. It does not push the local +branch. ## Base-ref selection -After a successful fetch, the worktree manager compares the local base `L` -with the fetched remote base `R`: +After a successful fetch, the worktree manager compares local base `L` and +remote base `R`: -| Relationship | Start ref | Reason | +| Relationship | Start ref | Result | | --- | --- | --- | -| `R` contains `L` | `R` | Includes all local commits and current remote commits. | -| `L` contains `R` | `L` | Preserves local-only commits and includes current remote commits. | -| Neither contains the other | Error | The refs diverged and either choice would omit commits. | -| Ancestry cannot be proven | Error | Kandev cannot prove that the start ref is safe. | +| `R` contains `L` | `R` | The worktree includes current remote commits. | +| `L` contains `R` | `L` | The worktree preserves local-only commits. | +| Refs diverge | `L` | The worktree preserves the selected local history and shows a warning. | +| Ancestry is unknown | `L` | The worktree preserves the selected local history and shows a warning. | -A successful fast-forward pull normally produces the first or second state. -If pull fails after fetch, the same table selects the safe ref. The manager does -not reset, rebase, merge, or delete a branch. +The manager never resets, rebases, merges, removes, or hides either ref during +selection. -`pullBaseBranch`, `resolveLocalBaseRef`, and -`pullCurrentBranchOrFallback` return errors to their callers. The errors retain -Git failure classification but exclude tokens, credential helper output, and -secret-bearing URLs. +## Required materialization -## Empty remote exception +Remote access is required when Kandev cannot verify a usable local base. This +case includes an explicit remote-only ref and an executor that must clone the +repository. -An authenticated remote advertisement with zero refs is not a stale local -fallback. It is the typed empty-remote state defined by +If materialization fails, lifecycle stops launch. The task error identifies the +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. + +## Empty remote + +An authenticated remote with zero refs uses the marked local baseline from the [Empty Remote Repository System Design](empty-remote-repositories.md). -This exception creates only the marked local baseline. It does not permit a -launch-time remote write or relax any authentication, network, timeout, -cancellation, missing-branch, or ancestry error. +This path creates no remote ref during launch. Publication remains an explicit +user action. ## Multi-repository launch -The executor resolves and prepares repository specs before agent startup. If a -required refresh fails for one spec, lifecycle preparation returns an error and -does not start the runtime. The error carries stable repository identity and a -safe display name so same-provider sibling repositories remain distinguishable. +Each repository resolves its base independently before agent startup. A local +fallback counts as a prepared repository. -Preparation can leave an already refreshed checkout on disk. This state is -safe and retryable. A later launch performs the required checks again. Kandev -does not report a partial task launch because no agent process started. - -## Launch-error projection - -The executor returns required-refresh errors through its existing launch -failure path. The task system stores the active error in the current typed -summary and renders it on desktop and mobile after reload. The generic launch -failure category is sufficient unless implementation evidence shows that a -distinct category enables a safe recovery action. - -The bounded detail names the repository and failure class. It does not include -credentials, tokens, raw authenticated URLs, or unrestricted Git output. Retry -uses the normal task launch action after the user corrects network, SSH, or -credential configuration. +If one repository needs remote materialization and has no usable base, the +whole task stops before agent startup. The error names that repository. ## Failure and recovery -- Fetch authentication, network, timeout, cancellation, and Git command errors - stop required preparation. -- A missing fetched remote base on a non-empty remote stops preparation. It - cannot fall back to a local ref when refresh is required. -- A failed ancestry probe stops preparation. -- A divergent base stops preparation and preserves both refs. -- Disabling pull-before-worktree is the explicit offline opt-out. Kandev does - not disable it automatically after a failure. -- Retrying task launch reruns remote refresh and base-ref selection. +- A refresh error with a valid local base produces a warning and continues. +- A missing local and remote base produces a launch error. +- 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. ## Security -Managed refresh uses the provider credential boundary and exact task, session, -and repository scope. Executor refresh uses only credentials available to the -selected host or executor route. Neither route copies a provider token into an -error, progress event, or log field. +Git stays non-interactive. Host refresh uses the host credential route. +Provider refresh uses exact task and repository scope. -Git remains non-interactive. `GIT_TERMINAL_PROMPT=0`, batch-mode SSH, bounded -command contexts, and Git subprocess admission prevent a hidden prompt from -blocking task launch. +Warnings contain a failure class, branch name, and repository identity. They do +not contain tokens, credential-helper output, or secret URLs. ## Observability -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, not a fallback-ref success message. +Sync progress reports the refresh attempt and its result. Local fallback +reports completion with a warning, not a launch error. -Structured logs record the repository ID, task ID, session ID, provider, -configured transport, refresh route, and failure class. Logs do not record -credential material or secret-bearing remote URLs. +Structured logs record repository identity, refresh route, failure class, and +selected fallback ref. Logs exclude credential material and raw remote URLs. ## Related decisions +- [Local Worktree Refresh Is Best Effort](../../../decisions/2026-08-31-local-worktree-refresh-best-effort.md) - [Required Worktree Refresh Fails Closed](../../../decisions/2026-08-25-required-worktree-refresh-fails-closed.md) - [Separate GitHub Automation From Task Git Credential Policy](../../../decisions/2026-07-27-task-git-credential-policy.md) - [Provider-Neutral Git Credential Broker](../../../decisions/2026-07-31-provider-neutral-git-credential-broker.md)