feat: disable branch selector for local executor and implement base branch checkout (#515)

This commit is contained in:
Carlos Florêncio
2026-04-01 22:55:02 +01:00
committed by GitHub
parent d7b8fdce60
commit 348ee25e6f
8 changed files with 279 additions and 7 deletions
@@ -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)
@@ -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)
}
}
@@ -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 ||
@@ -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);
@@ -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,
@@ -48,6 +48,7 @@ export type DialogComputedValues = {
agentProfileOptions: ReturnType<typeof useAgentProfileOptions>;
executorProfileOptions: ReturnType<typeof useExecutorProfileOptions>;
executorHint: string | null;
isLocalExecutor: boolean;
headerRepositoryOptions: ReturnType<typeof useRepositoryOptions>["headerRepositoryOptions"];
agentProfilesLoading: boolean;
executorsLoading: boolean;
+6 -1
View File
@@ -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 (
<div className="flex-1 space-y-4 overflow-y-auto pr-1">
@@ -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}
/>
<DialogFooter className="border-t border-border pt-3 flex-col gap-3 sm:flex-row sm:gap-2">
<TaskCreateDialogFooter
@@ -0,0 +1,151 @@
import { test, expect } from "../../fixtures/test-base";
import { KanbanPage } from "../../pages/kanban-page";
test.describe("Branch selector behavior with executor types", () => {
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 });
});
});