feat: improve git checkout with error classification and warning propagation (#386)

This commit is contained in:
Carlos Florêncio
2026-03-06 23:54:31 +00:00
committed by GitHub
parent 1724f10340
commit 21cb909ef4
26 changed files with 1516 additions and 131 deletions
+1 -1
View File
@@ -28,7 +28,7 @@ repos:
- id: go-lint
name: Go Lint (backend)
entry: bash -c 'cd apps/backend && golangci-lint run'
entry: bash -c 'unset GIT_DIR GIT_WORK_TREE; cd apps/backend && golangci-lint run --new-from-rev=HEAD'
language: system
files: \.go$
pass_filenames: false
+2
View File
@@ -3,3 +3,5 @@ node_modules
dist
build
coverage
test-results
playwright-report
@@ -44,12 +44,14 @@ type EnvPrepareRequest struct {
// PrepareStep represents a single step in the preparation process.
type PrepareStep struct {
Name string `json:"name"`
Status PrepareStepStatus `json:"status"`
Output string `json:"output,omitempty"`
Error string `json:"error,omitempty"`
StartedAt *time.Time `json:"started_at,omitempty"`
EndedAt *time.Time `json:"ended_at,omitempty"`
Name string `json:"name"`
Status PrepareStepStatus `json:"status"`
Output string `json:"output,omitempty"`
Error string `json:"error,omitempty"`
Warning string `json:"warning,omitempty"`
WarningDetail string `json:"warning_detail,omitempty"`
StartedAt *time.Time `json:"started_at,omitempty"`
EndedAt *time.Time `json:"ended_at,omitempty"`
}
// EnvPrepareResult contains the result of environment preparation.
@@ -11,10 +11,11 @@ import (
"go.uber.org/zap"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/worktree"
)
// LocalPreparer prepares a local (non-worktree) execution environment.
// Steps: validate workspace → run setup script (if any).
// Steps: validate workspace → checkout PR branch (if set) → run setup script (if any).
type LocalPreparer struct {
logger *logger.Logger
}
@@ -39,13 +40,18 @@ func (p *LocalPreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, onP
resolvedScript := resolvePreparerSetupScript(req, workspacePath)
totalSteps := 1 // validate workspace
if req.CheckoutBranch != "" {
totalSteps++
}
if resolvedScript != "" {
totalSteps++
}
stepIdx := 0
// Step 1: Validate workspace path
step := beginStep("Validate workspace")
reportProgress(onProgress, step, 0, totalSteps)
reportProgress(onProgress, step, stepIdx, totalSteps)
if req.WorkspacePath == "" && req.RepositoryPath == "" {
completeStepError(&step, "no workspace or repository path provided")
steps = append(steps, step)
@@ -53,25 +59,45 @@ func (p *LocalPreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, onP
}
completeStepSuccess(&step)
steps = append(steps, step)
reportProgress(onProgress, step, 0, totalSteps)
reportProgress(onProgress, step, stepIdx, totalSteps)
stepIdx++
// Step 2: Run setup script (if provided)
// Step 2: Checkout PR branch (if specified)
if req.CheckoutBranch != "" {
step = beginStep("Checkout branch")
reportProgress(onProgress, step, stepIdx, totalSteps)
output, err := checkoutBranch(ctx, workspacePath, req.CheckoutBranch)
if err != nil {
errMsg := fmt.Sprintf("failed to checkout branch %q: %s", req.CheckoutBranch, output)
completeStepError(&step, errMsg)
steps = append(steps, step)
reportProgress(onProgress, step, stepIdx, totalSteps)
return &EnvPrepareResult{Success: false, Steps: steps, ErrorMessage: errMsg, Duration: time.Since(start)}, fmt.Errorf("checkout branch: %w", err)
}
step.Output = output
completeStepSuccess(&step)
steps = append(steps, step)
reportProgress(onProgress, step, stepIdx, totalSteps)
stepIdx++
}
// Step 3: Run setup script (if provided)
if resolvedScript != "" {
step = beginStep("Run setup script")
reportProgress(onProgress, step, 1, totalSteps)
reportProgress(onProgress, step, stepIdx, totalSteps)
output, err := runSetupScript(ctx, resolvedScript, workspacePath, req.Env)
if err != nil {
completeStepError(&step, err.Error())
step.Output = output
steps = append(steps, step)
reportProgress(onProgress, step, 1, totalSteps)
reportProgress(onProgress, step, stepIdx, totalSteps)
p.logger.Warn("setup script failed", zap.String("task_id", req.TaskID), zap.Error(err))
// Setup script failure is non-fatal — log and continue
} else {
step.Output = output
completeStepSuccess(&step)
steps = append(steps, step)
reportProgress(onProgress, step, 1, totalSteps)
reportProgress(onProgress, step, stepIdx, totalSteps)
}
}
@@ -83,6 +109,28 @@ func (p *LocalPreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, onP
}, nil
}
// checkoutBranch ensures a branch is checked out in the given working directory.
// It first tries to fetch the latest from origin, then checks out the branch.
// If fetch fails (no remote, offline), it falls back to the local branch.
func checkoutBranch(ctx context.Context, workDir, branch string) (string, error) {
// Try to fetch the latest from origin (best-effort).
fetchCmd := exec.CommandContext(ctx, "git", "fetch", "origin", branch)
fetchCmd.Dir = workDir
fetchCmd.Run() //nolint:errcheck // fetch failure is non-fatal, we fall back to local
// Checkout the branch. If the local branch doesn't exist but the remote
// tracking branch does (from the fetch above), git will create a local
// branch tracking the remote automatically.
cmd := exec.CommandContext(ctx, "git", "checkout", branch)
cmd.Dir = workDir
out, err := cmd.CombinedOutput()
outStr := strings.TrimSpace(string(out))
if err != nil {
return outStr, worktree.ClassifyGitError(outStr, err)
}
return outStr, nil
}
// runSetupScript executes a setup script in the given working directory.
func runSetupScript(ctx context.Context, script, workDir string, env map[string]string) (string, error) {
cmd := exec.CommandContext(ctx, "sh", "-c", script)
@@ -0,0 +1,343 @@
package lifecycle
import (
"context"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"github.com/kandev/kandev/internal/common/logger"
)
func newTestLocalLogger() *logger.Logger {
log, _ := logger.NewLogger(logger.LoggingConfig{
Level: "error",
Format: "json",
})
return log
}
// initGitRepo creates a minimal git repo with an initial commit and returns the path.
func initGitRepo(t *testing.T) string {
t.Helper()
dir := t.TempDir()
// Start from a clean env: filter all GIT_* vars that may leak from parent
// processes (e.g. pre-commit hooks set GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE).
gitEnv := filterLocalTestGitEnv(os.Environ())
gitEnv = append(gitEnv,
"GIT_AUTHOR_NAME=Test",
"GIT_AUTHOR_EMAIL=test@test.local",
"GIT_COMMITTER_NAME=Test",
"GIT_COMMITTER_EMAIL=test@test.local",
"HOME="+dir,
"GIT_CONFIG_GLOBAL=/dev/null",
"GIT_CONFIG_NOSYSTEM=1",
)
for _, args := range [][]string{
{"init", "-b", "main"},
{"config", "core.hooksPath", "/dev/null"},
{"commit", "--allow-empty", "-m", "init"},
} {
cmd := exec.Command("git", args...)
cmd.Dir = dir
cmd.Env = gitEnv
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git %v failed: %s", args, out)
}
}
return dir
}
// filterLocalTestGitEnv removes GIT_* environment variables that can leak from
// parent processes (especially git hooks) and cause test git operations to
// modify the wrong repository.
func filterLocalTestGitEnv(env []string) []string {
filtered := make([]string, 0, len(env))
for _, e := range env {
key, _, _ := strings.Cut(e, "=")
if strings.HasPrefix(key, "GIT_") {
continue
}
filtered = append(filtered, e)
}
return filtered
}
// newIsolatedGitEnv returns a clean environment for test git commands that
// filters leaked GIT_* vars and re-adds isolation + committer identity vars.
func newIsolatedGitEnv() []string {
env := filterLocalTestGitEnv(os.Environ())
return append(env,
"GIT_AUTHOR_NAME=Test",
"GIT_AUTHOR_EMAIL=test@test.local",
"GIT_COMMITTER_NAME=Test",
"GIT_COMMITTER_EMAIL=test@test.local",
"GIT_CONFIG_GLOBAL=/dev/null",
"GIT_CONFIG_NOSYSTEM=1",
)
}
func currentBranch(t *testing.T, repoDir string) string {
t.Helper()
cmd := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD")
cmd.Dir = repoDir
out, err := cmd.Output()
if err != nil {
t.Fatalf("git rev-parse failed: %v", err)
}
return string(out[:len(out)-1]) // trim newline
}
// isolateGitEnv sets env vars to prevent git from reading the user's global
// config (which may have commit signing enabled) during tests, and clears
// GIT_* vars that leak from parent git hooks (pre-commit sets GIT_DIR, etc.).
func isolateGitEnv(t *testing.T) {
t.Helper()
t.Setenv("GIT_CONFIG_GLOBAL", "/dev/null")
t.Setenv("GIT_CONFIG_NOSYSTEM", "1")
// Unset vars set by git hooks that would redirect commands to the host repo.
// Cannot use t.Setenv("", "") because GIT_DIR="" makes git fail differently.
for _, key := range []string{"GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE"} {
if val, ok := os.LookupEnv(key); ok {
_ = os.Unsetenv(key)
t.Cleanup(func() { _ = os.Setenv(key, val) })
}
}
}
func TestLocalPreparer_NoCheckoutBranch(t *testing.T) {
isolateGitEnv(t)
log := newTestLocalLogger()
preparer := NewLocalPreparer(log)
repoDir := initGitRepo(t)
req := &EnvPrepareRequest{
TaskID: "task-1",
RepositoryPath: repoDir,
}
result, err := preparer.Prepare(context.Background(), req, nil)
if err != nil {
t.Fatalf("Prepare() error: %v", err)
}
if !result.Success {
t.Fatalf("Prepare() failed: %s", result.ErrorMessage)
}
// Only 1 step: validate workspace (no checkout step)
if len(result.Steps) != 1 {
t.Fatalf("expected 1 step, got %d", len(result.Steps))
}
if result.Steps[0].Name != "Validate workspace" {
t.Fatalf("expected step name 'Validate workspace', got %q", result.Steps[0].Name)
}
// Branch should still be main
if branch := currentBranch(t, repoDir); branch != "main" {
t.Fatalf("expected branch 'main', got %q", branch)
}
}
func TestLocalPreparer_CheckoutBranchSuccess(t *testing.T) {
isolateGitEnv(t)
log := newTestLocalLogger()
preparer := NewLocalPreparer(log)
repoDir := initGitRepo(t)
// Create a feature branch
env := newIsolatedGitEnv()
for _, args := range [][]string{
{"checkout", "-b", "feature/pr-branch"},
{"commit", "--allow-empty", "-m", "pr commit"},
{"checkout", "main"},
} {
cmd := exec.Command("git", args...)
cmd.Dir = repoDir
cmd.Env = env
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git %v failed: %s", args, out)
}
}
req := &EnvPrepareRequest{
TaskID: "task-1",
RepositoryPath: repoDir,
CheckoutBranch: "feature/pr-branch",
}
result, err := preparer.Prepare(context.Background(), req, nil)
if err != nil {
t.Fatalf("Prepare() error: %v", err)
}
if !result.Success {
t.Fatalf("Prepare() failed: %s", result.ErrorMessage)
}
// 2 steps: validate workspace + checkout branch
if len(result.Steps) != 2 {
t.Fatalf("expected 2 steps, got %d", len(result.Steps))
}
if result.Steps[1].Name != "Checkout branch" {
t.Fatalf("expected step name 'Checkout branch', got %q", result.Steps[1].Name)
}
if result.Steps[1].Status != PrepareStepCompleted {
t.Fatalf("expected checkout step completed, got %q", result.Steps[1].Status)
}
// Branch should be feature/pr-branch
if branch := currentBranch(t, repoDir); branch != "feature/pr-branch" {
t.Fatalf("expected branch 'feature/pr-branch', got %q", branch)
}
}
func TestLocalPreparer_CheckoutBranchNotFound(t *testing.T) {
isolateGitEnv(t)
log := newTestLocalLogger()
preparer := NewLocalPreparer(log)
repoDir := initGitRepo(t)
req := &EnvPrepareRequest{
TaskID: "task-1",
RepositoryPath: repoDir,
CheckoutBranch: "nonexistent-branch",
}
result, err := preparer.Prepare(context.Background(), req, nil)
if err == nil {
t.Fatal("Prepare() should fail when branch doesn't exist")
}
if result.Success {
t.Fatal("expected result.Success = false")
}
// 2 steps: validate (completed) + checkout (failed)
if len(result.Steps) != 2 {
t.Fatalf("expected 2 steps, got %d", len(result.Steps))
}
if result.Steps[1].Status != PrepareStepFailed {
t.Fatalf("expected checkout step failed, got %q", result.Steps[1].Status)
}
// Branch should still be main (checkout failed)
if branch := currentBranch(t, repoDir); branch != "main" {
t.Fatalf("expected branch 'main' after failed checkout, got %q", branch)
}
}
func TestLocalPreparer_CheckoutBranchWithSetupScript(t *testing.T) {
isolateGitEnv(t)
log := newTestLocalLogger()
preparer := NewLocalPreparer(log)
repoDir := initGitRepo(t)
env := newIsolatedGitEnv()
for _, args := range [][]string{
{"checkout", "-b", "feature/pr-branch"},
{"commit", "--allow-empty", "-m", "pr commit"},
{"checkout", "main"},
} {
cmd := exec.Command("git", args...)
cmd.Dir = repoDir
cmd.Env = env
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git %v failed: %s", args, out)
}
}
markerFile := filepath.Join(repoDir, "setup-ran")
req := &EnvPrepareRequest{
TaskID: "task-1",
RepositoryPath: repoDir,
CheckoutBranch: "feature/pr-branch",
SetupScript: "touch " + markerFile,
}
result, err := preparer.Prepare(context.Background(), req, nil)
if err != nil {
t.Fatalf("Prepare() error: %v", err)
}
if !result.Success {
t.Fatalf("Prepare() failed: %s", result.ErrorMessage)
}
// 3 steps: validate + checkout + setup script
if len(result.Steps) != 3 {
t.Fatalf("expected 3 steps, got %d", len(result.Steps))
}
// Branch checked out before script runs
if branch := currentBranch(t, repoDir); branch != "feature/pr-branch" {
t.Fatalf("expected branch 'feature/pr-branch', got %q", branch)
}
// Script ran
if _, err := os.Stat(markerFile); os.IsNotExist(err) {
t.Fatal("setup script did not run")
}
}
func TestLocalPreparer_CheckoutWithDirtyWorkdir(t *testing.T) {
isolateGitEnv(t)
log := newTestLocalLogger()
preparer := NewLocalPreparer(log)
repoDir := initGitRepo(t)
env := newIsolatedGitEnv()
// Create feature branch
for _, args := range [][]string{
{"checkout", "-b", "feature/pr-branch"},
{"commit", "--allow-empty", "-m", "pr commit"},
{"checkout", "main"},
} {
cmd := exec.Command("git", args...)
cmd.Dir = repoDir
cmd.Env = env
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git %v failed: %s", args, out)
}
}
// Create a dirty file that conflicts with checkout
dirtyFile := filepath.Join(repoDir, "dirty.txt")
if err := os.WriteFile(dirtyFile, []byte("dirty"), 0o644); err != nil {
t.Fatal(err)
}
// Stage and modify to create a conflict scenario
addCmd := exec.Command("git", "add", "dirty.txt")
addCmd.Dir = repoDir
addCmd.Env = env
if out, err := addCmd.CombinedOutput(); err != nil {
t.Fatalf("git add failed: %s", out)
}
// Commit on main so both branches diverge on this file
commitCmd := exec.Command("git", "commit", "-m", "add dirty on main")
commitCmd.Dir = repoDir
commitCmd.Env = env
if out, err := commitCmd.CombinedOutput(); err != nil {
t.Fatalf("git commit failed: %s", out)
}
// Now modify the file to make checkout fail
if err := os.WriteFile(dirtyFile, []byte("modified"), 0o644); err != nil {
t.Fatal(err)
}
req := &EnvPrepareRequest{
TaskID: "task-1",
RepositoryPath: repoDir,
CheckoutBranch: "feature/pr-branch",
}
result, err := preparer.Prepare(context.Background(), req, nil)
if err == nil {
t.Fatal("Prepare() should fail with dirty workdir")
}
if result.Success {
t.Fatal("expected result.Success = false")
}
// Should have 2 steps: validate (completed) + checkout (failed)
if len(result.Steps) != 2 {
t.Fatalf("expected 2 steps, got %d", len(result.Steps))
}
if result.Steps[1].Status != PrepareStepFailed {
t.Fatalf("expected checkout step failed, got %q", result.Steps[1].Status)
}
// Branch should still be main
if branch := currentBranch(t, repoDir); branch != "main" {
t.Fatalf("expected branch 'main' after failed checkout, got %q", branch)
}
}
@@ -132,6 +132,10 @@ func (p *WorktreePreparer) Prepare(ctx context.Context, req *EnvPrepareRequest,
if req.CheckoutBranch != "" {
step = beginStep("Fetch PR branch")
reportProgress(onProgress, step, stepIdx, totalSteps)
if wt.FetchWarning != "" {
step.Warning = wt.FetchWarning
step.WarningDetail = wt.FetchWarningDetail
}
completeStepSuccess(&step)
steps = append(steps, step)
reportProgress(onProgress, step, stepIdx, totalSteps)
@@ -42,16 +42,18 @@ type ACPSessionCreatedPayload struct {
// PrepareProgressEventPayload is the payload for environment preparation progress events.
type PrepareProgressEventPayload struct {
TaskID string `json:"task_id"`
SessionID string `json:"session_id"`
ExecutionID string `json:"execution_id"`
StepName string `json:"step_name"`
StepIndex int `json:"step_index"`
TotalSteps int `json:"total_steps"`
Status string `json:"status"`
Output string `json:"output,omitempty"`
Error string `json:"error,omitempty"`
Timestamp string `json:"timestamp"`
TaskID string `json:"task_id"`
SessionID string `json:"session_id"`
ExecutionID string `json:"execution_id"`
StepName string `json:"step_name"`
StepIndex int `json:"step_index"`
TotalSteps int `json:"total_steps"`
Status string `json:"status"`
Output string `json:"output,omitempty"`
Error string `json:"error,omitempty"`
Warning string `json:"warning,omitempty"`
WarningDetail string `json:"warning_detail,omitempty"`
Timestamp string `json:"timestamp"`
}
// GetSessionID returns the session ID for this event (used by event routing).
@@ -61,14 +63,15 @@ func (p PrepareProgressEventPayload) GetSessionID() string {
// PrepareCompletedEventPayload is the payload when environment preparation finishes.
type PrepareCompletedEventPayload struct {
TaskID string `json:"task_id"`
SessionID string `json:"session_id"`
ExecutionID string `json:"execution_id"`
Success bool `json:"success"`
ErrorMessage string `json:"error_message,omitempty"`
DurationMs int64 `json:"duration_ms"`
WorkspacePath string `json:"workspace_path,omitempty"`
Timestamp string `json:"timestamp"`
TaskID string `json:"task_id"`
SessionID string `json:"session_id"`
ExecutionID string `json:"execution_id"`
Success bool `json:"success"`
ErrorMessage string `json:"error_message,omitempty"`
DurationMs int64 `json:"duration_ms"`
WorkspacePath string `json:"workspace_path,omitempty"`
Steps []PrepareStep `json:"steps,omitempty"`
Timestamp string `json:"timestamp"`
}
// GetSessionID returns the session ID for this event (used by event routing).
@@ -153,14 +153,16 @@ func (m *Manager) launchPrepareRequest(req *LaunchRequest, profileInfo *AgentPro
func (m *Manager) newProgressCallback(taskID, sessionID string) PrepareProgressCallback {
return func(step PrepareStep, stepIndex int, totalSteps int) {
m.eventPublisher.PublishPrepareProgress(sessionID, &PrepareProgressEventPayload{
TaskID: taskID,
SessionID: sessionID,
StepName: step.Name,
StepIndex: stepIndex,
TotalSteps: totalSteps,
Status: string(step.Status),
Output: step.Output,
Error: step.Error,
TaskID: taskID,
SessionID: sessionID,
StepName: step.Name,
StepIndex: stepIndex,
TotalSteps: totalSteps,
Status: string(step.Status),
Output: step.Output,
Error: step.Error,
Warning: step.Warning,
WarningDetail: step.WarningDetail,
})
}
}
@@ -291,6 +293,7 @@ func (m *Manager) launchApplyPrepareResult(
SessionID: req.SessionID,
Success: false,
ErrorMessage: result.ErrorMessage,
Steps: result.Steps,
})
return fmt.Errorf("environment preparation failed: %s", result.ErrorMessage)
}
@@ -312,6 +315,7 @@ func (m *Manager) launchApplyPrepareResult(
Success: true,
DurationMs: result.Duration.Milliseconds(),
WorkspacePath: result.WorkspacePath,
Steps: result.Steps,
})
return nil
}
@@ -16,9 +16,28 @@ import (
// setupTestRepo creates a git repository with a remote for testing.
// Returns the repo path and a cleanup function.
// isolateTestGitEnv unsets GIT_* env vars that may be set by parent git hooks
// (pre-commit). This protects both test helper commands AND the WorkspaceTracker's
// internal git commands from operating on the wrong repository.
// Cannot use t.Setenv("", "") because GIT_DIR="" makes git fail differently.
func isolateTestGitEnv(t *testing.T) {
t.Helper()
for _, key := range []string{
"GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE",
"GIT_AUTHOR_DATE", "GIT_COMMITTER_DATE",
} {
if val, ok := os.LookupEnv(key); ok {
_ = os.Unsetenv(key)
t.Cleanup(func() { _ = os.Setenv(key, val) })
}
}
}
func setupTestRepo(t *testing.T) (string, func()) {
t.Helper()
isolateTestGitEnv(t)
// Create temp directory for the "remote" bare repo
remoteDir, err := os.MkdirTemp("", "test-remote-*")
if err != nil {
@@ -259,6 +278,8 @@ func TestGetGitStatus_AheadBehindWithoutUpstream(t *testing.T) {
// 3. User does git fetch && git reset --hard origin/main
// 4. The upstream commits should be filtered out
func TestFilterLocalCommits_PullAndResetScenario(t *testing.T) {
isolateTestGitEnv(t)
// Create temp directories
remoteDir, err := os.MkdirTemp("", "test-remote-*")
if err != nil {
@@ -131,10 +131,10 @@ func (b *TaskEventBroadcaster) subscribe(eventBus bus.EventBus, subject, action
return nil
}
case ws.ActionExecutorPrepareProgress, ws.ActionExecutorPrepareCompleted:
if sessionID != "" {
b.hub.BroadcastToSession(sessionID, msg)
return nil
}
// Broadcast globally so prepare progress/warnings are available
// when the user navigates to the session page after task creation.
b.hub.Broadcast(msg)
return nil
}
b.hub.Broadcast(msg)
return nil
+7 -6
View File
@@ -245,12 +245,13 @@ type GetTaskRequest struct {
}
type TaskRepositoryInput struct {
RepositoryID string
BaseBranch string
LocalPath string
Name string
DefaultBranch string
GitHubURL string
RepositoryID string
BaseBranch string
CheckoutBranch string
LocalPath string
Name string
DefaultBranch string
GitHubURL string
}
type CreateTaskRequest struct {
@@ -101,12 +101,13 @@ func convertToServiceRepos(repos []dto.TaskRepositoryInput) []service.TaskReposi
result := make([]service.TaskRepositoryInput, len(repos))
for i, r := range repos {
result[i] = service.TaskRepositoryInput{
RepositoryID: r.RepositoryID,
BaseBranch: r.BaseBranch,
LocalPath: r.LocalPath,
Name: r.Name,
DefaultBranch: r.DefaultBranch,
GitHubURL: r.GitHubURL,
RepositoryID: r.RepositoryID,
BaseBranch: r.BaseBranch,
CheckoutBranch: r.CheckoutBranch,
LocalPath: r.LocalPath,
Name: r.Name,
DefaultBranch: r.DefaultBranch,
GitHubURL: r.GitHubURL,
}
}
return result
@@ -284,12 +284,13 @@ func (h *TaskHandlers) httpBulkMoveTasks(c *gin.Context) {
}
type httpTaskRepositoryInput struct {
RepositoryID string `json:"repository_id"`
BaseBranch string `json:"base_branch"`
LocalPath string `json:"local_path"`
Name string `json:"name"`
DefaultBranch string `json:"default_branch"`
GitHubURL string `json:"github_url"`
RepositoryID string `json:"repository_id"`
BaseBranch string `json:"base_branch"`
CheckoutBranch string `json:"checkout_branch"`
LocalPath string `json:"local_path"`
Name string `json:"name"`
DefaultBranch string `json:"default_branch"`
GitHubURL string `json:"github_url"`
}
type httpCreateTaskRequest struct {
@@ -377,12 +378,13 @@ func convertCreateTaskRepositories(c *gin.Context, inputs []httpTaskRepositoryIn
return nil, false
}
repos = append(repos, dto.TaskRepositoryInput{
RepositoryID: r.RepositoryID,
BaseBranch: r.BaseBranch,
LocalPath: r.LocalPath,
Name: r.Name,
DefaultBranch: r.DefaultBranch,
GitHubURL: r.GitHubURL,
RepositoryID: r.RepositoryID,
BaseBranch: r.BaseBranch,
CheckoutBranch: r.CheckoutBranch,
LocalPath: r.LocalPath,
Name: r.Name,
DefaultBranch: r.DefaultBranch,
GitHubURL: r.GitHubURL,
})
}
return repos, true
+41 -1
View File
@@ -1,7 +1,11 @@
// Package worktree provides Git worktree management for concurrent agent execution.
package worktree
import "errors"
import (
"errors"
"fmt"
"strings"
)
var (
// ErrWorktreeExists is returned when attempting to create a worktree that already exists.
@@ -30,4 +34,40 @@ var (
// ErrInvalidSession is returned when the session ID is invalid or empty.
ErrInvalidSession = errors.New("invalid or empty session ID")
// ErrBranchCheckedOut is returned when a branch is already checked out in another worktree.
ErrBranchCheckedOut = errors.New("branch is already checked out in another worktree")
// ErrAuthFailed is returned when git authentication fails (e.g. no credentials in non-interactive mode).
ErrAuthFailed = errors.New("git authentication failed")
// ErrNonFastForward is returned when a fetch/pull is rejected due to non-fast-forward updates.
ErrNonFastForward = errors.New("non-fast-forward update rejected")
)
// containsAuthFailure checks if git output indicates an authentication failure.
func containsAuthFailure(lowerOutput string) bool {
return strings.Contains(lowerOutput, "authentication failed") ||
strings.Contains(lowerOutput, "terminal prompts disabled") ||
strings.Contains(lowerOutput, "could not read username") ||
strings.Contains(lowerOutput, "username for 'https://") ||
strings.Contains(lowerOutput, "askpass")
}
// ClassifyGitError wraps a raw git error with a user-friendly sentinel error
// based on the command output.
func ClassifyGitError(output string, _ error) error {
out := strings.ToLower(output)
trimmed := strings.TrimSpace(output)
switch {
case strings.Contains(out, "is already checked out at"):
return fmt.Errorf("%w: %s", ErrBranchCheckedOut, trimmed)
case containsAuthFailure(out):
return fmt.Errorf("%w: %s", ErrAuthFailed, trimmed)
case strings.Contains(out, "non-fast-forward"):
return fmt.Errorf("%w: %s", ErrNonFastForward, trimmed)
default:
return fmt.Errorf("%w: %s", ErrGitCommandFailed, trimmed)
}
}
@@ -0,0 +1,104 @@
package worktree
import (
"errors"
"fmt"
"testing"
)
func TestClassifyGitError_BranchCheckedOut(t *testing.T) {
output := "fatal: 'feature/pr-branch' is already checked out at '/tmp/worktree-123'"
err := ClassifyGitError(output, fmt.Errorf("exit status 128"))
if !errors.Is(err, ErrBranchCheckedOut) {
t.Fatalf("expected ErrBranchCheckedOut, got: %v", err)
}
}
func TestClassifyGitError_AuthFailed(t *testing.T) {
tests := []struct {
name string
output string
}{
{"could not read username", "fatal: could not read Username for 'https://github.com': terminal prompts disabled"},
{"authentication failed", "fatal: Authentication failed for 'https://github.com/repo.git'"},
{"askpass", "fatal: could not read Password via askpass"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ClassifyGitError(tt.output, fmt.Errorf("exit status 128"))
if !errors.Is(err, ErrAuthFailed) {
t.Fatalf("expected ErrAuthFailed, got: %v", err)
}
})
}
}
func TestClassifyGitError_NonFastForward(t *testing.T) {
output := "! [rejected] main -> main (non-fast-forward)"
err := ClassifyGitError(output, fmt.Errorf("exit status 1"))
if !errors.Is(err, ErrNonFastForward) {
t.Fatalf("expected ErrNonFastForward, got: %v", err)
}
}
func TestClassifyGitError_GenericFailure(t *testing.T) {
output := "fatal: some unknown git error"
err := ClassifyGitError(output, fmt.Errorf("exit status 1"))
if !errors.Is(err, ErrGitCommandFailed) {
t.Fatalf("expected ErrGitCommandFailed, got: %v", err)
}
}
func TestContainsAuthFailure(t *testing.T) {
tests := []struct {
input string
expected bool
}{
{"authentication failed", true},
{"terminal prompts disabled", true},
{"could not read username", true},
{"username for 'https://github.com'", true},
{"askpass error", true},
{"branch not found", false},
{"", false},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
if got := containsAuthFailure(tt.input); got != tt.expected {
t.Fatalf("containsAuthFailure(%q) = %v, want %v", tt.input, got, tt.expected)
}
})
}
}
func TestParseCheckedOutPath(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
"standard git output",
"fatal: 'feature/pr-branch' is already checked out at '/tmp/worktrees/my-worktree'",
"/tmp/worktrees/my-worktree",
},
{
"no match",
"fatal: some other error",
"",
},
{
"no closing quote",
"fatal: 'branch' is already checked out at '/path",
"",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := parseCheckedOutPath(tt.input)
if got != tt.expected {
t.Fatalf("parseCheckedOutPath(%q) = %q, want %q", tt.input, got, tt.expected)
}
})
}
}
+141 -22
View File
@@ -249,30 +249,42 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err
// createWorktree performs the actual git worktree creation.
func (m *Manager) createWorktree(ctx context.Context, req CreateRequest, baseRef string) (*Worktree, error) {
worktreeDirName, branchName := m.buildWorktreeNames(req)
// If a checkout branch is specified (e.g. a PR's head branch), fetch it
// first and use it as the base so the new worktree branch is created on
// top of the PR branch rather than on top of the default branch.
// and checkout it directly in the worktree instead of creating a new branch.
var fetchResult *FetchBranchResult
if req.CheckoutBranch != "" {
fetchedRef, err := m.fetchBranchRef(ctx, req.RepositoryPath, req.CheckoutBranch)
var err error
fetchResult, err = m.fetchBranchToLocal(ctx, req.RepositoryPath, req.CheckoutBranch)
if err != nil {
return nil, err
}
baseRef = fetchedRef
branchName = req.CheckoutBranch
}
worktreeDirName, branchName := m.buildWorktreeNames(req)
worktreePath, err := m.config.WorktreePath(worktreeDirName)
if err != nil {
return nil, fmt.Errorf("failed to get worktree path: %w", err)
}
worktreeID, err := m.gitAddWorktree(ctx, req.RepositoryPath, branchName, worktreePath, baseRef)
var worktreeID string
if req.CheckoutBranch != "" {
// Checkout the existing PR branch directly in the worktree.
worktreeID, err = m.gitAddWorktreeExisting(ctx, req.RepositoryPath, branchName, worktreePath)
} else {
// Create a new branch based on the base ref.
worktreeID, err = m.gitAddWorktree(ctx, req.RepositoryPath, branchName, worktreePath, baseRef)
}
if err != nil {
return nil, err
}
wt := m.buildWorktreeRecord(worktreeID, req, worktreePath, branchName)
if fetchResult != nil {
wt.FetchWarning = fetchResult.Warning
wt.FetchWarningDetail = fetchResult.WarningDetail
}
if err := m.persistAndCacheWorktree(ctx, wt, req, worktreePath); err != nil {
return nil, err
@@ -291,24 +303,136 @@ func (m *Manager) createWorktree(ctx context.Context, req CreateRequest, baseRef
return wt, nil
}
// fetchBranchRef fetches a branch from origin in the given repository and returns
// the FETCH_HEAD ref that can be used as the base for a new worktree branch.
// This is used for PR reviews so the new worktree branch is created on top of
// the PR's head branch.
func (m *Manager) fetchBranchRef(ctx context.Context, repoPath, branch string) (string, error) {
m.logger.Info("fetching branch for worktree base",
// FetchBranchResult holds the outcome of a fetchBranchToLocal call.
type FetchBranchResult struct {
Warning string // User-friendly warning (non-empty when fell back to local)
WarningDetail string // Raw git command output for debugging
}
// fetchBranchToLocal ensures a branch exists locally and is up-to-date.
// It first tries to fetch from origin to get the latest commits. If the fetch
// fails (no remote, auth issue, offline), it falls back to the local branch.
// Returns a FetchBranchResult with warning info and an error if the branch
// doesn't exist anywhere.
func (m *Manager) fetchBranchToLocal(ctx context.Context, repoPath, branch string) (*FetchBranchResult, error) {
m.logger.Info("syncing checkout branch",
zap.String("branch", branch),
zap.String("repo_path", repoPath))
// Try to fetch from origin to get the latest version.
fetchCtx, cancelFetch := context.WithTimeout(ctx, 30*time.Second)
defer cancelFetch()
fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", "origin", branch)
fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", "origin", branch+":"+branch)
if output, err := fetchCmd.CombinedOutput(); err != nil {
return "", fmt.Errorf("failed to fetch branch %q: %s", branch, string(output))
m.logger.Warn("fetch from origin failed, checking local branch",
zap.String("branch", branch),
zap.String("output", string(output)),
zap.Error(err))
// Fall back to local branch if it exists.
if !m.branchExists(repoPath, branch) {
return nil, fmt.Errorf("branch %q not found locally or on remote: %s", branch, string(output))
}
reason := classifyGitFallbackReason(err, string(output), fetchCtx.Err())
warning := fmt.Sprintf("Could not fetch latest from origin (%s). Using local version of branch %q which may be outdated.", reason, branch)
m.logger.Info("using local branch (fetch failed)",
zap.String("branch", branch),
zap.String("warning", warning))
return &FetchBranchResult{
Warning: warning,
WarningDetail: strings.TrimSpace(string(output)),
}, nil
}
return "FETCH_HEAD", nil
return &FetchBranchResult{}, nil
}
// gitAddWorktreeExisting creates a worktree that checks out an existing local branch.
// If the branch is already checked out in a stale worktree (directory no longer exists),
// it automatically prunes and retries.
func (m *Manager) gitAddWorktreeExisting(ctx context.Context, repoPath, branchName, worktreePath string) (string, error) {
worktreeID := uuid.New().String()
cmd := exec.CommandContext(ctx, "git", "worktree", "add", worktreePath, branchName)
cmd.Dir = repoPath
output, err := cmd.CombinedOutput()
if err == nil {
return worktreeID, nil
}
outStr := string(output)
if !strings.Contains(strings.ToLower(outStr), "is already checked out at") {
m.logger.Error("git worktree add (existing branch) failed",
zap.String("output", outStr), zap.Error(err))
return "", ClassifyGitError(outStr, err)
}
if recoveryErr := m.tryRecoverCheckedOutBranch(ctx, repoPath, branchName, outStr); recoveryErr != nil {
m.logger.Error("git worktree add (existing branch) failed",
zap.String("output", outStr), zap.Error(err))
return "", ClassifyGitError(outStr, err)
}
// Retry after pruning stale worktree.
retryCmd := exec.CommandContext(ctx, "git", "worktree", "add", worktreePath, branchName)
retryCmd.Dir = repoPath
if retryOutput, retryErr := retryCmd.CombinedOutput(); retryErr != nil {
m.logger.Error("git worktree add retry failed",
zap.String("output", string(retryOutput)), zap.Error(retryErr))
return "", ClassifyGitError(string(retryOutput), retryErr)
}
m.logger.Info("recovered from stale worktree checkout", zap.String("branch", branchName))
return worktreeID, nil
}
// tryRecoverCheckedOutBranch attempts to recover from "branch is already checked out"
// by pruning stale worktrees. Returns nil if recovery succeeded, error otherwise.
func (m *Manager) tryRecoverCheckedOutBranch(ctx context.Context, repoPath, branchName, gitOutput string) error {
// Parse the path from git output: "fatal: 'branch' is already checked out at '/path/to/worktree'"
checkedOutPath := parseCheckedOutPath(gitOutput)
if checkedOutPath == "" {
return fmt.Errorf("could not parse worktree path from git output")
}
// Check if the worktree directory still exists on disk.
if _, err := os.Stat(checkedOutPath); err == nil {
// Directory exists — worktree is genuinely in use, can't recover.
m.logger.Warn("branch checked out in active worktree, cannot recover",
zap.String("branch", branchName),
zap.String("worktree_path", checkedOutPath))
return fmt.Errorf("worktree at %s is still active", checkedOutPath)
}
// Directory is gone — prune stale worktree references.
m.logger.Info("pruning stale worktree reference",
zap.String("branch", branchName),
zap.String("stale_path", checkedOutPath))
pruneCmd := exec.CommandContext(ctx, "git", "worktree", "prune")
pruneCmd.Dir = repoPath
if output, err := pruneCmd.CombinedOutput(); err != nil {
m.logger.Error("git worktree prune failed",
zap.String("output", string(output)),
zap.Error(err))
return fmt.Errorf("worktree prune failed: %w", err)
}
return nil
}
// parseCheckedOutPath extracts the worktree path from git's
// "fatal: 'branch' is already checked out at '/path'" error message.
func parseCheckedOutPath(gitOutput string) string {
const marker = "checked out at '"
_, after, found := strings.Cut(gitOutput, marker)
if !found {
return ""
}
path, _, found := strings.Cut(after, "'")
if !found {
return ""
}
return path
}
// buildWorktreeNames derives the filesystem directory name and git branch name for a new worktree.
@@ -801,12 +925,7 @@ func classifyGitFallbackReason(cmdErr error, cmdOutput string, ctxErr error) str
return "timeout"
}
out := strings.ToLower(cmdOutput)
if strings.Contains(out, "authentication failed") ||
strings.Contains(out, "terminal prompts disabled") ||
strings.Contains(out, "could not read username") ||
strings.Contains(out, "username for 'https://") ||
strings.Contains(out, "askpass") {
if containsAuthFailure(strings.ToLower(cmdOutput)) {
return "non_interactive_auth_failed"
}
@@ -779,6 +779,122 @@ esac
}
}
func TestFetchBranchToLocal_FetchSucceeds(t *testing.T) {
scriptDir := writeFakeGitScript(t, `
case "${1:-}" in
fetch)
# Simulate successful fetch
exit 0
;;
*)
exit 0
;;
esac
`)
t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH"))
cfg := newTestConfig(t)
log := newTestLogger()
store := newMockStore()
mgr, err := NewManager(cfg, store, log)
if err != nil {
t.Fatalf("NewManager failed: %v", err)
}
repoPath := t.TempDir()
result, err := mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch")
if err != nil {
t.Fatalf("fetchBranchToLocal() unexpected error: %v", err)
}
if result.Warning != "" {
t.Fatalf("expected no warning on successful fetch, got %q", result.Warning)
}
}
func TestFetchBranchToLocal_FetchFailsLocalBranchExists(t *testing.T) {
scriptDir := writeFakeGitScript(t, `
case "${1:-}" in
fetch)
echo "fatal: no remote configured" >&2
exit 1
;;
rev-parse)
# Simulate branch exists locally
if [ "${2:-}" = "--verify" ]; then
exit 0
fi
exit 0
;;
*)
exit 0
;;
esac
`)
t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH"))
cfg := newTestConfig(t)
log := newTestLogger()
store := newMockStore()
mgr, err := NewManager(cfg, store, log)
if err != nil {
t.Fatalf("NewManager failed: %v", err)
}
repoPath := t.TempDir()
result, err := mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch")
if err != nil {
t.Fatalf("fetchBranchToLocal() should fall back to local branch, got error: %v", err)
}
if result.Warning == "" {
t.Fatal("expected a warning when falling back to local branch")
}
if !strings.Contains(result.Warning, "Could not fetch latest from origin") {
t.Fatalf("unexpected warning: %q", result.Warning)
}
if result.WarningDetail == "" {
t.Fatal("expected warning detail with raw git output")
}
}
func TestFetchBranchToLocal_FetchFailsNoBranch(t *testing.T) {
scriptDir := writeFakeGitScript(t, `
case "${1:-}" in
fetch)
echo "fatal: no remote configured" >&2
exit 1
;;
rev-parse)
# Simulate branch does NOT exist
if [ "${2:-}" = "--verify" ]; then
exit 1
fi
exit 0
;;
*)
exit 0
;;
esac
`)
t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH"))
cfg := newTestConfig(t)
log := newTestLogger()
store := newMockStore()
mgr, err := NewManager(cfg, store, log)
if err != nil {
t.Fatalf("NewManager failed: %v", err)
}
repoPath := t.TempDir()
_, err = mgr.fetchBranchToLocal(context.Background(), repoPath, "feature/pr-branch")
if err == nil {
t.Fatal("fetchBranchToLocal() should fail when branch not found anywhere")
}
if !strings.Contains(err.Error(), "not found locally or on remote") {
t.Fatalf("unexpected error message: %v", err)
}
}
func TestClassifyGitFallbackReason_AuthPrompt(t *testing.T) {
reason := classifyGitFallbackReason(nil, "fatal: could not read Username for 'https://github.com'", nil)
if reason != "non_interactive_auth_failed" {
@@ -64,6 +64,14 @@ type Worktree struct {
// DeletedAt is when this worktree was deleted (if applicable).
DeletedAt *time.Time `json:"deleted_at,omitempty"`
// FetchWarning is a non-fatal warning from fetching the checkout branch.
// Set when fetch from origin failed but a local branch was used as fallback.
FetchWarning string `json:"fetch_warning,omitempty"`
// FetchWarningDetail contains the raw git command output for debugging.
// Shown as collapsible content alongside the user-friendly FetchWarning.
FetchWarningDetail string `json:"fetch_warning_detail,omitempty"`
}
// CreateRequest contains the parameters for creating a new worktree.
@@ -1,6 +1,14 @@
"use client";
import { IconCheck, IconX, IconLoader2 } from "@tabler/icons-react";
import { useState } from "react";
import {
IconCheck,
IconX,
IconLoader2,
IconAlertTriangle,
IconChevronDown,
IconChevronRight,
} from "@tabler/icons-react";
import { useAppStore } from "@/components/state-provider";
import type { PrepareStepInfo } from "@/lib/state/slices/session-runtime/types";
@@ -8,7 +16,10 @@ type PrepareProgressProps = {
sessionId: string;
};
function StepIcon({ status }: { status: string }) {
function StepIcon({ status, hasWarning }: { status: string; hasWarning?: boolean }) {
if (status === "completed" && hasWarning) {
return <IconAlertTriangle className="h-3.5 w-3.5 text-amber-500" />;
}
if (status === "completed") {
return <IconCheck className="h-3.5 w-3.5 text-green-500" />;
}
@@ -22,10 +33,11 @@ function StepIcon({ status }: { status: string }) {
}
function StepRow({ step }: { step: PrepareStepInfo }) {
const [detailExpanded, setDetailExpanded] = useState(false);
return (
<div className="flex items-start gap-2 text-xs">
<div className="mt-0.5 flex-shrink-0">
<StepIcon status={step.status} />
<StepIcon status={step.status} hasWarning={Boolean(step.warning)} />
</div>
<div className="min-w-0 flex-1">
<span className="text-muted-foreground">{step.name || "Preparing..."}</span>
@@ -34,6 +46,34 @@ function StepRow({ step }: { step: PrepareStepInfo }) {
{step.output}
</pre>
)}
{step.warning && (
<div className="mt-0.5">
<pre className="text-amber-500 overflow-x-auto whitespace-pre text-xs">
{step.warning}
</pre>
{step.warningDetail && (
<>
<button
type="button"
className="flex items-center gap-0.5 text-amber-500/70 hover:text-amber-500 mt-0.5 cursor-pointer"
onClick={() => setDetailExpanded((v) => !v)}
>
{detailExpanded ? (
<IconChevronDown className="h-3 w-3" />
) : (
<IconChevronRight className="h-3 w-3" />
)}
<span>Details</span>
</button>
{detailExpanded && (
<pre className="text-amber-500/70 mt-0.5 overflow-x-auto whitespace-pre text-xs">
{step.warningDetail}
</pre>
)}
</>
)}
</div>
)}
{step.error && (
<pre className="text-destructive mt-0.5 overflow-x-auto whitespace-pre text-xs">
{step.error}
@@ -54,10 +94,32 @@ function useEffectivePrepareStatus(sessionId: string) {
return profile?.label ?? null;
});
const hasWarnings = prepareState?.steps.some((s) => s.warning);
if (!prepareState)
return { visible: false, status: "preparing", prepareState, profileLabel } as const;
if (prepareState.status === "completed")
return { visible: false, status: "completed", prepareState, profileLabel } as const;
return {
visible: false,
status: "preparing",
prepareState,
profileLabel,
hasWarnings: false,
} as const;
if (prepareState.status === "completed" && !hasWarnings)
return {
visible: false,
status: "completed",
prepareState,
profileLabel,
hasWarnings: false,
} as const;
if (prepareState.status === "completed" && hasWarnings)
return {
visible: true,
status: "completed_with_warnings",
prepareState,
profileLabel,
hasWarnings: true,
} as const;
// If session reached a terminal state but prepare status is still "preparing",
// treat it as failed (the completed event may not have arrived)
@@ -66,15 +128,75 @@ function useEffectivePrepareStatus(sessionId: string) {
const status =
prepareState.status === "preparing" && isSessionTerminal ? "failed" : prepareState.status;
return { visible: true, status, prepareState, profileLabel } as const;
return {
visible: true,
status,
prepareState,
profileLabel,
hasWarnings: hasWarnings ?? false,
} as const;
}
export function PrepareProgress({ sessionId }: PrepareProgressProps) {
const { visible, status, prepareState, profileLabel } = useEffectivePrepareStatus(sessionId);
const [dismissed, setDismissed] = useState(false);
const [expandedDetails, setExpandedDetails] = useState<Record<number, boolean>>({});
if (!visible || !prepareState) return null;
// After successful completion with warnings, show only the warning steps.
if (status === "completed_with_warnings") {
if (dismissed) return null;
const warningSteps = prepareState.steps
.map((s, i) => ({ step: s, index: i }))
.filter(({ step }) => step.warning);
return (
<div data-testid="prepare-warning-banner" className="px-3 py-1.5 border-b border-border/50">
{warningSteps.map(({ step, index }) => (
<div key={index} className="text-xs text-amber-500">
<div className="flex items-center gap-1.5">
<IconAlertTriangle className="h-3 w-3 flex-shrink-0" />
<span className="flex-1">{step.warning}</span>
{step.warningDetail && (
<button
type="button"
className="flex items-center gap-0.5 text-amber-500/70 hover:text-amber-500 cursor-pointer"
onClick={() => setExpandedDetails((prev) => ({ ...prev, [index]: !prev[index] }))}
>
{expandedDetails[index] ? (
<IconChevronDown className="h-3 w-3" />
) : (
<IconChevronRight className="h-3 w-3" />
)}
<span>Details</span>
</button>
)}
<button
type="button"
className="text-amber-500/70 hover:text-amber-500 ml-1 cursor-pointer"
onClick={() => setDismissed(true)}
aria-label="Dismiss warning"
>
<IconX className="h-3 w-3" />
</button>
</div>
{step.warningDetail && expandedDetails[index] && (
<pre className="mt-1 ml-4.5 text-amber-500/70 overflow-x-auto whitespace-pre text-xs">
{step.warningDetail}
</pre>
)}
</div>
))}
</div>
);
}
const visibleSteps = prepareState.steps.filter(
(step) => step.name.trim() !== "" || Boolean(step.output) || Boolean(step.error),
(step) =>
step.name.trim() !== "" ||
Boolean(step.output) ||
Boolean(step.error) ||
Boolean(step.warning),
);
return (
@@ -31,6 +31,7 @@ export type SubmitHandlersDeps = {
selectedLocalRepo: LocalRepository | null;
useGitHubUrl: boolean;
githubUrl: string;
githubPrHeadBranch: string | null;
branch: string;
agentProfileId: string;
executorId: string;
@@ -79,6 +80,7 @@ export function useTaskSubmitHandlers({
selectedLocalRepo,
useGitHubUrl,
githubUrl,
githubPrHeadBranch,
branch,
agentProfileId,
executorId,
@@ -136,6 +138,7 @@ export function useTaskSubmitHandlers({
{
repository_id: "",
base_branch: branch || undefined,
checkout_branch: githubPrHeadBranch || undefined,
github_url: githubUrl,
},
];
@@ -154,7 +157,7 @@ export function useTaskSubmitHandlers({
];
}
return [];
}, [useGitHubUrl, repositoryId, branch, githubUrl, selectedLocalRepo]);
}, [useGitHubUrl, repositoryId, branch, githubUrl, githubPrHeadBranch, selectedLocalRepo]);
const handleSessionSubmit = useCallback(async () => {
const description = descriptionInputRef.current?.getValue() ?? "";
+18 -17
View File
@@ -185,9 +185,9 @@ function DialogHeaderContent(props: DialogHeaderContentProps) {
if (isCreateMode || isEditMode) {
return (
<DialogTitle asChild>
<div className="flex flex-col gap-1 min-w-0">
<div className="flex items-center gap-1 text-sm font-medium">
<>
<DialogTitle asChild>
<div className="flex items-center gap-1 text-sm font-medium min-w-0">
<RepoSourceInput {...props} />
<span className="text-muted-foreground mr-2">/</span>
<InlineTaskName
@@ -196,20 +196,20 @@ function DialogHeaderContent(props: DialogHeaderContentProps) {
autoFocus={!isEditMode && !useGitHubUrl}
/>
</div>
{!isTaskStarted && (
<div className="flex items-center gap-2 pl-2">
<button
type="button"
onClick={onToggleGitHubUrl}
className="text-xs text-muted-foreground hover:text-foreground cursor-pointer transition-colors"
data-testid="toggle-github-url"
>
{useGitHubUrl ? "or select a repository" : "or paste a GitHub URL"}
</button>
</div>
)}
</div>
</DialogTitle>
</DialogTitle>
{!isTaskStarted && (
<div className="flex items-center gap-2 pl-2">
<button
type="button"
onClick={onToggleGitHubUrl}
className="text-xs text-muted-foreground hover:text-foreground cursor-pointer transition-colors"
data-testid="toggle-github-url"
>
{useGitHubUrl ? "or select a repository" : "or paste a GitHub URL"}
</button>
</div>
)}
</>
);
}
return (
@@ -407,6 +407,7 @@ function useTaskCreateDialogSetup(props: TaskCreateDialogProps) {
selectedLocalRepo: fs.selectedLocalRepo,
useGitHubUrl: fs.useGitHubUrl,
githubUrl: fs.githubUrl,
githubPrHeadBranch: fs.githubPrHeadBranch,
branch: fs.branch,
agentProfileId: fs.agentProfileId,
executorId: fs.executorId,
@@ -334,6 +334,413 @@ test.describe("Task creation from GitHub URL", () => {
});
});
test("PR URL auto-selects the PR head branch", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
// Pre-seed a GitHub-backed repository
const repoDir = `${backend.tmpDir}/repos/e2e-repo`;
await apiClient.createRepository(seedData.workspaceId, repoDir, "main", {
name: "test-owner/test-repo",
provider: "github",
provider_owner: "test-owner",
provider_name: "test-repo",
});
// Seed branches including the PR head branch
await apiClient.mockGitHubAddBranches("test-owner", "test-repo", [
{ name: "main" },
{ name: "feature/pr-branch" },
]);
// Seed a PR pointing to the feature branch
await apiClient.mockGitHubAddPRs([
{
number: 42,
title: "Test PR",
state: "open",
head_branch: "feature/pr-branch",
base_branch: "main",
author_login: "test-user",
repo_owner: "test-owner",
repo_name: "test-repo",
},
]);
const kanban = new KanbanPage(testPage);
await kanban.goto();
await kanban.createTaskButton.first().click();
const dialog = testPage.getByTestId("create-task-dialog");
await expect(dialog).toBeVisible();
// Toggle to GitHub URL mode and enter a PR URL
await testPage.getByTestId("toggle-github-url").click();
await testPage
.getByTestId("github-url-input")
.fill("https://github.com/test-owner/test-repo/pull/42");
// The PR head branch should be auto-selected in the branch selector
await expect(dialog.getByText("feature/pr-branch")).toBeVisible({ timeout: 10_000 });
});
test("creates task from PR URL with local executor end-to-end", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
test.setTimeout(90_000);
const { execSync } = await import("child_process");
const gitEnv = {
...process.env,
HOME: backend.tmpDir,
GIT_AUTHOR_NAME: "E2E Test",
GIT_AUTHOR_EMAIL: "e2e@test.local",
GIT_COMMITTER_NAME: "E2E Test",
GIT_COMMITTER_EMAIL: "e2e@test.local",
};
// Create the feature/pr-branch in the local repo so git checkout works
const repoDir = `${backend.tmpDir}/repos/e2e-repo`;
execSync("git checkout -b feature/pr-branch", { cwd: repoDir, env: gitEnv });
execSync('git commit --allow-empty -m "pr commit"', { cwd: repoDir, env: gitEnv });
execSync("git checkout main", { cwd: repoDir, env: gitEnv });
// Pre-seed a GitHub-backed repository
await apiClient.createRepository(seedData.workspaceId, repoDir, "main", {
name: "test-owner/test-repo",
provider: "github",
provider_owner: "test-owner",
provider_name: "test-repo",
});
await apiClient.mockGitHubAddBranches("test-owner", "test-repo", [
{ name: "main" },
{ name: "feature/pr-branch" },
]);
await apiClient.mockGitHubAddPRs([
{
number: 99,
title: "Feature PR",
state: "open",
head_branch: "feature/pr-branch",
base_branch: "main",
author_login: "test-user",
repo_owner: "test-owner",
repo_name: "test-repo",
},
]);
const kanban = new KanbanPage(testPage);
await kanban.goto();
await kanban.createTaskButton.first().click();
const dialog = testPage.getByTestId("create-task-dialog");
await expect(dialog).toBeVisible();
// Toggle to GitHub URL mode and enter the PR URL
await testPage.getByTestId("toggle-github-url").click();
await testPage
.getByTestId("github-url-input")
.fill("https://github.com/test-owner/test-repo/pull/99");
// Verify PR head branch is auto-selected
await expect(dialog.getByText("feature/pr-branch")).toBeVisible({ timeout: 10_000 });
// Fill in title and description
await testPage.getByTestId("task-title-input").fill("PR Task Local");
await testPage.getByTestId("task-description-input").fill("/e2e:simple-message");
// Wait for the start button to become enabled
const startBtn = testPage.getByTestId("submit-start-agent");
await expect(startBtn).toBeEnabled({ timeout: 15_000 });
// Submit (default executor is local/standalone)
await startBtn.click();
await expect(dialog).not.toBeVisible({ timeout: 10_000 });
// The new task card appears on the kanban board
const card = kanban.taskCardByTitle("PR Task Local");
await expect(card).toBeVisible({ timeout: 10_000 });
// Navigate to the session and verify the agent completes
await card.click();
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
const session = new SessionPage(testPage);
await session.waitForLoad();
await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible({
timeout: 30_000,
});
await expect(session.idleInput()).toBeVisible({ timeout: 15_000 });
// Verify the local executor checked out the PR branch.
await expect(session.terminal).toBeVisible({ timeout: 15_000 });
await session.typeInTerminal("git branch --show-current");
await session.expectTerminalHasText("feature/pr-branch");
});
test("creates task from PR URL with worktree executor and verifies branch", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
test.setTimeout(90_000);
const { execSync } = await import("child_process");
const fs = await import("fs");
const gitEnv = {
...process.env,
HOME: backend.tmpDir,
GIT_AUTHOR_NAME: "E2E Test",
GIT_AUTHOR_EMAIL: "e2e@test.local",
GIT_COMMITTER_NAME: "E2E Test",
GIT_COMMITTER_EMAIL: "e2e@test.local",
};
// Create a fresh repo with the PR branch available locally.
// No remote needed — the worktree manager falls back to local branches.
const repoDir = `${backend.tmpDir}/repos/e2e-pr-wt`;
fs.mkdirSync(repoDir, { recursive: true });
execSync("git init -b main", { cwd: repoDir, env: gitEnv });
execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv });
// Create the PR branch locally and switch back to main
execSync("git checkout -b feature/pr-branch", { cwd: repoDir, env: gitEnv });
execSync('git commit --allow-empty -m "pr branch commit"', { cwd: repoDir, env: gitEnv });
execSync("git checkout main", { cwd: repoDir, env: gitEnv });
// Register the repo with a unique provider name to avoid collisions with
// other tests that also register repos as test-owner/test-repo.
await apiClient.createRepository(seedData.workspaceId, repoDir, "main", {
name: "pr-owner/pr-wt-repo",
provider: "github",
provider_owner: "pr-owner",
provider_name: "pr-wt-repo",
});
// Seed mock GitHub branches and PR
await apiClient.mockGitHubAddBranches("pr-owner", "pr-wt-repo", [
{ name: "main" },
{ name: "feature/pr-branch" },
]);
await apiClient.mockGitHubAddPRs([
{
number: 77,
title: "Worktree PR",
state: "open",
head_branch: "feature/pr-branch",
base_branch: "main",
author_login: "test-user",
repo_owner: "pr-owner",
repo_name: "pr-wt-repo",
},
]);
// Look up the worktree executor profile
const { executors } = await apiClient.listExecutors();
const worktreeExec = executors.find((e) => e.type === "worktree");
const worktreeProfile = worktreeExec?.profiles?.[0];
if (!worktreeProfile) {
test.skip(true, "No worktree executor profile available");
return;
}
const kanban = new KanbanPage(testPage);
await kanban.goto();
await kanban.createTaskButton.first().click();
const dialog = testPage.getByTestId("create-task-dialog");
await expect(dialog).toBeVisible();
// Toggle to GitHub URL mode and enter the PR URL
await testPage.getByTestId("toggle-github-url").click();
await testPage
.getByTestId("github-url-input")
.fill("https://github.com/pr-owner/pr-wt-repo/pull/77");
// Verify PR head branch is auto-selected
await expect(dialog.getByText("feature/pr-branch")).toBeVisible({ timeout: 10_000 });
// Fill in title and description
await testPage.getByTestId("task-title-input").fill("PR Worktree Task");
await testPage.getByTestId("task-description-input").fill("/e2e:simple-message");
// Wait for selectors to be ready
const startBtn = testPage.getByTestId("submit-start-agent");
await expect(startBtn).toBeEnabled({ timeout: 15_000 });
// Select the worktree executor profile
const executorSelector = testPage.getByTestId("executor-profile-selector");
await executorSelector.click();
await testPage.getByRole("option", { name: /Worktree/i }).click();
// Submit
await startBtn.click();
await expect(dialog).not.toBeVisible({ timeout: 10_000 });
const card = kanban.taskCardByTitle("PR Worktree Task");
await expect(card).toBeVisible({ timeout: 10_000 });
await card.click();
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
const session = new SessionPage(testPage);
await session.waitForLoad();
await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible({
timeout: 30_000,
});
await expect(session.idleInput()).toBeVisible({ timeout: 15_000 });
// Verify the worktree checked out the PR branch directly.
await expect(session.terminal).toBeVisible({ timeout: 15_000 });
await session.typeInTerminal("git branch --show-current");
await session.expectTerminalHasText("feature/pr-branch");
});
test("shows fetch warning banner when PR branch has no remote", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
test.setTimeout(90_000);
const { execSync } = await import("child_process");
const fs = await import("fs");
const gitEnv = {
...process.env,
HOME: backend.tmpDir,
GIT_AUTHOR_NAME: "E2E Test",
GIT_AUTHOR_EMAIL: "e2e@test.local",
GIT_COMMITTER_NAME: "E2E Test",
GIT_COMMITTER_EMAIL: "e2e@test.local",
};
// Create a repo with the PR branch locally but NO remote.
// This causes fetchBranchToLocal to fail the fetch and fall back to local,
// which produces a warning that should be displayed in the UI.
const repoDir = `${backend.tmpDir}/repos/e2e-warning-repo`;
fs.mkdirSync(repoDir, { recursive: true });
execSync("git init -b main", { cwd: repoDir, env: gitEnv });
execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv });
execSync("git checkout -b feature/warn-branch", { cwd: repoDir, env: gitEnv });
execSync('git commit --allow-empty -m "feature commit"', { cwd: repoDir, env: gitEnv });
execSync("git checkout main", { cwd: repoDir, env: gitEnv });
await apiClient.createRepository(seedData.workspaceId, repoDir, "main", {
name: "warn-owner/warn-repo",
provider: "github",
provider_owner: "warn-owner",
provider_name: "warn-repo",
});
await apiClient.mockGitHubAddBranches("warn-owner", "warn-repo", [
{ name: "main" },
{ name: "feature/warn-branch" },
]);
await apiClient.mockGitHubAddPRs([
{
number: 200,
title: "Warning PR",
state: "open",
head_branch: "feature/warn-branch",
base_branch: "main",
author_login: "test-user",
repo_owner: "warn-owner",
repo_name: "warn-repo",
},
]);
// Need worktree executor to trigger fetchBranchToLocal (local executor doesn't produce warnings)
const { executors } = await apiClient.listExecutors();
const worktreeExec = executors.find((e) => e.type === "worktree");
const worktreeProfile = worktreeExec?.profiles?.[0];
if (!worktreeProfile) {
test.skip(true, "No worktree executor profile available");
return;
}
const kanban = new KanbanPage(testPage);
await kanban.goto();
await kanban.createTaskButton.first().click();
const dialog = testPage.getByTestId("create-task-dialog");
await expect(dialog).toBeVisible();
// Enter PR URL
await testPage.getByTestId("toggle-github-url").click();
await testPage
.getByTestId("github-url-input")
.fill("https://github.com/warn-owner/warn-repo/pull/200");
await expect(dialog.getByText("feature/warn-branch")).toBeVisible({ timeout: 10_000 });
await testPage.getByTestId("task-title-input").fill("Warning Test Task");
await testPage.getByTestId("task-description-input").fill("/e2e:simple-message");
const startBtn = testPage.getByTestId("submit-start-agent");
await expect(startBtn).toBeEnabled({ timeout: 15_000 });
// Select worktree executor
const executorSelector = testPage.getByTestId("executor-profile-selector");
await executorSelector.click();
await testPage.getByRole("option", { name: /Worktree/i }).click();
await startBtn.click();
await expect(dialog).not.toBeVisible({ timeout: 10_000 });
const card = kanban.taskCardByTitle("Warning Test Task");
await expect(card).toBeVisible({ timeout: 10_000 });
await card.click();
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
const session = new SessionPage(testPage);
await session.waitForLoad();
// Wait for agent to complete (preparation is done by this point)
await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible({
timeout: 30_000,
});
// The warning banner should persist after preparation completes.
// It shows because fetchBranchToLocal failed to reach origin (no remote)
// and fell back to the local branch.
const warningBanner = testPage.getByTestId("prepare-warning-banner");
await expect(warningBanner).toBeVisible({ timeout: 10_000 });
await expect(warningBanner).toContainText("Could not fetch latest from origin");
// The "Details" toggle should be visible and expand raw git output on click.
const detailsBtn = warningBanner.getByRole("button", { name: "Details" });
await expect(detailsBtn).toBeVisible();
await detailsBtn.click();
// After expanding, raw git output should appear (e.g. "fatal:" from the failed fetch)
await expect(warningBanner.locator("pre")).toBeVisible();
// Clicking again should collapse the details.
await detailsBtn.click();
await expect(warningBanner.locator("pre")).not.toBeVisible();
// Dismissing the banner hides it entirely.
const dismissBtn = warningBanner.getByRole("button", { name: "Dismiss warning" });
await dismissBtn.click();
await expect(warningBanner).not.toBeVisible();
});
test("can toggle between GitHub URL and repository selector", async ({ testPage }) => {
const kanban = new KanbanPage(testPage);
await kanban.goto();
+3
View File
@@ -14,6 +14,9 @@ const eslintConfig = defineConfig([
"out/**",
"build/**",
"next-env.d.ts",
// Test artifacts (Playwright):
"**/test-results/**",
"**/playwright-report/**",
]),
{
plugins: {
@@ -147,6 +147,8 @@ export type PrepareStepInfo = {
status: string;
output?: string;
error?: string;
warning?: string;
warningDetail?: string;
};
export type SessionPrepareState = {
+10
View File
@@ -286,6 +286,8 @@ export type PrepareProgressPayload = {
status: string;
output?: string;
error?: string;
warning?: string;
warning_detail?: string;
timestamp: string;
};
@@ -297,6 +299,14 @@ export type PrepareCompletedPayload = {
error_message?: string;
duration_ms: number;
workspace_path?: string;
steps?: Array<{
name: string;
status: string;
output?: string;
error?: string;
warning?: string;
warning_detail?: string;
}>;
timestamp: string;
};
+33 -14
View File
@@ -29,22 +29,39 @@ export function registerExecutorPrepareHandlers(store: StoreApi<AppState>): WsHa
},
"executor.prepare.completed": (message) => {
const payload = message.payload as PrepareCompletedPayload;
store.setState((state) => ({
...state,
prepareProgress: {
...state.prepareProgress,
bySessionId: {
...state.prepareProgress.bySessionId,
[payload.session_id]: {
...state.prepareProgress.bySessionId[payload.session_id],
sessionId: payload.session_id,
status: payload.success ? "completed" : "failed",
errorMessage: payload.error_message,
durationMs: payload.duration_ms,
store.setState((state) => {
const existing = state.prepareProgress.bySessionId[payload.session_id];
// Use steps from the completed payload if available (ensures warnings
// are present even if progress events were missed due to late subscription).
const steps = payload.steps?.length
? payload.steps.map((s) => ({
name: s.name,
status: s.status,
output: s.output,
error: s.error,
warning: s.warning,
warningDetail: s.warning_detail,
}))
: existing?.steps;
return {
...state,
prepareProgress: {
...state.prepareProgress,
bySessionId: {
...state.prepareProgress.bySessionId,
[payload.session_id]: {
...existing,
sessionId: payload.session_id,
status: payload.success ? "completed" : "failed",
steps: steps ?? [],
errorMessage: payload.error_message,
durationMs: payload.duration_ms,
},
},
},
},
}));
};
});
},
};
}
@@ -63,6 +80,8 @@ function updateSteps(
status: payload.status,
output: payload.output,
error: payload.error,
warning: payload.warning,
warningDetail: payload.warning_detail,
};
return steps;
}