diff --git a/apps/desktop/plans/20260416-v2-pr-checkout-endpoint.md b/apps/desktop/plans/20260416-v2-pr-checkout-endpoint.md new file mode 100644 index 00000000000..4f0251d13ef --- /dev/null +++ b/apps/desktop/plans/20260416-v2-pr-checkout-endpoint.md @@ -0,0 +1,396 @@ +# V2 PR Checkout + +Extend v2's `workspaceCreation.checkout` procedure to materialize a PR's branch +(via `gh pr checkout`) when the modal carries a `linkedPR`. Not a new endpoint +— `checkout` already means "materialize an externally-defined branch into a +worktree"; a PR branch is just another form of that. The client's +`pr-checkout` intent differentiates progress labels + payload construction, +but routes to the same tRPC mutation. + +Reuses the `getGitHubPullRequestContent` fetch that already happens at launch +time — moved earlier in the pending-page sequence and shared between the +mutation payload and the agent-launch resolver. **Zero net new fetches.** + +Cross-refs: +- `apps/desktop/V2_WORKSPACE_CREATION.md` — umbrella design this extends. +- `packages/host-service/GIT_REFS.md` — ref handling discipline. +- V1 source: `apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts:752` (`createFromPr`) + `.../utils/git.ts:1630-1791`. + +## Problem + +V2's `NewWorkspaceModal` accepts a `linkedPR` in its draft, and the UI already +signals the intent switch — when a PR is attached, the branch picker is +replaced with "based off PR #N" (`PromptGroup.tsx:365-376`). But submit +currently routes through the `fork` intent, which creates a new branch off +`baseBranch`. The PR is passed only as prompt context to the agent +(`buildForkAgentLaunch.ts:354`). Result: the workspace has no PR commits, +`git diff` shows nothing meaningful, and the user has to manually `gh pr +checkout` after the fact. + +V2's existing `checkout` procedure almost covers this case but not quite: +- Resolves branches via `origin/` — fork PRs live at + `refs/pull/N/head` and fail `resolveRef`. +- No fork-owner-prefix branch naming (`/` to avoid + collisions with local branches of the same name). +- No PR metadata awareness (base branch, state, cross-repo flag). + +The fix is a narrow expansion of `checkout`, not a new endpoint. + +## V1 pain points we're fixing + +1. **Server re-parses the PR URL** (`parsePrUrl` → `gh pr view`) even though + the picker already has structured data. +2. **`gh pr view` runs twice** — once at attach time, once at checkout time. +3. **`gh pr checkout --force` silently overwrites** any local branch with the + same name. V1's "existing worktree" check fires after the git op, not + before. +4. **Fire-and-forget** — no pending-row, no retry, no progress steps. +5. **Host-local only** — v1 writes to `worktrees` + `workspaces` tables, no + cloud `v2Workspace.create`, no `ensureV2Host`. +6. **Silent on closed/merged PRs** — worktree still created, user has to + notice. +7. **Untyped branch-name derivation** — inline string munging in `git.ts:1630`, + no unit tests. + +## Scope + +In: `checkout` widening, `getGitHubPullRequestContent` response widening, +pending-page fetch-before-mutate wiring, `buildForkAgentLaunch` resolved-PR +pass-through, tests. + +Out: picker-initiated PR checkout (no entry path today), PR comments, re-adopt +after cloud delete (existing `hasWorkspace` safety net covers this). + +--- + +## 1. Server + +### 1a. Widen `getGitHubPullRequestContent` + +File: `packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts` (line 1377) + +Currently returns `{number, title, body, url, state, branch, baseBranch, author, isDraft, createdAt, updatedAt}`. Missing for our use: `headRepositoryOwner`, `isCrossRepository`. + +`gh pr view --json` already returns both natively (v1 pulls them at +`git.ts:1704`). Three small edits: + +- Append `,headRepositoryOwner,isCrossRepository` to the `--json` flag + list (line 1394). +- Extend `PrSchema` zod (line 1430) with those fields. +- Expose them in the return mapping (line 1396-1409). + +Not a new fetch — same `gh pr view` call, two more fields surfaced. + +### 1b. Widen `checkout` + +File: same file, line 811. Two input changes: + +1. Add optional `pr` for the PR-path. +2. Add `composer.baseBranch` — matches `create`'s composer shape. Used by + the shared postlude to write `branch..base` for the Changes tab. + Populated client-side per mode (picker selection for branch path, + `pr.baseRefName` for PR path). + +Exactly one of `branch` or `pr` must be set — enforced at the zod layer: + +```ts +checkout: protectedProcedure + .input(z.object({ + pendingId: z.string(), + projectId: z.string(), + workspaceName: z.string(), + + branch: z.string().optional(), + pr: z.object({ + number: z.number().int().positive(), + url: z.string().url(), + title: z.string(), + headRefName: z.string(), + baseRefName: z.string(), + headRepositoryOwner: z.string(), + isCrossRepository: z.boolean(), + state: z.enum(["open", "closed", "merged", "draft"]), + }).optional(), + + composer: z.object({ + prompt: z.string().optional(), + baseBranch: z.string().optional(), // ← new; shared across branch + PR paths + runSetupScript: z.boolean().optional(), + }), + linkedContext: /* unchanged */, + }).refine( + (v) => (!!v.branch) !== (!!v.pr), + "exactly one of `branch` or `pr` must be set", + )) +``` + +### PR-path middle section + +```ts +if (input.pr) { + const branch = derivePrLocalBranchName(input.pr); + + // Idempotency: existing workspace for this branch → "open existing". + // Not an error — renderer navigates to it as if a create succeeded. + const existing = ctx.db.query.workspaces.findFirst({ + where: and(eq(workspaces.projectId, input.projectId), eq(workspaces.branch, branch)), + }).sync(); + if (existing) { + clearProgress(input.pendingId); + return { workspace: existing, terminals: [], warnings: [], alreadyExists: true }; + } + + const worktreePath = safeResolveWorktreePath(localProject.repoPath, branch); + const git = await ctx.git(localProject.repoPath); + + // Detached worktree → `gh pr checkout` inside creates the branch with + // correct fork-remote + upstream config. Matches v1's `createWorktreeFromPr`. + await git.raw(["worktree", "add", "--detach", worktreePath]); + try { + await execGh( + ["pr", "checkout", String(input.pr.number), "--branch", branch, "--force"], + { cwd: worktreePath, timeout: 120_000 }, + ); + } catch (err) { + await git.raw(["worktree", "remove", "--force", worktreePath]).catch(() => {}); + clearProgress(input.pendingId); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `gh pr checkout failed: ${errMsg(err)}`, + }); + } + + await git.raw(["-C", worktreePath, "config", "--local", "push.autoSetupRemote", "true"]).catch(warn); + // NOTE: `branch..base` write lives in `finishCheckout` and reads + // from `composer.baseBranch` — client passes `pr.baseRefName` for PR + // mode. No intent-specific config write here. See §3. + + return await finishCheckout(ctx, { + pendingId: input.pendingId, + projectId: input.projectId, + workspaceName: input.workspaceName, + branch, + worktreePath, + runSetup: input.composer.runSetupScript ?? false, + rollbackGit: git, + extraWarnings: input.pr.state !== "open" + ? [`PR is ${input.pr.state} — commits are included, but the PR may not merge.`] + : [], + }); +} + +// ...existing branch-path body, refactored to also call finishCheckout() +``` + +`finishCheckout` is a local helper in the same file wrapping: + +- `branch..base` config write (if `composer.baseBranch` set) +- `ensureV2Host` + `v2Workspace.create` (with rollback) +- local `workspaces` insert +- setup terminal +- `clearProgress` + +Called from both branches. Skips a full pipeline-extraction — two callers in +one file is a local helper, not a module. The existing branch-path in +`checkout` (non-PR) also routes through `finishCheckout`, which means +regular picker-driven checkouts start writing `branch..base` too — +fixes a current gap where only `create` records the base. + +### 1c. `derivePrLocalBranchName` + +New file: `packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-name.ts` + +```ts +export function derivePrLocalBranchName(pr: { + headRefName: string; + headRepositoryOwner: string; + isCrossRepository: boolean; +}): string { + if (pr.isCrossRepository) { + const owner = pr.headRepositoryOwner.toLowerCase(); + return `${owner}/${pr.headRefName}`; + } + return pr.headRefName; +} +``` + +Unit tests: same-repo passthrough, cross-repo prefix, owner case-folding, +cross-repo with slash-containing head refs, empty-field rejection. Pure +function — importable from the renderer too. + +## 2. Renderer + +### 2a. Pending-row schema + +File: `apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts` + +Only change: add `"pr-checkout"` to the `intent` enum. **`linkedPR` stays +narrow** (`{prNumber, title, url, state}`) — the enriched fields don't need to +persist in the pending row; they're fetched on-page via `useQuery`. + +### 2b. Submit dispatch + +File: `.../PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts` + +```ts +collections.pendingWorkspaces.insert({ + id: pendingId, + projectId, + intent: draft.linkedPR ? "pr-checkout" : "fork", + // ...rest unchanged +}); +``` + +### 2c. Pending-page dispatch + +File: `apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx` + +New case in `useFireIntent`: + +```ts +case "pr-checkout": { + // Fetch PR content before firing. Stable query key — react-query dedupes. + const { data: prContent, error } = useQuery({ + queryKey: ["workspaceCreation.getGitHubPullRequestContent", pending.projectId, pending.linkedPR.prNumber], + queryFn: () => hostServiceClient.workspaceCreation.getGitHubPullRequestContent.query({ + projectId: pending.projectId, + prNumber: pending.linkedPR!.prNumber, + }), + }); + if (error) { /* pending row error state, user can retry */ return; } + if (!prContent) { /* still loading — progress label: "Resolving PR..." */ return; } + + const result = await checkoutWorkspace(buildPrCheckoutPayload(pending, prContent)); + // ...agent-launch builder receives prContent via resolvedPr — no re-fetch +} +``` + +Progress labels at the UI layer differ per intent (`"Resolving PR..."`, +`"Checking out PR #123..."`). Server progress step names stay generic. + +### 2d. `buildForkAgentLaunch` — accept resolved PR + +File: `apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.ts` + +Add optional `resolvedPr` to `BuildForkAgentLaunchInputs`. When provided, +`fetchPullRequest` resolver (line 436) returns it directly instead of calling +`client.workspaceCreation.getGitHubPullRequestContent.query`. For `fork` +intent (no prefetch), the existing fetch-on-demand path is unchanged. + +The shape `prContent` returns already has `branch` (= `headRefName`) and +`body` — exactly what the resolver needs. + +### 2e. `buildPrCheckoutPayload` + +New file or addition alongside existing `buildForkPayload` etc. Pure function, +unit-tested. Constructs: + +```ts +{ + pendingId, projectId, workspaceName, + pr: { number, url, title, headRefName, baseRefName, headRepositoryOwner, isCrossRepository, state }, + composer: { + prompt: pending.prompt, + baseBranch: prContent.baseRefName, // ← sourced from fetched PR + runSetupScript: pending.runSetupScript, + }, + linkedContext: ..., +} +``` + +The branch-path equivalent (`buildCheckoutPayload`) gains a matching +`composer.baseBranch` field sourced from the picker's base selection. + +### What does NOT change + +- `DashboardNewWorkspaceDraftContext.tsx` — `LinkedPR` type stays narrow. +- `PRLinkCommand.tsx` — no attach-time fetch, no spinner on the pill, no + loading state on the modal. +- `searchPullRequests` endpoint — unchanged. +- `create` and `adopt` procedures — untouched. +- `checkout`'s existing branch-path body — the git ops stay as-is; only the + postlude is factored into `finishCheckout`, and `composer.baseBranch` + (new field) feeds into it. + +## 3. Base branch — the Changes-tab decision + +**Always write `branch..base` in the `checkout` postlude**, sourced from +`composer.baseBranch`. Client populates the field per mode: + +- Branch path: picker-selected base branch (same semantics as `create` + uses today). +- PR path: `pr.baseRefName` — the PR's merge target on GitHub. +- Absent: skip the write (matches `create`'s current `head`-start-point + behavior). + +Server doesn't branch on intent for this config write — it reads +`composer.baseBranch` uniformly and writes. Simpler server, simpler +contract. + +### Why PR path always uses `pr.baseRefName` + +- Changes tab compares workspace HEAD against `branch..base`. For a PR, + the semantically correct comparison is "my PR head vs PR's merge target on + GitHub" — that's `baseRefName`. +- Users don't have a mental model of "pick a base for a PR checkout." +- Rare retarget case: existing `setBranchBaseConfig` helper covers it + post-create. + +### Side benefit: fixes a current gap + +Today's `checkout` procedure doesn't write `branch..base` at all — only +`create` (fork) does. That means picker-driven "Check out" workspaces have no +recorded base, and the Changes tab has to infer. With this change, all three +intents (fork, checkout, pr-checkout) record a base via the same config key. +Consistent Changes-tab behavior across creation paths. + +## 4. Decisions locked + +- **`gh pr checkout` as mechanism.** Hard dep on `gh auth login`; handles + fork-remote + upstream in one shot. +- **Closed/merged PRs: allow with warning.** V1's silent-allow + `warnings[]` + entry. +- **Base branch: shared postlude write, sourced from `composer.baseBranch`.** + PR path fills it with `pr.baseRefName`; branch path fills it with picker + selection. See §3. +- **One endpoint, two modes.** Widen `checkout`; client keeps a distinct + `pr-checkout` pending intent. +- **Zero net new fetches.** Pending page fetches once via `useQuery`, feeds + both mutation payload and agent-launch resolver. Moves the existing + `buildForkAgentLaunch` fetch earlier, not adds a new one. + +## 5. Fetch accounting + +| Scenario | Before (today) | After | +|---|---|---| +| Fork, no PR | 0 PR fetches | 0 PR fetches | +| Fork with linkedPR (today's behavior, no longer reachable once `pr-checkout` intent branches) | 1 fetch at agent-launch | — | +| PR-checkout | — | 1 fetch at pending-page, shared with agent-launch | + +Same total call count per submit. Timing moves from "after mutation" to +"before mutation," which is required for the mutation payload. + +## 6. Test plan + +### Host-service + +1. `pr-branch-name.test.ts` — pure function, ~8 cases. +2. `workspace-creation.checkout.integration.test.ts`: + - Existing branch-path tests unchanged. + - PR-path: same-repo, fork, idempotency (existing workspace → `alreadyExists: true`), closed-PR warning, `gh pr checkout` failure rolls back worktree, cloud-create failure rolls back worktree. + - Schema guards: both `branch` + `pr` → zod error; neither → zod error. + +### Renderer + +3. `buildPrCheckoutPayload.test.ts` — pure builder, construction cases. +4. Manual smoke: + - Same-repo PR: attach, submit, verify PR commits in workspace. + - Cross-repo PR: fork remote added, branch named `/`. + - Re-attach same PR: `alreadyExists` navigation. + - Closed PR: warning toast, workspace still created. + - `gh` missing: clear error at pending page. + +## 7. Rollout + +One PR: server widenings (1a, 1b, 1c) + renderer wiring (2a-2e) + tests. No +feature flag — gated by "user links a PR in the modal," same as v1. diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.ts index 141b6d4f241..9dd4393a47c 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildForkAgentLaunch.ts @@ -29,6 +29,14 @@ export interface LoadedAttachment { filename: string; } +export interface ResolvedPrContent { + number: number; + title: string; + body: string; + url: string; + branch: string; +} + export interface BuildForkAgentLaunchInputs { pending: Pick< PendingWorkspaceRow, @@ -62,11 +70,20 @@ export interface BuildForkAgentLaunchInputs { state: string; branch: string; baseBranch: string; + headRepositoryOwner: string | null; + isCrossRepository: boolean; author: string | null; }>; }; }; }; + /** + * Pre-resolved PR content. Used by the pr-checkout flow to avoid a + * redundant `getGitHubPullRequestContent` call — the pending page + * already fetched this once to build the mutation payload, so we + * thread it through rather than re-fetching inside `fetchPullRequest`. + */ + resolvedPr?: ResolvedPrContent; } /** @@ -132,6 +149,7 @@ export async function buildForkAgentLaunch( resolveCtx: buildResolveCtxFromPending( inputs.pending, inputs.hostServiceClient, + inputs.resolvedPr, ), }, ); @@ -386,6 +404,7 @@ function slugifyTitle(title: string): string { function buildResolveCtxFromPending( pending: BuildForkAgentLaunchInputs["pending"], client?: BuildForkAgentLaunchInputs["hostServiceClient"], + resolvedPr?: ResolvedPrContent, ): ResolveCtx { return { projectId: pending.projectId, @@ -440,6 +459,19 @@ function buildResolveCtxFromPending( }); } + // Pre-resolved from the pending page (pr-checkout path) — skip + // the redundant host-service call. The mutation payload already + // used the same `getGitHubPullRequestContent` response. + if (resolvedPr && resolvedPr.url === url) { + return { + number: resolvedPr.number, + url: resolvedPr.url, + title: resolvedPr.title, + body: resolvedPr.body, + branch: resolvedPr.branch, + }; + } + // Try host-service for full body + branch; fall back to pending-row. if (client) { try { diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.test.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.test.ts index fd2dd0a6bf6..868955a2009 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.test.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.test.ts @@ -4,6 +4,7 @@ import { buildAdoptPayload, buildCheckoutPayload, buildForkPayload, + buildPrCheckoutPayload, mapLinkedContextFromPending, } from "./buildIntentPayload"; @@ -176,6 +177,127 @@ describe("buildCheckoutPayload", () => { }); }); +describe("buildPrCheckoutPayload", () => { + const prContent = { + number: 42, + url: "https://github.com/o/r/pull/42", + title: "Fix typo", + branch: "fix/typo", + baseBranch: "main", + headRepositoryOwner: "kietho", + isCrossRepository: true, + state: "open", + body: "body text", + }; + + test("maps PR content into the pr input with normalized state", () => { + const pending = makePending({ + intent: "pr-checkout", + prompt: "review this PR", + linkedPR: { + prNumber: 42, + title: "Fix typo", + url: "https://github.com/o/r/pull/42", + state: "open", + }, + }); + const payload = buildPrCheckoutPayload("pid", pending, prContent); + + expect(payload.pr).toEqual({ + number: 42, + url: "https://github.com/o/r/pull/42", + title: "Fix typo", + headRefName: "fix/typo", + baseRefName: "main", + headRepositoryOwner: "kietho", + isCrossRepository: true, + state: "open", + }); + expect(payload.branch).toBeUndefined(); + }); + + test("composer.baseBranch = PR's baseRefName (Changes-tab authority)", () => { + const pending = makePending({ intent: "pr-checkout" }); + const payload = buildPrCheckoutPayload("pid", pending, { + ...prContent, + baseBranch: "develop", + }); + expect(payload.composer.baseBranch).toBe("develop"); + }); + + test("preserves prompt and runSetupScript from pending row", () => { + const pending = makePending({ + intent: "pr-checkout", + prompt: "hey", + runSetupScript: false, + }); + const payload = buildPrCheckoutPayload("pid", pending, prContent); + expect(payload.composer.prompt).toBe("hey"); + expect(payload.composer.runSetupScript).toBe(false); + }); + + test("linkedPrUrl falls back to PR content URL when linkedPR-level missing", () => { + // linkedPR exists but for some reason url isn't in the linkedIssues map + // (shouldn't happen normally, but be resilient). + const pending = makePending({ + intent: "pr-checkout", + linkedPR: null, + }); + const payload = buildPrCheckoutPayload("pid", pending, prContent); + expect(payload.linkedContext?.linkedPrUrl).toBe( + "https://github.com/o/r/pull/42", + ); + }); + + test("closed state maps to closed", () => { + const pending = makePending({ intent: "pr-checkout" }); + const payload = buildPrCheckoutPayload("pid", pending, { + ...prContent, + state: "closed", + }); + expect(payload.pr?.state).toBe("closed"); + }); + + test("merged state maps to merged", () => { + const pending = makePending({ intent: "pr-checkout" }); + const payload = buildPrCheckoutPayload("pid", pending, { + ...prContent, + state: "merged", + }); + expect(payload.pr?.state).toBe("merged"); + }); + + test("unknown state falls back to open (safe default)", () => { + const pending = makePending({ intent: "pr-checkout" }); + const payload = buildPrCheckoutPayload("pid", pending, { + ...prContent, + state: "draft", + }); + expect(payload.pr?.state).toBe("open"); + }); + + test("throws clear error for cross-repo PR with deleted fork (null owner)", () => { + const pending = makePending({ intent: "pr-checkout" }); + expect(() => + buildPrCheckoutPayload("pid", pending, { + ...prContent, + headRepositoryOwner: null, + isCrossRepository: true, + }), + ).toThrow("head fork repository has been deleted"); + }); + + test("same-repo PR with null owner is fine (owner not needed)", () => { + const pending = makePending({ intent: "pr-checkout" }); + const payload = buildPrCheckoutPayload("pid", pending, { + ...prContent, + headRepositoryOwner: null, + isCrossRepository: false, + }); + expect(payload.pr?.headRepositoryOwner).toBe(""); + }); +}); + describe("buildAdoptPayload", () => { test("minimal payload: projectId + host + name + branch", () => { const pending = makePending({ diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.ts index ea7e7bfdb2f..75dedab6398 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildIntentPayload.ts @@ -72,7 +72,81 @@ export function buildCheckoutPayload( hostTarget: pending.hostTarget, workspaceName: pending.name, branch: pending.branchName, - composer: { runSetupScript: pending.runSetupScript }, + composer: { + baseBranch: pending.baseBranch || undefined, + runSetupScript: pending.runSetupScript, + }, + }; +} + +/** + * Builds the `workspaceCreation.checkout` payload for PR mode. Requires the + * resolved PR content fetched at pending-page time (not persisted in the + * pending row itself — kept narrow on purpose). + * + * The server derives the real local branch name from `pr.headRefName` + + * `pr.isCrossRepository`; the pending row's `branchName` is only a display + * placeholder in PR mode. + */ +export function buildPrCheckoutPayload( + pendingId: string, + pending: PendingWorkspaceRow, + prContent: { + number: number; + url: string; + title: string; + branch: string; // headRefName + baseBranch: string; // baseRefName + headRepositoryOwner: string | null; + isCrossRepository: boolean; + state: string; + }, +): CheckoutWorkspaceInput { + // Null owner on a cross-repo PR means the head fork repo has been + // deleted. We can't derive `/` without it, and + // `gh pr checkout` wouldn't have a fork to configure push against. + // Fail early with a clear error rather than a cryptic server-side + // "headRepositoryOwner is required". + if (prContent.isCrossRepository && !prContent.headRepositoryOwner) { + throw new Error( + `Cannot check out PR #${prContent.number}: the head fork repository has been deleted.`, + ); + } + const linked = mapLinkedContextFromPending(pending); + const normalizedState: "open" | "closed" | "merged" = + prContent.state === "closed" + ? "closed" + : prContent.state === "merged" + ? "merged" + : "open"; + return { + pendingId, + projectId: pending.projectId, + hostTarget: pending.hostTarget, + workspaceName: pending.name, + pr: { + number: prContent.number, + url: prContent.url, + title: prContent.title, + headRefName: prContent.branch, + baseRefName: prContent.baseBranch, + // Same-repo PRs don't need an owner for branch derivation; pass an + // empty string rather than leaking null into the server input. + headRepositoryOwner: prContent.headRepositoryOwner ?? "", + isCrossRepository: prContent.isCrossRepository, + state: normalizedState, + }, + composer: { + prompt: pending.prompt || undefined, + // PR's base is authoritative for the Changes tab — see plan §3. + baseBranch: prContent.baseBranch, + runSetupScript: pending.runSetupScript, + }, + linkedContext: { + internalIssueIds: linked.internalIssueIds, + githubIssueUrls: linked.githubIssueUrls, + linkedPrUrl: linked.linkedPrUrl ?? prContent.url, + }, }; } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/dispatchForkLaunch.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/dispatchForkLaunch.ts index 1b4d18999bf..e09251751b1 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/dispatchForkLaunch.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/dispatchForkLaunch.ts @@ -10,6 +10,7 @@ import type { ResolvedAgentConfig } from "shared/utils/agent-settings"; import { buildForkAgentLaunch, type LoadedAttachment, + type ResolvedPrContent, } from "./buildForkAgentLaunch"; export interface DispatchForkLaunchInputs { @@ -21,6 +22,12 @@ export interface DispatchForkLaunchInputs { loadedAttachments: LoadedAttachment[] | undefined; agentConfigs: ResolvedAgentConfig[]; activeHostUrl: string | null; + /** + * Pre-resolved PR content from the pr-checkout flow. Threaded into + * `buildForkAgentLaunch` so the `fetchPullRequest` resolver skips a + * redundant `getGitHubPullRequestContent` call. + */ + resolvedPr?: ResolvedPrContent; onApplyToRow: (patch: { terminalLaunch?: PendingTerminalLaunch | null; chatLaunch?: PendingChatLaunch | null; @@ -43,6 +50,7 @@ export async function dispatchForkLaunch({ loadedAttachments, agentConfigs, activeHostUrl, + resolvedPr, onApplyToRow, }: DispatchForkLaunchInputs): Promise { console.log("[v2-launch] dispatchForkLaunch: start", { @@ -62,6 +70,7 @@ export async function dispatchForkLaunch({ attachments: loadedAttachments, agentConfigs, hostServiceClient: hostClient, + resolvedPr, }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx index 35aeded6602..9f314110d8e 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx @@ -21,10 +21,12 @@ import { useDashboardSidebarState } from "renderer/routes/_authenticated/hooks/u import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider"; import type { PendingWorkspaceRow } from "renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema"; import { useLocalHostService } from "renderer/routes/_authenticated/providers/LocalHostServiceProvider"; +import type { ResolvedPrContent } from "./buildForkAgentLaunch"; import { buildAdoptPayload, buildCheckoutPayload, buildForkPayload, + buildPrCheckoutPayload, } from "./buildIntentPayload"; import { buildSetupPaneLayout } from "./buildSetupPaneLayout"; import { dispatchForkLaunch } from "./dispatchForkLaunch"; @@ -74,6 +76,9 @@ function useFireIntent(pendingId: string, pending: PendingWorkspaceRow | null) { let loadedAttachments: | Array<{ data: string; mediaType: string; filename: string }> | undefined; + // Populated in the pr-checkout path; threaded into dispatchForkLaunch + // so the agent-launch resolver reuses the data instead of re-fetching. + let resolvedPr: ResolvedPrContent | undefined; switch (pending.intent) { case "fork": { @@ -103,6 +108,40 @@ function useFireIntent(pendingId: string, pending: PendingWorkspaceRow | null) { result = await adoptWorktree(buildAdoptPayload(pending)); break; } + case "pr-checkout": { + if (!pending.linkedPR) { + throw new Error("pr-checkout intent requires a linkedPR"); + } + const hostUrl = + pending.hostTarget.kind === "local" + ? activeHostUrl + : `${env.RELAY_URL}/hosts/${pending.hostTarget.hostId}`; + if (!hostUrl) { + throw new Error("Host service not available"); + } + const hostClient = getHostServiceClientByUrl(hostUrl); + // Single fetch — reused by both the mutation payload and the + // agent-launch resolver (via resolvedPr). Zero net new fetches + // vs fork-with-PR, which fetches the same data at launch build. + const prContent = + await hostClient.workspaceCreation.getGitHubPullRequestContent.query( + { + projectId: pending.projectId, + prNumber: pending.linkedPR.prNumber, + }, + ); + resolvedPr = { + number: prContent.number, + url: prContent.url, + title: prContent.title, + body: prContent.body, + branch: prContent.branch, + }; + result = await checkoutWorkspace( + buildPrCheckoutPayload(pendingId, pending, prContent), + ); + break; + } } // V2 dispatch: after host-service.create resolves, build the launch @@ -114,7 +153,10 @@ function useFireIntent(pendingId: string, pending: PendingWorkspaceRow | null) { // a useQuery hook — a not-yet-resolved query would silently skip // the dispatch, permanently losing the launch for a successful // workspace create. - if (pending.intent === "fork" && result.workspace?.id) { + const needsLaunchDispatch = + (pending.intent === "fork" || pending.intent === "pr-checkout") && + !!result.workspace?.id; + if (needsLaunchDispatch && result.workspace?.id) { const agentConfigs = await trpcUtils.settings.getAgentPresets.fetch(); await dispatchForkLaunch({ workspaceId: result.workspace.id, @@ -122,6 +164,7 @@ function useFireIntent(pendingId: string, pending: PendingWorkspaceRow | null) { loadedAttachments, agentConfigs, activeHostUrl, + resolvedPr, onApplyToRow: (patch) => { collections.pendingWorkspaces.update(pendingId, (draft) => { if (patch.terminalLaunch !== undefined) { diff --git a/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts b/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts index 1fd123714d3..22fb819e9ab 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts @@ -39,6 +39,19 @@ export function useSubmitWorkspace(projectId: string | null) { const { branchName, workspaceName } = resolveNames(draft); const pendingId = crypto.randomUUID(); + // PR mode: route to pr-checkout intent. Pending page fetches full + // PR details (getGitHubPullRequestContent) before firing the + // mutation, and derives the real branch name server-side from the + // resolved PR data. The `branchName` field here is a display + // placeholder; workspaceName similarly falls back to the PR title. + const isPrCheckout = draft.linkedPR !== null; + const prPlaceholderBranch = isPrCheckout + ? `pr-${draft.linkedPR?.prNumber}` + : null; + const prPlaceholderName = isPrCheckout + ? draft.linkedPR?.title || `PR #${draft.linkedPR?.prNumber}` + : null; + if (files.length > 0) { try { await storeAttachments(pendingId, files); @@ -53,9 +66,9 @@ export function useSubmitWorkspace(projectId: string | null) { collections.pendingWorkspaces.insert({ id: pendingId, projectId, - intent: "fork", - name: workspaceName, - branchName, + intent: isPrCheckout ? "pr-checkout" : "fork", + name: prPlaceholderName ?? workspaceName, + branchName: prPlaceholderBranch ?? branchName, prompt: draft.prompt, baseBranch: draft.baseBranch ?? null, baseBranchSource: draft.baseBranchSource ?? null, diff --git a/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCheckoutDashboardWorkspace/useCheckoutDashboardWorkspace.ts b/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCheckoutDashboardWorkspace/useCheckoutDashboardWorkspace.ts index 2d0d44c14c2..30a9bf145da 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCheckoutDashboardWorkspace/useCheckoutDashboardWorkspace.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/hooks/useCheckoutDashboardWorkspace/useCheckoutDashboardWorkspace.ts @@ -9,9 +9,25 @@ export interface CheckoutWorkspaceInput { projectId: string; hostTarget: WorkspaceHostTarget; workspaceName: string; - branch: string; + // Exactly one of `branch` or `pr` must be set — enforced server-side + // via zod refine. Branch mode: materialize an existing local/remote + // branch. PR mode: materialize a PR's branch via `gh pr checkout`. + branch?: string; + pr?: { + number: number; + url: string; + title: string; + headRefName: string; + baseRefName: string; + headRepositoryOwner: string; + isCrossRepository: boolean; + state: "open" | "closed" | "merged"; + }; composer: { prompt?: string; + // Written to `branch..base` for the Changes tab. Filled from + // picker selection in branch mode, `pr.baseRefName` in PR mode. + baseBranch?: string; runSetupScript?: boolean; }; linkedContext?: { @@ -28,7 +44,11 @@ export interface CheckoutWorkspaceInput { /** * Thin wrapper around the host-service `workspaceCreation.checkout` mutation. - * Creates a new workspace that reuses an existing branch (no new branch). + * Two modes: + * - Branch mode (`branch` set): reuse an existing local/remote branch. + * - PR mode (`pr` set): materialize a PR's branch via `gh pr checkout`; + * idempotent (returns `alreadyExists: true` if a workspace already exists + * for the derived branch). */ export function useCheckoutDashboardWorkspace() { const { activeHostUrl } = useLocalHostService(); @@ -51,6 +71,7 @@ export function useCheckoutDashboardWorkspace() { projectId: input.projectId, workspaceName: input.workspaceName, branch: input.branch, + pr: input.pr, composer: input.composer, linkedContext: input.linkedContext, }); diff --git a/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts b/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts index bf818eda570..ec6a969f096 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts @@ -154,7 +154,7 @@ export const pendingWorkspaceSchema = z.object({ hostTarget: pendingHostTargetSchema, // Which mutation the pending page should run. See V2_WORKSPACE_CREATION.md §3. // Defaults to "fork" for any rows that predate this field. - intent: z.enum(["fork", "checkout", "adopt"]).default("fork"), + intent: z.enum(["fork", "checkout", "adopt", "pr-checkout"]).default("fork"), name: z.string(), // fork: derived branch name from prompt; checkout/adopt: existing branch. branchName: z.string(), diff --git a/packages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.ts b/packages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.ts index 451f7989b17..765ee396507 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.ts @@ -10,19 +10,28 @@ const execFileAsync = promisify(execFile); * credential-manager plumbing and matches V1's behavior for * getIssueContent. * - * Returns parsed JSON output. Throws on non-zero exit or JSON parse - * failure. + * Returns parsed JSON output if stdout is JSON (typical for `--json` + * queries). For commands that don't return JSON (e.g., `gh pr checkout`), + * returns the trimmed stdout string. Throws on non-zero exit. */ -export async function execGh(args: string[]): Promise { +export async function execGh( + args: string[], + options?: { cwd?: string; timeout?: number }, +): Promise { const env = await getStrictShellEnvironment().catch( () => process.env as Record, ); const { stdout } = await execFileAsync("gh", args, { encoding: "utf8", - timeout: 10_000, + timeout: options?.timeout ?? 10_000, + cwd: options?.cwd, env, }); const trimmed = stdout.trim(); if (!trimmed) return {}; - return JSON.parse(trimmed); + try { + return JSON.parse(trimmed); + } catch { + return trimmed; + } } diff --git a/packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-name.test.ts b/packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-name.test.ts new file mode 100644 index 00000000000..b1547db43a5 --- /dev/null +++ b/packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-name.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, test } from "bun:test"; +import { derivePrLocalBranchName } from "./pr-branch-name"; + +describe("derivePrLocalBranchName", () => { + test("same-repo PR returns head ref as-is", () => { + expect( + derivePrLocalBranchName({ + headRefName: "fix/typo", + headRepositoryOwner: "cli", + isCrossRepository: false, + }), + ).toBe("fix/typo"); + }); + + test("cross-repo PR prefixes with lowercased owner", () => { + expect( + derivePrLocalBranchName({ + headRefName: "fix/browse-decimal-short-sha", + headRepositoryOwner: "lawrence3699", + isCrossRepository: true, + }), + ).toBe("lawrence3699/fix/browse-decimal-short-sha"); + }); + + test("cross-repo PR lowercases mixed-case owner", () => { + expect( + derivePrLocalBranchName({ + headRefName: "feat", + headRepositoryOwner: "Kietho", + isCrossRepository: true, + }), + ).toBe("kietho/feat"); + }); + + test("cross-repo PR preserves slashes in head ref", () => { + expect( + derivePrLocalBranchName({ + headRefName: "feat/foo/bar", + headRepositoryOwner: "user", + isCrossRepository: true, + }), + ).toBe("user/feat/foo/bar"); + }); + + test("same-repo PR ignores owner field", () => { + expect( + derivePrLocalBranchName({ + headRefName: "main", + headRepositoryOwner: "WHATEVER", + isCrossRepository: false, + }), + ).toBe("main"); + }); + + test("cross-repo PR handles owner with hyphens/numbers", () => { + expect( + derivePrLocalBranchName({ + headRefName: "branch", + headRepositoryOwner: "User-123", + isCrossRepository: true, + }), + ).toBe("user-123/branch"); + }); + + test("empty headRefName throws", () => { + expect(() => + derivePrLocalBranchName({ + headRefName: "", + headRepositoryOwner: "user", + isCrossRepository: false, + }), + ).toThrow("headRefName is required"); + }); + + test("whitespace-only headRefName throws", () => { + expect(() => + derivePrLocalBranchName({ + headRefName: " ", + headRepositoryOwner: "user", + isCrossRepository: true, + }), + ).toThrow("headRefName is required"); + }); + + test("trims surrounding whitespace on headRefName", () => { + expect( + derivePrLocalBranchName({ + headRefName: " fix/foo ", + headRepositoryOwner: "user", + isCrossRepository: true, + }), + ).toBe("user/fix/foo"); + }); + + test("cross-repo with empty owner throws", () => { + expect(() => + derivePrLocalBranchName({ + headRefName: "foo", + headRepositoryOwner: "", + isCrossRepository: true, + }), + ).toThrow("headRepositoryOwner is required"); + }); + + test("cross-repo with whitespace-only owner throws", () => { + expect(() => + derivePrLocalBranchName({ + headRefName: "foo", + headRepositoryOwner: " ", + isCrossRepository: true, + }), + ).toThrow("headRepositoryOwner is required"); + }); +}); diff --git a/packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-name.ts b/packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-name.ts new file mode 100644 index 00000000000..a6d702c91bf --- /dev/null +++ b/packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-name.ts @@ -0,0 +1,29 @@ +/** + * Derive the local branch name for a PR workspace. + * + * Same-repo PRs use the head ref as-is. Cross-repo (fork) PRs get the + * fork owner as a lowercase prefix — avoids collisions with local/upstream + * branches of the same name and namespaces by author. + * + * Mirrors v1 (`apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts:1630`). + */ +export function derivePrLocalBranchName(pr: { + headRefName: string; + headRepositoryOwner: string; + isCrossRepository: boolean; +}): string { + const headRef = pr.headRefName.trim(); + if (!headRef) { + throw new Error("derivePrLocalBranchName: headRefName is required"); + } + if (pr.isCrossRepository) { + const owner = pr.headRepositoryOwner.trim().toLowerCase(); + if (!owner) { + throw new Error( + "derivePrLocalBranchName: headRepositoryOwner is required for cross-repo PRs", + ); + } + return `${owner}/${headRef}`; + } + return headRef; +} diff --git a/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts b/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts index 49514552376..ffd521186a8 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts @@ -2,7 +2,7 @@ import { existsSync, mkdirSync } from "node:fs"; import { dirname, join, resolve, sep } from "node:path"; import { getDeviceName, getHashedDeviceId } from "@superset/shared/device-info"; import { TRPCError } from "@trpc/server"; -import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import simpleGit from "simple-git"; import { z } from "zod"; import { projects, workspaces } from "../../../db/schema"; @@ -17,6 +17,7 @@ import { createTerminalSessionInternal } from "../../../terminal/terminal"; import type { HostServiceContext } from "../../../types"; import { protectedProcedure, router } from "../../index"; import { execGh } from "./utils/exec-gh"; +import { derivePrLocalBranchName } from "./utils/pr-branch-name"; import { resolveStartPoint } from "./utils/resolve-start-point"; import { deduplicateBranchName } from "./utils/sanitize-branch"; @@ -290,6 +291,153 @@ function buildStartPointFromHint( }; } +/** + * Shared postlude for `checkout` (both branch and PR paths). + * + * - Writes `branch..base` from `composer.baseBranch` for the Changes tab. + * - `ensureV2Host` + `v2Workspace.create` with rollback on failure. + * - Inserts the local `workspaces` row. + * - Optionally spawns the setup terminal. + * - Clears progress. + */ +async function finishCheckout( + ctx: HostServiceContext, + args: { + pendingId: string; + projectId: string; + workspaceName: string; + branch: string; + worktreePath: string; + baseBranch: string | undefined; + runSetupScript: boolean; + git: GitClient; + extraWarnings: string[]; + }, +): Promise<{ + workspace: { id: string }; + terminals: Array<{ id: string; role: string; label: string }>; + warnings: string[]; + alreadyExists?: false; +}> { + setProgress(args.pendingId, "registering"); + + // Record the base branch for the Changes tab (skipped if unset — matches + // `create`'s head-start-point behavior). + if (args.baseBranch) { + await args.git + .raw([ + "-C", + args.worktreePath, + "config", + `branch.${args.branch}.base`, + args.baseBranch, + ]) + .catch((err) => { + console.warn( + `[workspaceCreation.checkout] failed to record base branch ${args.baseBranch}:`, + err, + ); + }); + } + + const rollbackWorktree = async () => { + try { + await args.git.raw(["worktree", "remove", args.worktreePath]); + } catch (err) { + console.warn("[workspaceCreation.checkout] failed to rollback worktree", { + worktreePath: args.worktreePath, + err, + }); + } + }; + + const deviceClientId = getHashedDeviceId(); + const deviceName = getDeviceName(); + + let host: { id: string }; + try { + host = await ctx.api.device.ensureV2Host.mutate({ + organizationId: ctx.organizationId, + machineId: deviceClientId, + name: deviceName, + }); + } catch (err) { + console.error("[workspaceCreation.checkout] ensureV2Host failed", err); + clearProgress(args.pendingId); + await rollbackWorktree(); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `Failed to register host: ${err instanceof Error ? err.message : String(err)}`, + }); + } + + const cloudRow = await ctx.api.v2Workspace.create + .mutate({ + organizationId: ctx.organizationId, + projectId: args.projectId, + name: args.workspaceName, + branch: args.branch, + hostId: host.id, + }) + .catch(async (err) => { + console.error( + "[workspaceCreation.checkout] v2Workspace.create failed", + err, + ); + clearProgress(args.pendingId); + await rollbackWorktree(); + throw err; + }); + + if (!cloudRow) { + clearProgress(args.pendingId); + await rollbackWorktree(); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Cloud workspace create returned no row", + }); + } + + ctx.db + .insert(workspaces) + .values({ + id: cloudRow.id, + projectId: args.projectId, + worktreePath: args.worktreePath, + branch: args.branch, + }) + .run(); + + const terminals: Array<{ id: string; role: string; label: string }> = []; + const warnings: string[] = [...args.extraWarnings]; + + if (args.runSetupScript) { + const setupScriptPath = join(args.worktreePath, ".superset", "setup.sh"); + if (existsSync(setupScriptPath)) { + const terminalId = crypto.randomUUID(); + const result = createTerminalSessionInternal({ + terminalId, + workspaceId: cloudRow.id, + db: ctx.db, + initialCommand: `bash "${setupScriptPath}"`, + }); + if ("error" in result) { + warnings.push(`Failed to start setup terminal: ${result.error}`); + } else { + terminals.push({ + id: terminalId, + role: "setup", + label: "Workspace Setup", + }); + } + } + } + + clearProgress(args.pendingId); + + return { workspace: cloudRow, terminals, warnings }; +} + // ── Router ─────────────────────────────────────────────────────────── export const workspaceCreationRouter = router({ @@ -827,39 +975,62 @@ export const workspaceCreationRouter = router({ */ checkout: protectedProcedure .input( - z.object({ - pendingId: z.string(), - projectId: z.string(), - workspaceName: z.string(), - branch: z.string(), - composer: z.object({ - prompt: z.string().optional(), - runSetupScript: z.boolean().optional(), + z + .object({ + pendingId: z.string(), + projectId: z.string(), + workspaceName: z.string(), + // Exactly one of `branch` or `pr` must be set (refine below). + // Branch mode: caller supplies a branch name; server resolves it. + // PR mode: caller supplies PR metadata; server derives branch name + // + runs `gh pr checkout`. + branch: z.string().optional(), + pr: z + .object({ + number: z.number().int().positive(), + url: z.string().url(), + title: z.string(), + headRefName: z.string(), + baseRefName: z.string(), + headRepositoryOwner: z.string(), + isCrossRepository: z.boolean(), + state: z.enum(["open", "closed", "merged"]), + }) + .optional(), + composer: z.object({ + prompt: z.string().optional(), + // Written to `branch..base` for the Changes tab. Client + // fills from picker in branch mode, or `pr.baseRefName` in PR + // mode. Server reads uniformly — no intent branching for this + // write. + baseBranch: z.string().optional(), + runSetupScript: z.boolean().optional(), + }), + linkedContext: z + .object({ + internalIssueIds: z.array(z.string()).optional(), + githubIssueUrls: z.array(z.string()).optional(), + linkedPrUrl: z.string().optional(), + attachments: z + .array( + z.object({ + data: z.string(), + mediaType: z.string(), + filename: z.string().optional(), + }), + ) + .optional(), + }) + .optional(), + }) + .refine((v) => Boolean(v.branch) !== Boolean(v.pr), { + message: "exactly one of `branch` or `pr` must be set", }), - linkedContext: z - .object({ - internalIssueIds: z.array(z.string()).optional(), - githubIssueUrls: z.array(z.string()).optional(), - linkedPrUrl: z.string().optional(), - attachments: z - .array( - z.object({ - data: z.string(), - mediaType: z.string(), - filename: z.string().optional(), - }), - ) - .optional(), - }) - .optional(), - }), ) .mutation(async ({ ctx, input }) => { - const deviceClientId = getHashedDeviceId(); - const deviceName = getDeviceName(); setProgress(input.pendingId, "ensuring_repo"); - // 1. Ensure project locally (clone if missing) — same as create + // Ensure project locally (clone if missing) — shared across both paths. let localProject = ctx.db.query.projects .findFirst({ where: eq(projects.id, input.projectId) }) .sync(); @@ -890,7 +1061,148 @@ export const workspaceCreationRouter = router({ setProgress(input.pendingId, "creating_worktree"); - const branch = input.branch.trim(); + // ── PR path ──────────────────────────────────────────────────────── + if (input.pr) { + const branch = derivePrLocalBranchName(input.pr); + + // Idempotency: existing workspace for this PR's branch → + // return it. Renderer navigates to it via `alreadyExists: true` + // instead of treating as a new create. + const existing = ctx.db.query.workspaces + .findFirst({ + where: and( + eq(workspaces.projectId, input.projectId), + eq(workspaces.branch, branch), + ), + }) + .sync(); + if (existing) { + clearProgress(input.pendingId); + return { + workspace: { id: existing.id }, + terminals: [], + warnings: [], + alreadyExists: true as const, + }; + } + + let worktreePath: string; + try { + worktreePath = safeResolveWorktreePath(localProject.repoPath, branch); + } catch (err) { + clearProgress(input.pendingId); + throw err; + } + const git = await ctx.git(localProject.repoPath); + + // Detect a pre-existing local branch with the same derived name + // BEFORE running `gh pr checkout --force`. The idempotency check + // above rules out Superset-managed worktrees, but a branch can + // exist outside any workspace — e.g., from a prior manual + // `gh pr checkout` in the primary working tree. `--force` would + // reset it to the PR HEAD, silently losing any unpushed commits. + // We surface a warning pointing at reflog for recovery rather + // than blocking, so the point-and-click flow stays smooth. + let preExistingLocalBranch = false; + try { + await git.raw([ + "show-ref", + "--verify", + "--quiet", + `refs/heads/${branch}`, + ]); + preExistingLocalBranch = true; + } catch { + // Non-zero exit = branch doesn't exist. Expected path. + } + + // Detached worktree first — `gh pr checkout` inside it creates the + // branch with correct fork-remote + upstream config. Mirrors v1's + // `createWorktreeFromPr`. + try { + await git.raw(["worktree", "add", "--detach", worktreePath]); + } catch (err) { + clearProgress(input.pendingId); + throw new TRPCError({ + code: "CONFLICT", + message: + err instanceof Error + ? err.message + : "Failed to add detached worktree", + }); + } + + try { + await execGh( + [ + "pr", + "checkout", + String(input.pr.number), + "--branch", + branch, + "--force", + ], + { cwd: worktreePath, timeout: 120_000 }, + ); + } catch (err) { + await git + .raw(["worktree", "remove", "--force", worktreePath]) + .catch(() => {}); + clearProgress(input.pendingId); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `gh pr checkout failed: ${ + err instanceof Error ? err.message : String(err) + }`, + }); + } + + // Push ergonomics. `gh pr checkout` sets per-branch push config + // to the fork URL for cross-repo PRs; this covers the same-repo + // case where upstream isn't auto-set. + await git + .raw([ + "-C", + worktreePath, + "config", + "--local", + "push.autoSetupRemote", + "true", + ]) + .catch((err) => { + console.warn( + "[workspaceCreation.checkout] failed to set push.autoSetupRemote:", + err, + ); + }); + + const extraWarnings: string[] = []; + if (input.pr.state !== "open") { + extraWarnings.push( + `PR is ${input.pr.state} — commits are included, but the PR may not merge.`, + ); + } + if (preExistingLocalBranch) { + extraWarnings.push( + `Reset existing local branch "${branch}" to PR HEAD. If you had unpushed commits there, recover them via \`git reflog show ${branch}\`.`, + ); + } + + return await finishCheckout(ctx, { + pendingId: input.pendingId, + projectId: input.projectId, + workspaceName: input.workspaceName, + branch, + worktreePath, + baseBranch: input.composer.baseBranch, + runSetupScript: input.composer.runSetupScript ?? false, + git, + extraWarnings, + }); + } + + // ── Branch path ──────────────────────────────────────────────────── + const branch = (input.branch ?? "").trim(); if (!branch) { clearProgress(input.pendingId); throw new TRPCError({ @@ -989,101 +1301,17 @@ export const workspaceCreationRouter = router({ ); }); - setProgress(input.pendingId, "registering"); - - const rollbackWorktree = async () => { - try { - await git.raw(["worktree", "remove", worktreePath]); - } catch (err) { - console.warn( - "[workspaceCreation.checkout] failed to rollback worktree", - { worktreePath, err }, - ); - } - }; - - let host: { id: string }; - try { - host = await ctx.api.device.ensureV2Host.mutate({ - organizationId: ctx.organizationId, - machineId: deviceClientId, - name: deviceName, - }); - } catch (err) { - console.error("[workspaceCreation.checkout] ensureV2Host failed", err); - clearProgress(input.pendingId); - await rollbackWorktree(); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: `Failed to register host: ${err instanceof Error ? err.message : String(err)}`, - }); - } - - const cloudRow = await ctx.api.v2Workspace.create - .mutate({ - organizationId: ctx.organizationId, - projectId: input.projectId, - name: input.workspaceName, - branch, - hostId: host.id, - }) - .catch(async (err) => { - console.error( - "[workspaceCreation.checkout] v2Workspace.create failed", - err, - ); - clearProgress(input.pendingId); - await rollbackWorktree(); - throw err; - }); - - if (!cloudRow) { - clearProgress(input.pendingId); - await rollbackWorktree(); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "Cloud workspace create returned no row", - }); - } - - ctx.db - .insert(workspaces) - .values({ - id: cloudRow.id, - projectId: input.projectId, - worktreePath, - branch, - }) - .run(); - - const terminals: Array<{ id: string; role: string; label: string }> = []; - const warnings: string[] = []; - - if (input.composer.runSetupScript) { - const setupScriptPath = join(worktreePath, ".superset", "setup.sh"); - if (existsSync(setupScriptPath)) { - const terminalId = crypto.randomUUID(); - const result = createTerminalSessionInternal({ - terminalId, - workspaceId: cloudRow.id, - db: ctx.db, - initialCommand: `bash "${setupScriptPath}"`, - }); - if ("error" in result) { - warnings.push(`Failed to start setup terminal: ${result.error}`); - } else { - terminals.push({ - id: terminalId, - role: "setup", - label: "Workspace Setup", - }); - } - } - } - - clearProgress(input.pendingId); - - return { workspace: cloudRow, terminals, warnings }; + return await finishCheckout(ctx, { + pendingId: input.pendingId, + projectId: input.projectId, + workspaceName: input.workspaceName, + branch, + worktreePath, + baseBranch: input.composer.baseBranch, + runSetupScript: input.composer.runSetupScript ?? false, + git, + extraWarnings: [], + }); }), /** @@ -1408,7 +1636,7 @@ export const workspaceCreationRouter = router({ "--repo", `${repo.owner}/${repo.name}`, "--json", - "number,title,body,url,state,author,headRefName,baseRefName,isDraft,createdAt,updatedAt", + "number,title,body,url,state,author,headRefName,baseRefName,headRepositoryOwner,isCrossRepository,isDraft,createdAt,updatedAt", ]); const data = PrSchema.parse(raw); return { @@ -1419,6 +1647,8 @@ export const workspaceCreationRouter = router({ state: data.state.toLowerCase(), branch: data.headRefName, baseBranch: data.baseRefName, + headRepositoryOwner: data.headRepositoryOwner?.login ?? null, + isCrossRepository: data.isCrossRepository, author: data.author?.login ?? null, isDraft: data.isDraft, createdAt: data.createdAt, @@ -1452,6 +1682,12 @@ const PrSchema = z.object({ state: z.string(), headRefName: z.string(), baseRefName: z.string(), + // `gh pr view` returns null when the PR's head fork repository has been + // deleted. Nullable so the schema parse doesn't fail; consumers decide + // how to handle a missing owner (client surfaces a clear error for + // cross-repo PRs — same-repo PRs shouldn't see null in practice). + headRepositoryOwner: z.object({ login: z.string() }).nullable(), + isCrossRepository: z.boolean(), isDraft: z.boolean(), author: z.object({ login: z.string() }).optional(), createdAt: z.string().optional(),