feat: disable branch selector for local executor and implement base branch checkout (#515)
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user