From 348ee25e6fc4be77601d01ed3210faf32b2b7fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 1 Apr 2026 22:55:02 +0100 Subject: [PATCH] feat: disable branch selector for local executor and implement base branch checkout (#515) --- .../agent/lifecycle/env_preparer_local.go | 18 ++- .../lifecycle/env_preparer_local_test.go | 91 +++++++++++ .../task-create-dialog-form-body.tsx | 8 + .../components/task-create-dialog-options.tsx | 7 + .../components/task-create-dialog-state.ts | 3 + .../components/task-create-dialog-types.ts | 1 + apps/web/components/task-create-dialog.tsx | 7 +- .../task/create-task-branch-selector.spec.ts | 151 ++++++++++++++++++ 8 files changed, 279 insertions(+), 7 deletions(-) create mode 100644 apps/web/e2e/tests/task/create-task-branch-selector.spec.ts diff --git a/apps/backend/internal/agent/lifecycle/env_preparer_local.go b/apps/backend/internal/agent/lifecycle/env_preparer_local.go index ad1db7ce23..8e43a76147 100644 --- a/apps/backend/internal/agent/lifecycle/env_preparer_local.go +++ b/apps/backend/internal/agent/lifecycle/env_preparer_local.go @@ -15,7 +15,7 @@ import ( ) // LocalPreparer prepares a local (non-worktree) execution environment. -// Steps: validate workspace → checkout PR branch (if set) → run setup script (if any). +// Steps: validate workspace → checkout branch (if set) → run setup script (if any). type LocalPreparer struct { logger *logger.Logger } @@ -39,8 +39,14 @@ func (p *LocalPreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, onP } resolvedScript := resolvePreparerSetupScript(req, workspacePath) + // Determine effective branch: CheckoutBranch (PR head) takes priority over BaseBranch. + effectiveBranch := req.CheckoutBranch + if effectiveBranch == "" { + effectiveBranch = req.BaseBranch + } + totalSteps := 1 // validate workspace - if req.CheckoutBranch != "" { + if effectiveBranch != "" { totalSteps++ } if resolvedScript != "" { @@ -62,13 +68,13 @@ func (p *LocalPreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, onP reportProgress(onProgress, step, stepIdx, totalSteps) stepIdx++ - // Step 2: Checkout PR branch (if specified) - if req.CheckoutBranch != "" { + // Step 2: Checkout branch (if specified) + if effectiveBranch != "" { step = beginStep("Checkout branch") reportProgress(onProgress, step, stepIdx, totalSteps) - output, err := checkoutBranch(ctx, workspacePath, req.CheckoutBranch) + output, err := checkoutBranch(ctx, workspacePath, effectiveBranch) if err != nil { - errMsg := fmt.Sprintf("failed to checkout branch %q: %s", req.CheckoutBranch, output) + errMsg := fmt.Sprintf("failed to checkout branch %q: %s", effectiveBranch, output) completeStepError(&step, errMsg) steps = append(steps, step) reportProgress(onProgress, step, stepIdx, totalSteps) diff --git a/apps/backend/internal/agent/lifecycle/env_preparer_local_test.go b/apps/backend/internal/agent/lifecycle/env_preparer_local_test.go index 312a4bf118..3e950ae0d5 100644 --- a/apps/backend/internal/agent/lifecycle/env_preparer_local_test.go +++ b/apps/backend/internal/agent/lifecycle/env_preparer_local_test.go @@ -341,3 +341,94 @@ func TestLocalPreparer_CheckoutWithDirtyWorkdir(t *testing.T) { t.Fatalf("expected branch 'main' after failed checkout, got %q", branch) } } + +func TestLocalPreparer_BaseBranchCheckout(t *testing.T) { + isolateGitEnv(t) + log := newTestLocalLogger() + preparer := NewLocalPreparer(log) + + repoDir := initGitRepo(t) + env := newIsolatedGitEnv() + // Create a feature branch + for _, args := range [][]string{ + {"checkout", "-b", "develop"}, + {"commit", "--allow-empty", "-m", "develop 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, + BaseBranch: "develop", + } + + 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) + } + // Branch should be develop + if branch := currentBranch(t, repoDir); branch != "develop" { + t.Fatalf("expected branch 'develop', got %q", branch) + } +} + +func TestLocalPreparer_CheckoutBranchPriorityOverBaseBranch(t *testing.T) { + isolateGitEnv(t) + log := newTestLocalLogger() + preparer := NewLocalPreparer(log) + + repoDir := initGitRepo(t) + env := newIsolatedGitEnv() + // Create two branches + for _, args := range [][]string{ + {"checkout", "-b", "develop"}, + {"commit", "--allow-empty", "-m", "develop commit"}, + {"checkout", "main"}, + {"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, + BaseBranch: "develop", + 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) + } + // CheckoutBranch should win over BaseBranch + if branch := currentBranch(t, repoDir); branch != "feature/pr-branch" { + t.Fatalf("expected branch 'feature/pr-branch' (CheckoutBranch priority), got %q", branch) + } +} diff --git a/apps/web/components/task-create-dialog-form-body.tsx b/apps/web/components/task-create-dialog-form-body.tsx index 53393b23a7..a37cd51848 100644 --- a/apps/web/components/task-create-dialog-form-body.tsx +++ b/apps/web/components/task-create-dialog-form-body.tsx @@ -59,6 +59,8 @@ type CreateEditSelectorsProps = { placeholder: string; triggerClassName?: string; }>; + isLocalExecutor: boolean; + useGitHubUrl: boolean; }; export const CreateEditSelectors = memo(function CreateEditSelectors({ @@ -79,19 +81,25 @@ export const CreateEditSelectors = memo(function CreateEditSelectors({ executorProfileId, onExecutorProfileChange, executorsLoading, + isLocalExecutor, + useGitHubUrl, BranchSelectorComponent, AgentSelectorComponent, ExecutorProfileSelectorComponent, }: CreateEditSelectorsProps) { if (isTaskStarted) return null; + const isLocalWithoutGitHubUrl = isLocalExecutor && !useGitHubUrl; + const branchPlaceholder = (() => { + if (isLocalWithoutGitHubUrl) return "Uses current branch"; if (!hasRepositorySelection) return "Select repository first"; if (branchesLoading || localBranchesLoading) return "Loading branches..."; return branchOptions.length > 0 ? "Select branch" : "No branches found"; })(); const branchDisabled = + isLocalWithoutGitHubUrl || !hasRepositorySelection || branchesLoading || localBranchesLoading || diff --git a/apps/web/components/task-create-dialog-options.tsx b/apps/web/components/task-create-dialog-options.tsx index 774e8676e8..d16bc4e120 100644 --- a/apps/web/components/task-create-dialog-options.tsx +++ b/apps/web/components/task-create-dialog-options.tsx @@ -161,6 +161,13 @@ export function useExecutorOptions(executors: Executor[]): OptionItem[] { }, [executors]); } +export function useIsLocalExecutor(executors: Executor[], executorId: string): boolean { + return useMemo(() => { + const selected = executors.find((e: Executor) => e.id === executorId); + return selected?.type === "local"; + }, [executors, executorId]); +} + export function useExecutorHint(executors: Executor[], executorId: string): string | null { return useMemo(() => { const selectedExecutor = executors.find((e: Executor) => e.id === executorId); diff --git a/apps/web/components/task-create-dialog-state.ts b/apps/web/components/task-create-dialog-state.ts index c1f037e1f1..3226fa6979 100644 --- a/apps/web/components/task-create-dialog-state.ts +++ b/apps/web/components/task-create-dialog-state.ts @@ -19,6 +19,7 @@ import { useAgentProfileOptions, useExecutorHint, useExecutorProfileOptions, + useIsLocalExecutor, } from "@/components/task-create-dialog-options"; import { setLocalStorage, @@ -551,6 +552,7 @@ export function useDialogComputed({ }, [executors]); const executorProfileOptions = useExecutorProfileOptions(allExecutorProfiles); const executorHint = useExecutorHint(executors, fs.executorId); + const isLocalExecutor = useIsLocalExecutor(executors, fs.executorId); const { headerRepositoryOptions } = useRepositoryOptions(repositories, fs.discoveredRepositories); const agentProfilesLoading = open && !settingsData.agentsLoaded; const executorsLoading = open && !settingsData.executorsLoaded; @@ -564,6 +566,7 @@ export function useDialogComputed({ agentProfileOptions, executorProfileOptions, executorHint, + isLocalExecutor, headerRepositoryOptions, agentProfilesLoading, executorsLoading, diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index 1081aa4291..04dbbbf235 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -48,6 +48,7 @@ export type DialogComputedValues = { agentProfileOptions: ReturnType; executorProfileOptions: ReturnType; executorHint: string | null; + isLocalExecutor: boolean; headerRepositoryOptions: ReturnType["headerRepositoryOptions"]; agentProfilesLoading: boolean; executorsLoading: boolean; diff --git a/apps/web/components/task-create-dialog.tsx b/apps/web/components/task-create-dialog.tsx index 3a8e9c148e..b4229f6101 100644 --- a/apps/web/components/task-create-dialog.tsx +++ b/apps/web/components/task-create-dialog.tsx @@ -259,6 +259,7 @@ type DialogFormBodyProps = { onExecutorProfileChange: (v: string) => void; onWorkflowChange: (v: string) => void; hasRepositorySelection: boolean; + isLocalExecutor: boolean; }; function DialogFormBody({ @@ -286,6 +287,7 @@ function DialogFormBody({ onExecutorProfileChange, onWorkflowChange, hasRepositorySelection, + isLocalExecutor, }: DialogFormBodyProps) { return (
@@ -327,6 +329,8 @@ function DialogFormBody({ executorProfileId={fs.executorProfileId} onExecutorProfileChange={onExecutorProfileChange} executorsLoading={executorsLoading} + isLocalExecutor={isLocalExecutor} + useGitHubUrl={fs.useGitHubUrl} BranchSelectorComponent={BranchSelector} AgentSelectorComponent={AgentSelector} ExecutorProfileSelectorComponent={ExecutorProfileSelector} @@ -414,7 +418,7 @@ function useTaskCreateDialogSetup(props: TaskCreateDialogProps) { useGitHubUrl: fs.useGitHubUrl, githubUrl: fs.githubUrl, githubPrHeadBranch: fs.githubPrHeadBranch, - branch: fs.branch, + branch: computed.isLocalExecutor && !fs.useGitHubUrl ? "" : fs.branch, agentProfileId: fs.agentProfileId, executorId: fs.executorId, executorProfileId: fs.executorProfileId, @@ -523,6 +527,7 @@ export function TaskCreateDialog(props: TaskCreateDialogProps) { onExecutorProfileChange={handlers.handleExecutorProfileChange} onWorkflowChange={handlers.handleWorkflowChange} hasRepositorySelection={computed.hasRepositorySelection} + isLocalExecutor={computed.isLocalExecutor} /> { + test.describe.configure({ retries: 1 }); + + test("branch selector is disabled for local executor with local repo", async ({ + testPage, + apiClient, + }) => { + // Find the system "local" executor and create a profile on it + const { executors } = await apiClient.listExecutors(); + const localExec = executors.find((e) => e.type === "local"); + if (!localExec) { + test.skip(true, "No local executor available"); + return; + } + await apiClient.createExecutorProfile(localExec.id, "E2E Local Profile"); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + await kanban.createTaskButton.first().click(); + const dialog = testPage.getByTestId("create-task-dialog"); + await expect(dialog).toBeVisible(); + + // Fill title + description + await testPage.getByTestId("task-title-input").fill("Branch Selector Test"); + await testPage.getByTestId("task-description-input").fill("testing branch selector disable"); + + // Select the local executor profile + const executorSelector = testPage.getByTestId("executor-profile-selector"); + await executorSelector.click(); + await testPage.getByRole("option", { name: /E2E Local Profile/i }).click(); + + // Branch selector should be disabled when local executor is selected + const branchSelector = testPage.getByTestId("branch-selector"); + await expect(branchSelector).toBeDisabled({ timeout: 5_000 }); + }); + + test("branch selector re-enables when switching from local to worktree executor", async ({ + testPage, + apiClient, + }) => { + // Find executors + const { executors } = await apiClient.listExecutors(); + const localExec = executors.find((e) => e.type === "local"); + const worktreeExec = executors.find((e) => e.type === "worktree"); + if (!localExec || !worktreeExec) { + test.skip(true, "Need both local and worktree executors"); + return; + } + await apiClient.createExecutorProfile(localExec.id, "E2E Local"); + const worktreeProfile = worktreeExec.profiles?.[0]; + if (!worktreeProfile) { + test.skip(true, "No worktree 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(); + + await testPage.getByTestId("task-title-input").fill("Switch Executor Test"); + await testPage.getByTestId("task-description-input").fill("testing executor switch"); + + // Select local executor -> branch should be disabled + const executorSelector = testPage.getByTestId("executor-profile-selector"); + await executorSelector.click(); + await testPage.getByRole("option", { name: /E2E Local/i }).click(); + + const branchSelector = testPage.getByTestId("branch-selector"); + await expect(branchSelector).toBeDisabled({ timeout: 5_000 }); + + // Switch to worktree executor -> branch should be enabled + await executorSelector.click(); + await testPage.getByRole("option", { name: worktreeProfile.name }).click(); + + await expect(branchSelector).toBeEnabled({ timeout: 5_000 }); + }); + + test("branch selector stays enabled for local executor with GitHub URL", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const fs = await import("fs"); + 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", + }; + + // Find local executor and create profile + const { executors } = await apiClient.listExecutors(); + const localExec = executors.find((e) => e.type === "local"); + if (!localExec) { + test.skip(true, "No local executor available"); + return; + } + await apiClient.createExecutorProfile(localExec.id, "E2E Local GitHub URL"); + + // Create a unique repo for this test + const repoDir = `${backend.tmpDir}/repos/e2e-branch-gh`; + 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 }); + + // Seed mock GitHub branches + await apiClient.createRepository(seedData.workspaceId, repoDir, "main", { + name: "branch-test-owner/branch-test-repo", + provider: "github", + provider_owner: "branch-test-owner", + provider_name: "branch-test-repo", + }); + await apiClient.mockGitHubAddBranches("branch-test-owner", "branch-test-repo", [ + { name: "main" }, + { name: "develop" }, + ]); + + 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 + await testPage.getByTestId("toggle-github-url").click(); + await testPage + .getByTestId("github-url-input") + .fill("https://github.com/branch-test-owner/branch-test-repo"); + + // Select local executor profile + const executorSelector = testPage.getByTestId("executor-profile-selector"); + await executorSelector.click(); + await testPage.getByRole("option", { name: /E2E Local GitHub URL/i }).click(); + + // Branch selector should NOT be disabled (GitHub URL mode overrides) + const branchSelector = testPage.getByTestId("branch-selector"); + await expect(branchSelector).toBeEnabled({ timeout: 10_000 }); + }); +});