fix(web): pre-fill PR head branch when launching task from GitHub PR list (#960)
This commit is contained in:
@@ -0,0 +1,60 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import type { GitHubPR, GitHubPRStatus } from "@/lib/types/github";
|
||||
import { pickPRForLaunch } from "./pr-list";
|
||||
|
||||
function makePR(overrides?: Partial<GitHubPR>): GitHubPR {
|
||||
return {
|
||||
number: 42,
|
||||
title: "Test PR",
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
html_url: "https://github.com/owner/repo/pull/42",
|
||||
state: "open",
|
||||
head_branch: "",
|
||||
base_branch: "",
|
||||
author_login: "author",
|
||||
repo_owner: "owner",
|
||||
repo_name: "repo",
|
||||
draft: false,
|
||||
mergeable: true,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
requested_reviewers: [],
|
||||
created_at: "2024-01-01T00:00:00Z",
|
||||
updated_at: "2024-01-01T00:00:00Z",
|
||||
merged_at: null,
|
||||
closed_at: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeStatus(pr: GitHubPR): GitHubPRStatus {
|
||||
return {
|
||||
pr,
|
||||
review_state: "",
|
||||
checks_state: "",
|
||||
mergeable_state: "unknown",
|
||||
review_count: 0,
|
||||
pending_review_count: 0,
|
||||
checks_total: 0,
|
||||
checks_passing: 0,
|
||||
};
|
||||
}
|
||||
|
||||
describe("pickPRForLaunch", () => {
|
||||
it("prefers the enriched PR from the batched status (which carries head/base branches)", () => {
|
||||
const rawSearchPR = makePR({ head_branch: "", base_branch: "" });
|
||||
const enrichedPR = makePR({ head_branch: "feature/x", base_branch: "main" });
|
||||
|
||||
const result = pickPRForLaunch(rawSearchPR, makeStatus(enrichedPR));
|
||||
|
||||
expect(result.head_branch).toBe("feature/x");
|
||||
expect(result.base_branch).toBe("main");
|
||||
});
|
||||
|
||||
it("falls back to the raw PR when no status is loaded yet", () => {
|
||||
const rawPR = makePR({ head_branch: "feature/raw", base_branch: "main" });
|
||||
|
||||
expect(pickPRForLaunch(rawPR, undefined).head_branch).toBe("feature/raw");
|
||||
expect(pickPRForLaunch(rawPR, null).head_branch).toBe("feature/raw");
|
||||
});
|
||||
});
|
||||
@@ -31,6 +31,13 @@ type PRListProps = {
|
||||
onStartTask: (payload: LaunchPayload) => void;
|
||||
};
|
||||
|
||||
// Prefer the enriched PR returned by the batched status endpoint — the search
|
||||
// API used to populate `items` does not include head/base branches, so the
|
||||
// launcher needs the enriched copy to pre-fill the task dialog correctly.
|
||||
export function pickPRForLaunch(pr: GitHubPR, status: GitHubPRStatus | null | undefined): GitHubPR {
|
||||
return status?.pr ?? pr;
|
||||
}
|
||||
|
||||
function prStateIcon(pr: GitHubPR): { Icon: Icon; className: string } {
|
||||
if (pr.state === "merged")
|
||||
return { Icon: IconGitMerge, className: "text-purple-600 dark:text-purple-400" };
|
||||
@@ -53,7 +60,12 @@ function StartTaskMenu({
|
||||
return (
|
||||
<DropdownMenu>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button size="sm" variant="outline" className="h-7 gap-1 cursor-pointer">
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
className="h-7 gap-1 cursor-pointer"
|
||||
data-testid="pr-start-task-trigger"
|
||||
>
|
||||
<IconPlus className="h-3.5 w-3.5" />
|
||||
Task
|
||||
<IconChevronDown className="h-3 w-3" />
|
||||
@@ -67,6 +79,8 @@ function StartTaskMenu({
|
||||
key={p.id}
|
||||
className="cursor-pointer gap-2 py-1.5"
|
||||
onSelect={() => launch(p)}
|
||||
data-testid="pr-start-task-preset"
|
||||
data-preset-id={p.id}
|
||||
>
|
||||
<ItemIcon className="h-3.5 w-3.5 text-muted-foreground shrink-0" />
|
||||
<div className="flex flex-col min-w-0">
|
||||
@@ -94,7 +108,11 @@ function PRRow({
|
||||
}) {
|
||||
const { Icon: StateIcon, className: stateIconClass } = prStateIcon(pr);
|
||||
return (
|
||||
<div className="flex items-start gap-3 px-4 py-3 hover:bg-muted/40 transition-colors">
|
||||
<div
|
||||
className="flex items-start gap-3 px-4 py-3 hover:bg-muted/40 transition-colors"
|
||||
data-testid="pr-row"
|
||||
data-pr-number={pr.number}
|
||||
>
|
||||
<StateIcon className={cn("h-4 w-4 mt-1 shrink-0", stateIconClass)} />
|
||||
<div className="min-w-0 flex-1">
|
||||
<Link
|
||||
@@ -117,7 +135,11 @@ function PRRow({
|
||||
</div>
|
||||
</div>
|
||||
<div className="shrink-0">
|
||||
<StartTaskMenu pr={pr} presets={presets} onStartTask={onStartTask} />
|
||||
<StartTaskMenu
|
||||
pr={pickPRForLaunch(pr, status)}
|
||||
presets={presets}
|
||||
onStartTask={onStartTask}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -36,6 +36,10 @@ function matchRepo(repos: Repository[], owner: string, name: string): Repository
|
||||
);
|
||||
}
|
||||
|
||||
function emptyToUndefined(value: string | undefined): string | undefined {
|
||||
return value ? value : undefined;
|
||||
}
|
||||
|
||||
function extractPayload(payload: LaunchPayload) {
|
||||
if (payload.kind === "pr") {
|
||||
return {
|
||||
@@ -43,7 +47,7 @@ function extractPayload(payload: LaunchPayload) {
|
||||
title: payload.pr.title,
|
||||
owner: payload.pr.repo_owner,
|
||||
name: payload.pr.repo_name,
|
||||
branch: payload.pr.head_branch,
|
||||
branch: emptyToUndefined(payload.pr.head_branch),
|
||||
};
|
||||
}
|
||||
return {
|
||||
@@ -60,6 +64,10 @@ function buildDialogState(payload: LaunchPayload, repositories: Repository[]): D
|
||||
const repo = matchRepo(repositories, data.owner, data.name);
|
||||
const description = payload.preset.prompt({ url: data.url, title: data.title });
|
||||
const title = `${payload.preset.label}: ${data.title}`;
|
||||
// For a PR launch we want the dialog to display and check out the PR's head
|
||||
// branch — matching the GitHub-URL-paste flow, where the branch selector
|
||||
// auto-resolves to the PR head. Same branch for both: the chip shows it and
|
||||
// the worktree checks it out.
|
||||
const checkoutBranch = payload.kind === "pr" ? data.branch : undefined;
|
||||
if (repo) {
|
||||
return {
|
||||
|
||||
@@ -99,11 +99,21 @@ export function useRepositoryAutoSelectEffect(
|
||||
} else if (repositories.length === 1) {
|
||||
pickId = repositories[0].id;
|
||||
}
|
||||
void Promise.resolve().then(() =>
|
||||
setRepositories([
|
||||
pickId ? { key: "row-0", repositoryId: pickId, branch: "" } : { key: "row-0", branch: "" },
|
||||
]),
|
||||
);
|
||||
// Use the functional setter so the deferred microtask sees fresh state.
|
||||
// Without this, a sibling effect (resetTaskForm / useLockedFieldSync) that
|
||||
// seeds rows from `initialValues.repositoryId` synchronously races with
|
||||
// this microtask — the microtask captured rows.length === 0 at queue time
|
||||
// and would blindly clobber the initialValues-seeded row.
|
||||
void Promise.resolve().then(() => {
|
||||
setRepositories((prev) => {
|
||||
if (prev.length > 0) return prev;
|
||||
return [
|
||||
pickId
|
||||
? { key: "row-0", repositoryId: pickId, branch: "" }
|
||||
: { key: "row-0", branch: "" },
|
||||
];
|
||||
});
|
||||
});
|
||||
}, [open, repositories, rows, useGitHubUrl, workspaceId, setRepositories]);
|
||||
}
|
||||
|
||||
@@ -490,7 +500,7 @@ export function useTaskCreateDialogEffects(fs: DialogFormState, args: TaskCreate
|
||||
useDiscoverReposEffect(fs, open, workspaceId, repositoriesLoading, toast);
|
||||
useBranchAutoSelectEffect(fs);
|
||||
useCurrentLocalBranchEffect(fs, open, workspaceId, repositories);
|
||||
useResetBranchOnLocalSwitchEffect(fs, isLocalExecutor);
|
||||
useResetBranchOnLocalSwitchEffect(fs, isLocalExecutor, args.preserveBranch);
|
||||
useDefaultSelectionsEffect(fs, open, { agentProfiles, executors, workspaceDefaults }, workflows);
|
||||
useGitHubUrlBranchesEffect(fs, open);
|
||||
}
|
||||
@@ -504,7 +514,11 @@ export function useTaskCreateDialogEffects(fs: DialogFormState, args: TaskCreate
|
||||
// tree. With the reset, switching to local always defaults to "current
|
||||
// branch on disk" and the user has to opt back into a different branch
|
||||
// explicitly.
|
||||
function useResetBranchOnLocalSwitchEffect(fs: DialogFormState, isLocalExecutor: boolean) {
|
||||
function useResetBranchOnLocalSwitchEffect(
|
||||
fs: DialogFormState,
|
||||
isLocalExecutor: boolean,
|
||||
preserveBranch: string | undefined,
|
||||
) {
|
||||
const { repositories: rows, updateRepository } = fs;
|
||||
const wasLocalRef = useRef(isLocalExecutor);
|
||||
useEffect(() => {
|
||||
@@ -512,7 +526,16 @@ function useResetBranchOnLocalSwitchEffect(fs: DialogFormState, isLocalExecutor:
|
||||
wasLocalRef.current = isLocalExecutor;
|
||||
if (!isLocalExecutor || prev) return; // only fire on false → true transition
|
||||
for (const row of rows) {
|
||||
if (row.branch) updateRepository(row.key, { branch: "" });
|
||||
// Preserve a branch the caller asked us to keep (e.g. the PR head branch
|
||||
// when launching from a GitHub PR). Without this, the executor's async
|
||||
// settle on dialog open looks like a worktree→local switch and clobbers
|
||||
// the explicit branch choice, leaving the chip showing "current: main".
|
||||
// Both `row.branch` and `preserveBranch` are bare branch names with no
|
||||
// remote prefix — current callers (`initialValues.checkoutBranch` /
|
||||
// `initialValues.branch`) never pass `origin/...` here.
|
||||
if (row.branch && row.branch !== preserveBranch) {
|
||||
updateRepository(row.key, { branch: "" });
|
||||
}
|
||||
}
|
||||
}, [isLocalExecutor, rows, updateRepository]);
|
||||
}, [isLocalExecutor, rows, updateRepository, preserveBranch]);
|
||||
}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type React from "react";
|
||||
import type { LocalRepository, Repository, Executor, Branch, Task } from "@/lib/types/http";
|
||||
import type { AgentProfileOption, WorkspaceState } from "@/lib/state/slices";
|
||||
import type {
|
||||
@@ -128,6 +129,13 @@ export type TaskCreateEffectsArgs = {
|
||||
* worktree run that would trigger a destructive `git checkout` on submit.
|
||||
*/
|
||||
isLocalExecutor: boolean;
|
||||
/**
|
||||
* Branch the caller wants preserved across the local-switch reset (e.g. the
|
||||
* PR head branch when launching from a GitHub PR). The reset effect skips
|
||||
* rows whose branch matches this value so the explicit caller choice isn't
|
||||
* clobbered by the executor's async settle on mount.
|
||||
*/
|
||||
preserveBranch?: string;
|
||||
};
|
||||
|
||||
import type { FileAttachment } from "@/components/task/chat/file-attachment";
|
||||
@@ -158,7 +166,7 @@ export type DialogFormState = {
|
||||
* order is the position the backend sees. There is no "primary" concept.
|
||||
*/
|
||||
repositories: TaskRepoRow[];
|
||||
setRepositories: (v: TaskRepoRow[]) => void;
|
||||
setRepositories: React.Dispatch<React.SetStateAction<TaskRepoRow[]>>;
|
||||
addRepository: () => void;
|
||||
removeRepository: (key: string) => void;
|
||||
updateRepository: (key: string, patch: Partial<TaskRepoRow>) => void;
|
||||
@@ -265,7 +273,7 @@ export type SubmitHandlersDeps = {
|
||||
setHasTitle: (v: boolean) => void;
|
||||
setHasDescription: (v: boolean) => void;
|
||||
setTaskName: (v: string) => void;
|
||||
setRepositories: (v: TaskRepoRow[]) => void;
|
||||
setRepositories: React.Dispatch<React.SetStateAction<TaskRepoRow[]>>;
|
||||
setGitHubBranch: (v: string) => void;
|
||||
setAgentProfileId: (v: string) => void;
|
||||
setExecutorId: (v: string) => void;
|
||||
|
||||
@@ -408,6 +408,7 @@ export function useTaskCreateDialogSetup(props: TaskCreateDialogProps) {
|
||||
toast,
|
||||
workflows,
|
||||
isLocalExecutor: computed.isLocalExecutor,
|
||||
preserveBranch: initialValues?.checkoutBranch || initialValues?.branch,
|
||||
});
|
||||
useLockedFieldSync(open, workflowId, initialValues, fs);
|
||||
const handlers = useDialogHandlers(fs, repositories);
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import { execSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { makeGitEnv } from "../../helpers/git-helper";
|
||||
|
||||
test.describe("Create task from GitHub PR list", () => {
|
||||
// Cold-start backend boots can race on the first test in a worker.
|
||||
test.describe.configure({ retries: 1 });
|
||||
|
||||
test("starting a task from a PR pre-fills the matching repo and the PR head branch", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
backend,
|
||||
}) => {
|
||||
test.setTimeout(60_000);
|
||||
|
||||
// seedData is worker-scoped, so the workspace can accumulate repos across
|
||||
// test reruns. Use a unique owner/repo per invocation so matchRepo lands
|
||||
// on exactly the one we just created.
|
||||
const suffix = `${process.pid}-${Date.now()}`;
|
||||
const ownerSlug = `owner-${suffix}`;
|
||||
const repoSlug = `repo-${suffix}`;
|
||||
|
||||
// Create a real local git repo so the branch chip can enumerate branches.
|
||||
// Both `main` and the PR head branch must exist locally — the chip lists
|
||||
// local branches, not the GitHub mock's branches.
|
||||
const repoDir = `${backend.tmpDir}/repos/pr-list-${suffix}`;
|
||||
fs.mkdirSync(repoDir, { recursive: true });
|
||||
const gitEnv = makeGitEnv(backend.tmpDir);
|
||||
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/from-pr-list", { cwd: repoDir, env: gitEnv });
|
||||
execSync('git commit --allow-empty -m "feature"', { cwd: repoDir, env: gitEnv });
|
||||
execSync("git checkout main", { cwd: repoDir, env: gitEnv });
|
||||
|
||||
// Pre-seed a GitHub-backed repository that matches the mock PR's owner/repo.
|
||||
// The launcher's matchRepo() looks up workspace repos by provider_owner/name.
|
||||
await apiClient.createRepository(seedData.workspaceId, repoDir, "main", {
|
||||
name: `${ownerSlug}/${repoSlug}`,
|
||||
provider: "github",
|
||||
provider_owner: ownerSlug,
|
||||
provider_name: repoSlug,
|
||||
});
|
||||
|
||||
await apiClient.mockGitHubReset();
|
||||
await apiClient.mockGitHubSetUser("test-user");
|
||||
await apiClient.mockGitHubAddBranches(ownerSlug, repoSlug, [
|
||||
{ name: "main" },
|
||||
{ name: "feature/from-pr-list" },
|
||||
]);
|
||||
await apiClient.mockGitHubAddPRs([
|
||||
{
|
||||
number: 77,
|
||||
title: "PR from GitHub page",
|
||||
state: "open",
|
||||
head_branch: "feature/from-pr-list",
|
||||
base_branch: "main",
|
||||
author_login: "test-user",
|
||||
repo_owner: ownerSlug,
|
||||
repo_name: repoSlug,
|
||||
},
|
||||
]);
|
||||
|
||||
await testPage.goto("/github");
|
||||
|
||||
// Wait for the seeded PR row to render. The "Review requested" default
|
||||
// preset matches because the mock user is the requested reviewer fallback
|
||||
// for the seeded PR. We scope by data-pr-number to avoid catching any
|
||||
// leftover PRs cached by previous tests.
|
||||
const prRow = testPage.locator('[data-testid="pr-row"][data-pr-number="77"]');
|
||||
await expect(prRow).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Open the task launcher menu for this PR and choose the "Review" preset.
|
||||
await prRow.getByTestId("pr-start-task-trigger").click();
|
||||
await testPage.locator('[data-testid="pr-start-task-preset"][data-preset-id="review"]').click();
|
||||
|
||||
// The create-task dialog should open with the matching workspace repo
|
||||
// selected and the PR head branch pre-filled in the branch chip.
|
||||
const dialog = testPage.getByTestId("create-task-dialog");
|
||||
await expect(dialog).toBeVisible();
|
||||
await expect(dialog.getByTestId("repo-chip-trigger").first()).toContainText(
|
||||
`${ownerSlug}/${repoSlug}`,
|
||||
);
|
||||
await expect(dialog.getByTestId("branch-chip-trigger").first()).toContainText(
|
||||
"feature/from-pr-list",
|
||||
{ timeout: 10_000 },
|
||||
);
|
||||
|
||||
// The dialog title should be derived from the preset + PR title.
|
||||
await expect(testPage.getByTestId("task-title-input")).toHaveValue(
|
||||
/Review: PR from GitHub page/,
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user