diff --git a/docs/development.md b/docs/development.md index f9361cc29..3fd7e4ff3 100644 --- a/docs/development.md +++ b/docs/development.md @@ -280,11 +280,18 @@ for machine-readable output. ## Worktree starting refs -The New workspace branch picker treats local and `origin/*` refs as distinct starting points when -they differ. Local `main` is sent as `refs/heads/main`; remote `origin/main` is sent as -`refs/remotes/origin/main`. Matching local/remote refs collapse to one local row. Unqualified refs -from older clients resolve local-first, then fall back to `origin`. Worktrees inherit committed Git -state only; uncommitted source-checkout changes are not copied. +A new worktree starts from the current branch's upstream, or the local branch when it has no +upstream. This keeps unpushed local commits out of new workspaces by default. The picker collapses +identical refs; divergent local or non-origin refs remain explicit, qualified choices. + +The daemon sends the exact upstream ref because the remote and branch names cannot be inferred. +Worktrees retain that ref for comparisons and updates from base while exposing its branch name to +the UI. Merging into base requires a mutable local target: `origin/main` maps to local `main`, while +another remote fails closed until the worktree records an explicit local target. Older daemons omit +the optional field and retain the previous local-first behavior; older worktree metadata without the +exact ref also resolves through its stored branch name. + +Worktrees inherit committed Git state only; uncommitted source-checkout changes are not copied. ## paseo.json service scripts diff --git a/packages/app/e2e/browser/new-workspace.spec.ts b/packages/app/e2e/browser/new-workspace.spec.ts index 8342dca68..49062e482 100644 --- a/packages/app/e2e/browser/new-workspace.spec.ts +++ b/packages/app/e2e/browser/new-workspace.spec.ts @@ -24,6 +24,10 @@ import { openProjectViaDaemon, openStartingRefPicker, pasteGithubPrUrl, + captureStartingRefPicker, + expectStartingRefRows, + startingRefRow, + submitNewWorkspaceEmpty, searchAndSelectBranchInPicker, selectBranchInPicker, selectGitHubPrInPicker, @@ -31,7 +35,14 @@ import { selectWorkspaceIsolation, submitNewWorkspacePrompt, } from "../support/helpers/new-workspace"; -import { createTempGitRepo, readWorktreeBranchInfo } from "../support/helpers/workspace"; +import { + commitLocalOnly, + createTempGitRepo, + readRepoRef, + readWorktreeBaseMetadata, + readWorktreeBranchInfo, + trackForkUpstream, +} from "../support/helpers/workspace"; import { createLocalGithubPrFixture, cloneGithubRepoDefaultBranchOnly, @@ -743,6 +754,142 @@ test.describe("New workspace flow", () => { } }); + // The starting ref the daemon actually cuts from is the thing that broke: the picker said + // one ref and the worktree was created from another. Every assertion here reads the + // created worktree's commits or its recorded base, never the trigger text alone. + test.describe("default starting ref", () => { + async function openWorktreeComposerForRepo( + page: import("@playwright/test").Page, + repoPath: string, + ) { + const openedProject = await openProjectViaDaemon(client, repoPath); + localWorkspaceIds.add(openedProject.workspaceId); + + await gotoAppShell(page); + await waitForSidebarHydration(page); + await openNewWorkspaceComposer(page, { + projectKey: openedProject.projectKey, + projectDisplayName: openedProject.projectDisplayName, + }); + await selectWorkspaceIsolation(page, "worktree"); + return openedProject; + } + + async function createWorktreeAndRead( + page: import("@playwright/test").Page, + openedProject: Awaited>, + ) { + await submitNewWorkspaceEmpty(page); + const createdWorkspace = await assertNewWorkspaceSidebarAndHeader(page, { + serverId: getServerId(), + client, + previousWorkspaceId: openedProject.workspaceId, + projectDisplayName: openedProject.projectDisplayName, + }); + createdWorktreeDirectories.add(createdWorkspace.workspaceDirectory); + return { + ...createdWorkspace, + branchInfo: await readWorktreeBranchInfo({ + worktreePath: createdWorkspace.workspaceDirectory, + }), + }; + } + + test("branches off the upstream when the local branch is ahead and the picker is untouched", async ({ + page, + }) => { + const tempRepo = await createTempGitRepo("ref-default-ahead-", { withRemote: true }); + + try { + const originHead = readRepoRef(tempRepo.path, "refs/remotes/origin/main"); + commitLocalOnly(tempRepo.path, "one"); + const localHead = commitLocalOnly(tempRepo.path, "two"); + + const openedProject = await openWorktreeComposerForRepo(page, tempRepo.path); + const created = await createWorktreeAndRead(page, openedProject); + + expect(created.branchInfo.hasAncestor(originHead)).toBe(true); + expect(created.branchInfo.hasAncestor(localHead)).toBe(false); + } finally { + await tempRepo.cleanup(); + } + }); + + test("branches off the local ref when the local row is chosen explicitly", async ({ + page, + }, testInfo) => { + const tempRepo = await createTempGitRepo("ref-default-local-pick-", { withRemote: true }); + + try { + commitLocalOnly(tempRepo.path, "one"); + const localHead = commitLocalOnly(tempRepo.path, "two"); + + const openedProject = await openWorktreeComposerForRepo(page, tempRepo.path); + + await openStartingRefPicker(page); + await expectStartingRefRows(page, [ + "main, origin branch", + "main, local branch, 2 commits ahead of origin main", + ]); + const screenshotPath = testInfo.outputPath("ref-picker-local-ahead.png"); + await captureStartingRefPicker(page, screenshotPath); + await testInfo.attach("Ref picker: local ahead of upstream", { + path: screenshotPath, + contentType: "image/png", + }); + await startingRefRow(page, "main, local branch, 2 commits ahead of origin main").click(); + await expectPickerSelected(page, "main (local)"); + + const created = await createWorktreeAndRead(page, openedProject); + expect(created.branchInfo.hasAncestor(localHead)).toBe(true); + } finally { + await tempRepo.cleanup(); + } + }); + + test("branches off a fork's upstream remote and records the branch name", async ({ + page, + }, testInfo) => { + const tempRepo = await createTempGitRepo("ref-default-fork-", { withRemote: true }); + + try { + const upstreamHead = await trackForkUpstream(tempRepo.path); + const originHead = readRepoRef(tempRepo.path, "refs/remotes/origin/main"); + + const openedProject = await openWorktreeComposerForRepo(page, tempRepo.path); + + await openStartingRefPicker(page); + // Branch suggestions only know about origin, so the upstream the fork actually + // tracks gets its own row rather than silently sharing origin's. Two rows reading + // "main" is the ambiguity this whole change exists to remove. + await expectStartingRefRows(page, [ + "main (upstream), upstream branch", + "main, origin branch", + ]); + await expectPickerSelected(page, "main (upstream)"); + const screenshotPath = testInfo.outputPath("ref-picker-fork.png"); + await captureStartingRefPicker(page, screenshotPath); + await testInfo.attach("Ref picker: fork tracking upstream/main", { + path: screenshotPath, + contentType: "image/png", + }); + await closeBranchPicker(page); + + const created = await createWorktreeAndRead(page, openedProject); + + expect(created.branchInfo.hasAncestor(upstreamHead)).toBe(true); + expect(upstreamHead).not.toBe(originHead); + // The name is what the UI shows; the ref is what resolves back to this commit. + expect(await readWorktreeBaseMetadata(created.workspaceDirectory)).toEqual({ + baseRefName: "main", + baseRef: "refs/remotes/upstream/main", + }); + } finally { + await tempRepo.cleanup(); + } + }); + }); + test("branch picker opens via keyboard and selects the filtered option on Enter", async ({ page, }) => { diff --git a/packages/app/e2e/support/helpers/new-workspace.ts b/packages/app/e2e/support/helpers/new-workspace.ts index 4d451c147..a20811591 100644 --- a/packages/app/e2e/support/helpers/new-workspace.ts +++ b/packages/app/e2e/support/helpers/new-workspace.ts @@ -348,6 +348,23 @@ export async function searchAndSelectBranchInPicker(page: Page, name: string): P await selectBranchInPicker(page, name); } +// Ref picker rows are named for a user: "main, origin branch" is the upstream copy and +// "main, local branch, 2 commits ahead of origin main" is the local one. +export function startingRefRow(page: Page, accessibleName: string) { + return page.getByRole("button", { name: accessibleName, exact: true }); +} + +export async function expectStartingRefRows(page: Page, accessibleNames: string[]): Promise { + for (const name of accessibleNames) { + await expect(startingRefRow(page, name)).toBeVisible({ timeout: 30_000 }); + } +} + +export async function captureStartingRefPicker(page: Page, screenshotPath: string): Promise { + await expect(page.getByTestId("combobox-desktop-container")).toBeVisible({ timeout: 30_000 }); + await page.getByTestId("combobox-desktop-container").screenshot({ path: screenshotPath }); +} + export async function selectGitHubPrInPicker(page: Page, number: number): Promise { const prRow = page.getByTestId(`new-workspace-ref-picker-pr-${number}`); await expect(prRow).toBeVisible({ timeout: 30_000 }); diff --git a/packages/app/e2e/support/helpers/workspace.ts b/packages/app/e2e/support/helpers/workspace.ts index 0ab5a9559..23bfc4bf9 100644 --- a/packages/app/e2e/support/helpers/workspace.ts +++ b/packages/app/e2e/support/helpers/workspace.ts @@ -1,5 +1,5 @@ import { execSync } from "node:child_process"; -import { mkdtemp, writeFile, rm, mkdir, realpath } from "node:fs/promises"; +import { mkdtemp, readFile, writeFile, rm, mkdir, realpath } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -159,6 +159,80 @@ export async function createTempDirectory(prefix = "paseo-e2e-dir-"): Promise { + const upstreamDir = path.join(repoPath, "upstream.git"); + const upstreamClone = await mkdtemp(path.join(await resolveTempRoot(), "paseo-e2e-upstream-")); + await mkdir(upstreamDir, { recursive: true }); + execSync(`git init --bare -b main ${upstreamDir}`, { cwd: repoPath, stdio: "ignore" }); + execSync(`git remote add upstream ${upstreamDir}`, { cwd: repoPath, stdio: "ignore" }); + execSync("git push upstream main", { cwd: repoPath, stdio: "ignore" }); + + execSync(`git clone ${upstreamDir} ${upstreamClone}`, { stdio: "ignore" }); + execSync('git config user.email "e2e@paseo.test"', { cwd: upstreamClone, stdio: "ignore" }); + execSync('git config user.name "Paseo E2E"', { cwd: upstreamClone, stdio: "ignore" }); + execSync("git config commit.gpgsign false", { cwd: upstreamClone, stdio: "ignore" }); + execSync('git commit --allow-empty -m "upstream only"', { + cwd: upstreamClone, + stdio: "ignore", + }); + execSync("git push origin main", { cwd: upstreamClone, stdio: "ignore" }); + await rm(upstreamClone, { recursive: true, force: true }); + + execSync("git fetch upstream", { cwd: repoPath, stdio: "ignore" }); + execSync("git branch --set-upstream-to=upstream/main main", { + cwd: repoPath, + stdio: "ignore", + }); + return execSync("git rev-parse refs/remotes/upstream/main", { cwd: repoPath, stdio: "pipe" }) + .toString() + .trim(); +} + +export function readRepoRef(repoPath: string, ref: string): string { + return execSync(`git rev-parse ${JSON.stringify(ref)}`, { cwd: repoPath, stdio: "pipe" }) + .toString() + .trim(); +} + +/** + * The base identity the daemon recorded in worktree.json: the display name the UI reads back, + * and the exact ref every comparison and action resolves through. + */ +export async function readWorktreeBaseMetadata( + worktreePath: string, +): Promise<{ baseRefName: string; baseRef?: string }> { + const gitDir = execSync("git rev-parse --absolute-git-dir", { cwd: worktreePath, stdio: "pipe" }) + .toString() + .trim(); + const metadata = JSON.parse( + await readFile(path.join(gitDir, "paseo", "worktree.json"), "utf8"), + ) as { baseRefName?: string; baseRef?: string }; + if (!metadata.baseRefName) { + throw new Error(`worktree.json has no baseRefName: ${worktreePath}`); + } + return { + baseRefName: metadata.baseRefName, + ...(metadata.baseRef ? { baseRef: metadata.baseRef } : {}), + }; +} + export async function readWorktreeBranchInfo({ worktreePath }: { worktreePath: string }): Promise<{ currentBranch: string; hasAncestor: (ref: string) => boolean; diff --git a/packages/app/src/screens/new-workspace-picker-item.test.ts b/packages/app/src/screens/new-workspace-picker-item.test.ts index 12c343dc8..739738e0f 100644 --- a/packages/app/src/screens/new-workspace-picker-item.test.ts +++ b/packages/app/src/screens/new-workspace-picker-item.test.ts @@ -1,11 +1,12 @@ import { describe, expect, it } from "vitest"; import type { ForgeSearchItem } from "@getpaseo/protocol/messages"; -import type { CheckoutStatusPayload } from "@/git/checkout-status-cache"; import { + type BranchPickerDetail, branchPickerOptionId, buildBranchPickerItems, + buildPickerOptionData, + defaultBasePickerItem, pickerItemToCheckoutRequest, - resolveCheckoutRequest, type PickerItem, } from "./new-workspace-picker-item"; @@ -104,7 +105,7 @@ describe("pickerItemToCheckoutRequest", () => { }); describe("buildBranchPickerItems", () => { - it("hides the origin duplicate when local and origin match", () => { + it("keeps a single origin row when local and origin match", () => { expect( buildBranchPickerItems([ { @@ -120,14 +121,14 @@ describe("buildBranchPickerItems", () => { { kind: "branch", name: "main", - refName: "refs/heads/main", - accessibilityLabel: "main, local branch, up to date with origin main", + refName: "refs/remotes/origin/main", + accessibilityLabel: "main, origin branch", committerDate: 10, }, ]); }); - it("creates compact local and origin rows when the refs differ", () => { + it("puts the origin row first and disambiguates the local row when the refs differ", () => { expect( buildBranchPickerItems([ { @@ -143,17 +144,16 @@ describe("buildBranchPickerItems", () => { { kind: "branch", name: "main", - refName: "refs/heads/main", - divergenceLabel: "+3 −2", - accessibilityLabel: "main, local branch, 3 commits ahead and 2 behind origin main", + refName: "refs/remotes/origin/main", + accessibilityLabel: "main, origin branch", committerDate: 10, }, { kind: "branch", - name: "origin/main", - refName: "refs/remotes/origin/main", - divergenceLabel: "+2 −3", - accessibilityLabel: "origin main, 2 commits ahead and 3 behind local main", + name: "main (local)", + refName: "refs/heads/main", + divergenceLabel: "+3 −2", + accessibilityLabel: "main, local branch, 3 commits ahead and 2 behind origin main", committerDate: 10, }, ]); @@ -172,9 +172,30 @@ describe("buildBranchPickerItems", () => { ).toEqual([ { kind: "branch", - name: "origin/release", + name: "release", refName: "refs/remotes/origin/release", - accessibilityLabel: "origin release, origin branch", + accessibilityLabel: "release, origin branch", + committerDate: 5, + }, + ]); + }); + + it("keeps the plain name for a local-only branch", () => { + expect( + buildBranchPickerItems([ + { + name: "scratch", + committerDate: 5, + hasLocal: true, + hasRemote: false, + }, + ]), + ).toEqual([ + { + kind: "branch", + name: "scratch", + refName: "refs/heads/scratch", + accessibilityLabel: "scratch, local branch", committerDate: 5, }, ]); @@ -194,15 +215,15 @@ describe("buildBranchPickerItems", () => { { kind: "branch", name: "main", - refName: "refs/heads/main", - accessibilityLabel: "main, local branch", + refName: "refs/remotes/origin/main", + accessibilityLabel: "main, origin branch", committerDate: 10, }, { kind: "branch", - name: "origin/main", - refName: "refs/remotes/origin/main", - accessibilityLabel: "origin main, origin branch", + name: "main (local)", + refName: "refs/heads/main", + accessibilityLabel: "main, local branch", committerDate: 10, }, ]); @@ -221,38 +242,101 @@ describe("buildBranchPickerItems", () => { }); }); -describe("resolveCheckoutRequest", () => { - const checkoutStatus = { - currentBranch: "feature/current", - } as CheckoutStatusPayload; +const mainRow: BranchPickerDetail = { + name: "main", + committerDate: 10, + hasLocal: true, + hasRemote: true, + localAhead: 2, + localBehind: 0, +}; - it("branches from the loaded current branch when nothing was picked", () => { - expect(resolveCheckoutRequest(null, checkoutStatus)).toEqual({ - action: "branch-off", - refName: "feature/current", +describe("buildPickerOptionData", () => { + it("marks origin and keeps an ahead local main explicit", () => { + const baseItem = defaultBasePickerItem({ + currentBranch: "main", + upstreamRef: "refs/remotes/origin/main", }); + const data = buildPickerOptionData({ branchDetails: [mainRow], prItems: [], baseItem }); + + expect(data.options.map((option) => option.label)).toEqual(["main", "main (local)"]); + expect(data.selectedOptionId).toBe(branchPickerOptionId("refs/remotes/origin/main")); }); - it("prefers an explicit picker selection", () => { - expect( - resolveCheckoutRequest( - { - kind: "branch", - name: "feature/picked", - refName: "refs/heads/feature/picked", - accessibilityLabel: "feature/picked, local branch", - }, - checkoutStatus, - ), - ).toEqual({ - action: "branch-off", - refName: "refs/heads/feature/picked", + it("adds and disambiguates a fork upstream absent from branch suggestions", () => { + const baseItem = defaultBasePickerItem({ + currentBranch: "main", + upstreamRef: "refs/remotes/upstream/main", }); + const data = buildPickerOptionData({ + branchDetails: [{ ...mainRow, localAhead: 0, localBehind: 0 }], + prItems: [], + baseItem, + }); + + expect(data.options.map((option) => option.label)).toEqual(["main (upstream)", "main"]); + expect(data.selectedOptionId).toBe(branchPickerOptionId("refs/remotes/upstream/main")); }); - it("preserves the server-default intent for a detached checkout", () => { + it("marks a visible row on an old daemon when local and origin are in sync", () => { + const baseItem = defaultBasePickerItem({ currentBranch: "main" }); + const data = buildPickerOptionData({ + branchDetails: [{ ...mainRow, localAhead: 0, localBehind: 0 }], + prItems: [], + baseItem, + }); + + expect(data.options.map((option) => option.label)).toEqual(["main (local)", "main"]); + expect(data.selectedOptionId).toBe(branchPickerOptionId("refs/heads/main")); + }); + + it("keeps an explicit local selection marked", () => { + const baseItem: PickerItem = { + kind: "branch", + name: "main (local)", + refName: "refs/heads/main", + accessibilityLabel: "main, local branch", + }; + const data = buildPickerOptionData({ + branchDetails: [mainRow], + prItems: [], + baseItem, + }); + const selected = data.itemById.get(data.selectedOptionId) ?? null; + expect(pickerItemToCheckoutRequest(selected)).toEqual({ + action: "branch-off", + refName: "refs/heads/main", + }); + }); +}); + +describe("defaultBasePickerItem", () => { + it("uses origin when the current branch tracks origin", () => { expect( - resolveCheckoutRequest(null, { ...checkoutStatus, currentBranch: null }), - ).toBeUndefined(); + defaultBasePickerItem({ + currentBranch: "main", + upstreamRef: "refs/remotes/origin/main", + }), + ).toMatchObject({ refName: "refs/remotes/origin/main", name: "main" }); + }); + + it("uses the exact non-origin upstream", () => { + expect( + defaultBasePickerItem({ + currentBranch: "main", + upstreamRef: "refs/remotes/upstream/main", + }), + ).toMatchObject({ refName: "refs/remotes/upstream/main", name: "main" }); + }); + + it("keeps old-daemon behavior local", () => { + expect(defaultBasePickerItem({ currentBranch: "main" })).toMatchObject({ + refName: "refs/heads/main", + name: "main", + }); + }); + + it("has no default for detached HEAD", () => { + expect(defaultBasePickerItem({ currentBranch: null })).toBeNull(); }); }); diff --git a/packages/app/src/screens/new-workspace-picker-item.ts b/packages/app/src/screens/new-workspace-picker-item.ts index ded824e3d..9f04d35e4 100644 --- a/packages/app/src/screens/new-workspace-picker-item.ts +++ b/packages/app/src/screens/new-workspace-picker-item.ts @@ -1,6 +1,7 @@ import type { CreatePaseoWorktreeInput } from "@getpaseo/client/internal/daemon-client"; import type { ForgeSearchItem } from "@getpaseo/protocol/messages"; -import type { CheckoutStatusPayload } from "@/git/checkout-status-cache"; +import type { ComboboxOptionModel } from "@/components/ui/combobox-options"; +import { getForgePresentation } from "@/git/forge"; export interface BranchPickerDetail { name: string; @@ -31,6 +32,8 @@ export type PickerCheckoutRequest = Pick< >; const BRANCH_OPTION_PREFIX = "branch:"; +const PR_OPTION_PREFIX = "github-pr:"; +const REMOTE_TRACKING_PREFIX = "refs/remotes/"; export function branchPickerOptionId(refName: string): string { return `${BRANCH_OPTION_PREFIX}${refName}`; @@ -82,11 +85,27 @@ export function buildBranchPickerItems(details: readonly BranchPickerDetail[]): const localBehind = detail.localBehind; const hasDivergence = hasLocal && hasRemote && localAhead !== undefined && localBehind !== undefined; + const refsDiffer = hasDivergence && (localAhead > 0 || localBehind > 0); + // One row when the two refs point at the same commit, two rows when they differ or + // when the divergence is unknown. + const showsBothRefs = hasLocal && hasRemote && (!hasDivergence || refsDiffer); - if (hasLocal) { - const localItem: Extract = { + // The origin row goes first because it is the default base, and it never states + // divergence: it is the reference point the local row is measured against. + if (hasRemote) { + items.push({ kind: "branch", name: detail.name, + refName: `refs/remotes/origin/${detail.name}`, + accessibilityLabel: `${detail.name}, origin branch`, + committerDate: detail.committerDate, + }); + } + + if (hasLocal && (showsBothRefs || !hasRemote)) { + const localItem: Extract = { + kind: "branch", + name: showsBothRefs ? `${detail.name} (local)` : detail.name, refName: `refs/heads/${detail.name}`, accessibilityLabel: `${detail.name}, local branch`, committerDate: detail.committerDate, @@ -101,33 +120,64 @@ export function buildBranchPickerItems(details: readonly BranchPickerDetail[]): } items.push(localItem); } - - const refsDiffer = hasDivergence && (localAhead > 0 || localBehind > 0); - if (hasRemote && (!hasLocal || !hasDivergence || refsDiffer)) { - const remoteAhead = hasDivergence ? localBehind : 0; - const remoteBehind = hasDivergence ? localAhead : 0; - const remoteItem: Extract = { - kind: "branch", - name: `origin/${detail.name}`, - refName: `refs/remotes/origin/${detail.name}`, - accessibilityLabel: `origin ${detail.name}, origin branch`, - committerDate: detail.committerDate, - }; - if (hasDivergence) { - remoteItem.divergenceLabel = divergenceLabel(remoteAhead, remoteBehind); - remoteItem.accessibilityLabel = `origin ${detail.name}, ${divergenceAccessibility( - remoteAhead, - remoteBehind, - `local ${detail.name}`, - )}`; - } - items.push(remoteItem); - } } return items; } +export interface BaseRefCheckoutStatus { + currentBranch: string | null; + upstreamRef?: string | null; +} + +// Display only. The exact ref is what every request carries; this is just how a ref reads in +// a row label, so "refs/remotes/origin/other-name" shows as "other-name". +function branchNameFromRef(refName: string): string { + if (refName.startsWith("refs/heads/")) return refName.slice("refs/heads/".length); + if (refName.startsWith(REMOTE_TRACKING_PREFIX)) { + const remainder = refName.slice(REMOTE_TRACKING_PREFIX.length); + const separator = remainder.indexOf("/"); + return separator === -1 ? remainder : remainder.slice(separator + 1); + } + return refName; +} + +// Where a ref lives, for disambiguating two rows that would otherwise read the same. +function refQualifier(refName: string): string | null { + if (refName.startsWith("refs/heads/")) return "local"; + if (refName.startsWith(REMOTE_TRACKING_PREFIX)) { + const remainder = refName.slice(REMOTE_TRACKING_PREFIX.length); + const separator = remainder.indexOf("/"); + return separator === -1 ? null : remainder.slice(0, separator); + } + return null; +} + +// The one owner of "what do we branch off when the user picked nothing". The checkmarked +// row, the trigger label, and the created ref all read this; computing it twice is how the +// picker once showed local main while branching off something else. +// +// The upstream wins when the branch has one, because branching off the local ref silently +// carries unpushed commits into the new workspace. The daemon sends the resolved ref rather +// than a remote name, so a fork tracking upstream/main branches from upstream/main. +export function defaultBasePickerItem(status: BaseRefCheckoutStatus): PickerItem | null { + const currentBranch = status.currentBranch; + if (!currentBranch) return null; + // COMPAT(checkoutUpstreamRef): added in v0.2.6, remove after 2027-02-01 once the daemon + // floor sends upstreamRef. Daemons that predate it omit the field, which lands on the + // local ref — the base those daemons always used. + const refName = status.upstreamRef ?? `refs/heads/${currentBranch}`; + // The upstream branch can be named differently from the local one, so the row reads the + // ref rather than the branch the user happens to be on. + const name = branchNameFromRef(refName); + return { + kind: "branch", + name, + refName, + accessibilityLabel: status.upstreamRef ? `${name}, upstream branch` : `${name}, local branch`, + }; +} + export function pickerItemToCheckoutRequest( item: PickerItem | null, ): PickerCheckoutRequest | undefined { @@ -159,15 +209,98 @@ export function pickerItemToCheckoutRequest( } } -export function resolveCheckoutRequest( - selectedItem: PickerItem | null, - checkoutStatus: CheckoutStatusPayload, -): PickerCheckoutRequest | undefined { - const selectedCheckoutRequest = pickerItemToCheckoutRequest(selectedItem); - if (selectedCheckoutRequest) return selectedCheckoutRequest; - if (!checkoutStatus.currentBranch) return undefined; +export function prPickerOptionId(number: number): string { + return `${PR_OPTION_PREFIX}${number}`; +} + +export function pickerOptionId(item: PickerItem): string { + return item.kind === "branch" + ? branchPickerOptionId(item.refName) + : prPickerOptionId(item.item.number); +} + +function formatPrLabel(item: Pick): string { + const presentation = getForgePresentation(item.forge ?? "github"); + return `${presentation.numberPrefix}${item.number} ${item.title}`; +} + +export function pickerItemLabel(item: PickerItem): string { + return item.kind === "branch" ? item.name : formatPrLabel(item.item); +} + +export interface PickerOptionData { + options: ComboboxOptionModel[]; + itemById: Map; + selectedOptionId: string; +} + +interface TimedOption { + option: ComboboxOptionModel; + timestamp: number; +} + +export function buildPickerOptionData(input: { + branchDetails: readonly BranchPickerDetail[]; + prItems: readonly ForgeSearchItem[]; + baseItem: PickerItem | null; +}): PickerOptionData { + const itemById = new Map(); + const timedOptions: TimedOption[] = []; + + for (const branch of buildBranchPickerItems(input.branchDetails)) { + if (branch.kind !== "branch") continue; + const id = branchPickerOptionId(branch.refName); + itemById.set(id, branch); + timedOptions.push({ + option: { id, label: pickerItemLabel(branch) }, + timestamp: branch.committerDate ?? 0, + }); + } + + for (const pr of input.prItems) { + if (!pr.headRefName) continue; + const id = prPickerOptionId(pr.number); + itemById.set(id, { kind: "github-pr", item: pr }); + const updatedAtMs = pr.updatedAt ? Date.parse(pr.updatedAt) : 0; + const timestamp = Number.isNaN(updatedAtMs) ? 0 : Math.floor(updatedAtMs / 1000); + timedOptions.push({ option: { id, label: formatPrLabel(pr) }, timestamp }); + } + + const baseItem = input.baseItem; + if (!baseItem) { + timedOptions.sort((a, b) => b.timestamp - a.timestamp); + return { options: timedOptions.map((t) => t.option), itemById, selectedOptionId: "" }; + } + + const selectedOptionId = pickerOptionId(baseItem); + if (!itemById.has(selectedOptionId)) { + // The label lives on the item, so the row and the trigger read it from the same place. + const labeledItem = disambiguate(baseItem, timedOptions); + itemById.set(selectedOptionId, labeledItem); + timedOptions.push({ + option: { id: selectedOptionId, label: pickerItemLabel(labeledItem) }, + // The base sorts first: it is what a workspace is created from unless you pick something. + timestamp: Number.POSITIVE_INFINITY, + }); + } + + timedOptions.sort((a, b) => b.timestamp - a.timestamp); + return { options: timedOptions.map((t) => t.option), itemById, selectedOptionId }; +} + +// Two rows reading "main" would be a coin flip for the user, so the added row says where its +// ref lives. Only when it collides — a lone row needs no qualifier. +function disambiguate(item: PickerItem, existing: readonly TimedOption[]): PickerItem { + if (item.kind !== "branch") return item; + if (!existing.some((entry) => entry.option.label === item.name)) return item; + const qualifier = refQualifier(item.refName); + if (!qualifier) return item; + const name = `${item.name} (${qualifier})`; return { - action: "branch-off", - refName: checkoutStatus.currentBranch, + ...item, + name, + // Accessibility labels are built as `${name}${suffix}`, so re-prefixing keeps the spoken + // label and the visible one the same string. + accessibilityLabel: `${name}${item.accessibilityLabel.slice(item.name.length)}`, }; } diff --git a/packages/app/src/screens/new-workspace-screen.tsx b/packages/app/src/screens/new-workspace-screen.tsx index f60fff2a4..01988b411 100644 --- a/packages/app/src/screens/new-workspace-screen.tsx +++ b/packages/app/src/screens/new-workspace-screen.tsx @@ -61,7 +61,6 @@ import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler"; import type { KeyboardActionId } from "@/keyboard/keyboard-action-dispatcher"; import { useFormPreferences } from "@/hooks/use-form-preferences"; import { useShortcutKeys } from "@/hooks/use-shortcut-keys"; -import { getForgePresentation } from "@/git/forge"; import type { CreateAgentInitialValues } from "@/hooks/use-agent-form-state"; import { generateMessageId } from "@/types/stream"; import { toErrorMessage } from "@/utils/error-messages"; @@ -90,12 +89,14 @@ import { remapDraftCwdToWorkspace, } from "./new-workspace-fork-context"; import { - branchPickerOptionId, - buildBranchPickerItems, - resolveCheckoutRequest, + buildPickerOptionData, + defaultBasePickerItem, + pickerItemLabel, + pickerItemToCheckoutRequest, type BranchPickerDetail, type PickerCheckoutRequest, type PickerItem, + type PickerOptionData, } from "./new-workspace-picker-item"; import { clearPickerPrAttachmentForTargetChange, @@ -169,12 +170,6 @@ interface NewWorkspaceScreenProps { draftId?: string; } -interface PickerOptionData { - options: ComboboxOptionType[]; - itemById: Map; -} - -const PR_OPTION_PREFIX = "github-pr:"; const PROJECT_ICON_FALLBACK_FONT_SIZE = 10; // Stable reference so the keyboard-action handler doesn't re-register each render. const PROJECT_PICK_ACTIONS: readonly KeyboardActionId[] = ["workspace.project.pick"]; @@ -475,10 +470,6 @@ function ProjectOptionItem({ ); } -function prOptionId(number: number): string { - return `${PR_OPTION_PREFIX}${number}`; -} - function NewWorkspacePickerOption({ option, selected, @@ -590,57 +581,10 @@ function AddProjectPickerAction({ onPress }: { onPress: () => void }) { ); } -function formatPrLabel(item: Pick): string { - const presentation = getForgePresentation(item.forge ?? "github"); - return `${presentation.numberPrefix}${item.number} ${item.title}`; -} - -function pickerItemLabel(item: PickerItem): string { - return item.kind === "branch" ? item.name : formatPrLabel(item.item); -} - -function pickerItemTriggerLabel(item: PickerItem): string { - return item.kind === "branch" ? item.name : formatPrLabel(item.item); -} - function newWorkspaceHostOptionTestID(serverId: string): string { return `new-workspace-host-picker-option-${serverId}`; } -function computePickerOptionData( - branchDetails: readonly BranchPickerDetail[], - prItems: ReadonlyArray, -): PickerOptionData { - const idMap = new Map(); - - interface TimedOption { - option: ComboboxOptionType; - timestamp: number; - } - const timedOptions: TimedOption[] = []; - - for (const branch of buildBranchPickerItems(branchDetails)) { - if (branch.kind !== "branch") continue; - const id = branchPickerOptionId(branch.refName); - const option = { id, label: branch.name }; - idMap.set(id, branch); - timedOptions.push({ option, timestamp: branch.committerDate ?? 0 }); - } - - for (const pr of prItems) { - if (!pr.headRefName) continue; - const id = prOptionId(pr.number); - const option = { id, label: formatPrLabel(pr) }; - idMap.set(id, { kind: "github-pr", item: pr }); - const updatedAtMs = pr.updatedAt ? Date.parse(pr.updatedAt) : 0; - const timestamp = Number.isNaN(updatedAtMs) ? 0 : Math.floor(updatedAtMs / 1000); - timedOptions.push({ option, timestamp }); - } - - timedOptions.sort((a, b) => b.timestamp - a.timestamp); - return { options: timedOptions.map((t) => t.option), itemById: idMap }; -} - function IsolationPickerTrigger({ pickerAnchorRef, onPress, @@ -1655,7 +1599,6 @@ export function NewWorkspaceScreen({ cwd: selectedSourceDirectory ?? "", }); - const currentBranch = checkoutStatus?.currentBranch ?? null; const worktreeSupport = selectedProject ? getWorktreeSupportForHostProject({ project: selectedProject, serverId: selectedServerId }) : "unsupported"; @@ -1709,25 +1652,23 @@ export function NewWorkspaceScreen({ return githubPrSearchQuery.data?.items ?? []; }, [forgeSearchAuthenticated, githubPrSearchQuery.data?.items]); - const { options, itemById }: PickerOptionData = useMemo( - () => computePickerOptionData(branchDetails, prItems), - [branchDetails, prItems], + const baseItem = useMemo( + () => selectedItem ?? (checkoutStatus ? defaultBasePickerItem(checkoutStatus) : null), + [checkoutStatus, selectedItem], + ); + const { options, itemById, selectedOptionId }: PickerOptionData = useMemo( + () => + buildPickerOptionData({ + branchDetails, + prItems, + baseItem, + }), + [baseItem, branchDetails, prItems], ); const triggerLabel = useMemo(() => { - if (selectedItem) return pickerItemTriggerLabel(selectedItem); - return currentBranch ?? "main"; - }, [currentBranch, selectedItem]); - - const selectedOptionId = useMemo(() => { - if (!selectedItem) { - if (!currentBranch) return ""; - const exactLocalId = branchPickerOptionId(`refs/heads/${currentBranch}`); - return itemById.has(exactLocalId) ? exactLocalId : branchPickerOptionId(currentBranch); - } - return selectedItem.kind === "branch" - ? branchPickerOptionId(selectedItem.refName) - : prOptionId(selectedItem.item.number); - }, [currentBranch, itemById, selectedItem]); + const displayItem = itemById.get(selectedOptionId); + return displayItem ? pickerItemLabel(displayItem) : "main"; + }, [itemById, selectedOptionId]); const selectPickerItem = useCallback( (item: PickerItem) => { const nextAttachments = syncPickerPrAttachment({ @@ -1929,15 +1870,17 @@ export function NewWorkspaceScreen({ } const connectedClient = withConnectedClient(); const createsWorktree = !supportsWorkspaceMultiplicity || effectiveIsolation === "worktree"; - const checkoutRequest = createsWorktree - ? resolveCheckoutRequest( - selectedItem, - await ensureCheckoutStatus({ - queryClient, - client: connectedClient, - serverId: selectedServerId, - cwd: selectedSourceDirectory, - }), + const checkoutStatusForCreate = createsWorktree + ? await ensureCheckoutStatus({ + queryClient, + client: connectedClient, + serverId: selectedServerId, + cwd: selectedSourceDirectory, + }) + : null; + const checkoutRequest = checkoutStatusForCreate + ? pickerItemToCheckoutRequest( + selectedItem ?? defaultBasePickerItem(checkoutStatusForCreate), ) : undefined; const normalizedWorkspace = supportsWorkspaceMultiplicity diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index be7579257..09b536d96 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -4060,6 +4060,13 @@ const CheckoutStatusCommonSchema = z.object({ cwd: z.string(), error: CheckoutErrorSchema.nullable(), requestId: z.string(), + // The full ref currentBranch tracks, as git resolves `@{upstream}`: + // "refs/remotes/origin/main", "refs/remotes/upstream/main" on a fork, or a + // "refs/heads/..." ref for a branch tracking a local branch. Null when there is no + // upstream. Clients use it verbatim — the remote is not necessarily origin and the + // upstream branch name is not necessarily currentBranch, so composing one is wrong. + // aheadOfOrigin/behindOfOrigin are measured against exactly this ref. + upstreamRef: z.string().nullable().optional(), }); const CheckoutStatusNotGitSchema = CheckoutStatusCommonSchema.extend({ diff --git a/packages/server/src/server/checkout/status-projection.ts b/packages/server/src/server/checkout/status-projection.ts index 28c4095c0..b9405be3e 100644 --- a/packages/server/src/server/checkout/status-projection.ts +++ b/packages/server/src/server/checkout/status-projection.ts @@ -33,6 +33,7 @@ export function buildCheckoutStatusPayloadFromSnapshot({ isDirty: null, baseRef: null, aheadBehind: null, + upstreamRef: null, aheadOfOrigin: null, behindOfOrigin: null, hasRemote: false, @@ -61,6 +62,7 @@ export function buildCheckoutStatusPayloadFromSnapshot({ isDirty: snapshot.git.isDirty, baseRef: snapshot.git.baseRef, aheadBehind: snapshot.git.aheadBehind ?? null, + upstreamRef: snapshot.git.upstreamRef ?? null, aheadOfOrigin: snapshot.git.aheadOfOrigin ?? null, behindOfOrigin: snapshot.git.behindOfOrigin ?? null, hasRemote: snapshot.git.hasRemote, @@ -80,6 +82,7 @@ export function buildCheckoutStatusPayloadFromSnapshot({ isDirty: snapshot.git.isDirty, baseRef: snapshot.git.baseRef ?? null, aheadBehind: snapshot.git.aheadBehind ?? null, + upstreamRef: snapshot.git.upstreamRef ?? null, aheadOfOrigin: snapshot.git.aheadOfOrigin ?? null, behindOfOrigin: snapshot.git.behindOfOrigin ?? null, hasRemote: snapshot.git.hasRemote, diff --git a/packages/server/src/server/session.test.ts b/packages/server/src/server/session.test.ts index 0b0568686..9322e8888 100644 --- a/packages/server/src/server/session.test.ts +++ b/packages/server/src/server/session.test.ts @@ -3373,6 +3373,7 @@ describe("session checkout status handling", () => { aheadBehind: { ahead: 2, behind: 1 }, aheadOfOrigin: 2, behindOfOrigin: 1, + upstreamRef: null, hasRemote: true, remoteUrl: "https://github.com/getpaseo/paseo.git", isPaseoOwnedWorktree: false, diff --git a/packages/server/src/server/websocket-server.ts b/packages/server/src/server/websocket-server.ts index fa70b9905..c807554bc 100644 --- a/packages/server/src/server/websocket-server.ts +++ b/packages/server/src/server/websocket-server.ts @@ -178,6 +178,7 @@ function createFallbackWorkspaceGitSnapshot(cwd: string): WorkspaceGitRuntimeSna isDirty: null, baseRef: null, aheadBehind: null, + upstreamRef: null, aheadOfOrigin: null, behindOfOrigin: null, hasRemote: false, diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index 0fbf9d30f..f74c9670c 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -80,6 +80,7 @@ export interface WorkspaceGitRuntimeSnapshot { isDirty: boolean | null; baseRef: string | null; aheadBehind: { ahead: number; behind: number } | null; + upstreamRef: string | null; aheadOfOrigin: number | null; behindOfOrigin: number | null; hasRemote: boolean; @@ -2182,6 +2183,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { : (target.latestGit?.isDirty ?? checkoutStatus.isDirty), baseRef: checkoutStatus.baseRef, aheadBehind: checkoutStatus.aheadBehind, + upstreamRef: checkoutStatus.upstreamRef, aheadOfOrigin: checkoutStatus.aheadOfOrigin, behindOfOrigin: checkoutStatus.behindOfOrigin, hasRemote: checkoutStatus.hasRemote, @@ -2598,6 +2600,7 @@ function buildNotGitSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot { isDirty: null, baseRef: null, aheadBehind: null, + upstreamRef: null, aheadOfOrigin: null, behindOfOrigin: null, hasRemote: false, diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index c5f8db167..0c6978407 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -737,29 +737,107 @@ const x = 1; execFileSync("git", ["push"], { cwd: cloneDir }); execFileSync("git", ["fetch", "origin"], { cwd: repoDir }); - const facts = await getCheckoutSnapshotFacts(repoDir); startGitCommandMetrics(); + const facts = await getCheckoutSnapshotFacts(repoDir); const divergedStatus = await getCheckoutStatus(repoDir, { facts }); const metrics = stopGitCommandMetrics(); const upstreamCountCommands = metrics.commands.filter( - (command) => command.args[0] === "rev-list" && command.args.join(" ").includes("main"), + (command) => + command.args[0] === "for-each-ref" && command.args.join(" ").includes("%(upstream)"), ); expect(divergedStatus.isGit).toBe(true); if (!divergedStatus.isGit) { return; } + expect(divergedStatus.upstreamRef).toBe("refs/remotes/origin/main"); expect(divergedStatus.aheadOfOrigin).toBe(3); expect(divergedStatus.behindOfOrigin).toBe(2); expect(upstreamCountCommands).toHaveLength(1); expect(upstreamCountCommands[0]?.args).toEqual([ - "rev-list", - "--left-right", - "--count", - "main...origin/main", + "for-each-ref", + "--format=%(upstream)%00%(upstream:track,nobracket)", + "refs/heads/main", ]); }); + it("reports the fork remote, not origin, when the branch tracks a second remote", async () => { + setupRemoteTrackingMain(repoDir, tempDir); + const forkDir = join(tempDir, "fork.git"); + execFileSync("git", ["init", "--bare", "-b", "main", forkDir]); + execFileSync("git", ["remote", "add", "upstream", forkDir], { cwd: repoDir }); + execFileSync("git", ["push", "-u", "upstream", "main"], { cwd: repoDir }); + + const status = await getCheckoutStatus(repoDir); + expect(status.isGit).toBe(true); + if (!status.isGit) { + return; + } + expect(status.upstreamRef).toBe("refs/remotes/upstream/main"); + }); + + it("reports the upstream branch name when it differs from the local branch name", async () => { + setupRemoteTrackingMain(repoDir, tempDir); + execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir }); + commitFile(repoDir, "feature.txt", "feature\n", "feature commit"); + execFileSync("git", ["push", "-u", "origin", "HEAD:other-name"], { cwd: repoDir }); + + const status = await getCheckoutStatus(repoDir); + expect(status.isGit).toBe(true); + if (!status.isGit) { + return; + } + expect(status.upstreamRef).toBe("refs/remotes/origin/other-name"); + }); + + it("reports the upstream when the repo's only remote is not origin", async () => { + const forkDir = join(tempDir, "fork.git"); + execFileSync("git", ["init", "--bare", "-b", "main", forkDir]); + execFileSync("git", ["remote", "add", "upstream", forkDir], { cwd: repoDir }); + execFileSync("git", ["push", "-u", "upstream", "main"], { cwd: repoDir }); + + const status = await getCheckoutStatus(repoDir); + expect(status.isGit).toBe(true); + if (!status.isGit) { + return; + } + expect(status.upstreamRef).toBe("refs/remotes/upstream/main"); + expect(status.aheadOfOrigin).toBe(0); + }); + + // A legal local branch named "origin/main" shadows the short form "origin/main" in git's + // ref resolution. The counts and the reported ref have to describe the same commit. + it("is not fooled by a local branch named like the remote-tracking ref", async () => { + setupRemoteTrackingMain(repoDir, tempDir); + commitFile(repoDir, "shadow.txt", "shadow\n", "shadow commit"); + // Pointed at HEAD, so comparing against the shadow reports 0 ahead while the real + // remote-tracking ref reports 1. + execFileSync("git", ["branch", "origin/main", "main"], { cwd: repoDir }); + + const status = await getCheckoutStatus(repoDir); + expect(status.isGit).toBe(true); + if (!status.isGit) { + return; + } + expect(status.upstreamRef).toBe("refs/remotes/origin/main"); + expect(status.aheadOfOrigin).toBe(1); + expect(status.behindOfOrigin).toBe(0); + }); + + it("reports no upstream ref for a branch that was never pushed", async () => { + setupRemoteTrackingMain(repoDir, tempDir); + execFileSync("git", ["checkout", "-b", "local-only"], { cwd: repoDir }); + commitFile(repoDir, "local-only.txt", "local\n", "local commit"); + + const status = await getCheckoutStatus(repoDir); + expect(status.isGit).toBe(true); + if (!status.isGit) { + return; + } + expect(status.upstreamRef).toBeNull(); + expect(status.aheadOfOrigin).toBeNull(); + }); + it("reports a PR worktree as not ahead when its branch is pushed to the configured PR remote", async () => { setupRemoteTrackingMain(repoDir, tempDir); const prRemoteDir = join(tempDir, "pr-remote.git"); @@ -3396,7 +3474,7 @@ const x = 1; await expect( getCheckoutDiff(worktree.worktreePath, { mode: "base", baseRef: "other" }, { paseoHome }), - ).rejects.toThrow("Base ref mismatch: stored main, requested other"); + ).rejects.toThrow("Base ref mismatch: stored refs/heads/main, requested other"); }); it("excludes dirty working tree changes from Paseo worktree base diffs", async () => { diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index bd83a8dda..7dcb8bf28 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -30,6 +30,7 @@ import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-pa import { runGitCommand } from "./run-git-command.js"; import { isPaseoOwnedWorktreeCwd, resolvePaseoWorktreesBaseRoot } from "./worktree.js"; import { + branchNameFromRef, getPaseoWorktreeChangeRequestHintForBranch, type PaseoWorktreeMetadata, readPaseoWorktreeMetadata, @@ -786,6 +787,10 @@ export interface CheckoutStatusGitNonPaseo { isDirty: boolean; baseRef: string | null; aheadBehind: AheadBehind | null; + // Remote-tracking ref for currentBranch, e.g. "refs/remotes/origin/main". Null when the + // branch has no upstream or git could not resolve it. aheadOfOrigin/behindOfOrigin are + // measured against exactly this ref. + upstreamRef: string | null; aheadOfOrigin: number | null; behindOfOrigin: number | null; hasRemote: boolean; @@ -801,6 +806,7 @@ export interface CheckoutStatusGitPaseo { isDirty: boolean; baseRef: string; aheadBehind: AheadBehind | null; + upstreamRef: string | null; aheadOfOrigin: number | null; behindOfOrigin: number | null; hasRemote: boolean; @@ -859,6 +865,7 @@ export type CheckoutSnapshotFacts = comparisonBaseRef: string | null; branchRemoteName: string | null; branchMergeRef: string | null; + upstreamStatus: UpstreamStatus | null; pullRequestLookupTarget: PullRequestStatusLookupTarget | null; }; @@ -1153,8 +1160,14 @@ async function getPaseoWorktreeForCwd( }; } +// Worktrees created before baseRef existed only stored the stripped name; it resolves +// local-first, which is the base they were actually cut from. +function storedBaseRefFromMetadata(metadata: PaseoWorktreeMetadata | null): string | null { + return metadata?.baseRef ?? metadata?.baseRefName ?? null; +} + function readPaseoWorktreeBaseRef(worktreeRoot: string): string | null { - return readPaseoWorktreeMetadata(worktreeRoot)?.baseRefName ?? null; + return storedBaseRefFromMetadata(readPaseoWorktreeMetadata(worktreeRoot)); } async function getStoredBaseRefForCwd( @@ -1205,12 +1218,37 @@ async function resolveBaseRefForCwd( }; } +// The worktree stores the exact ref it was cut from ("refs/remotes/upstream/main") while +// callers still send its display name ("main"). A different qualified ref retains its own +// identity even when it has the same branch name. +function isSameBaseRef(stored: string, requested: string): boolean { + return stored === requested || branchNameFromRef(stored) === requested; +} + // Names both refs rather than labelling either one correct: a caller's ref can be stale, but the // stored ref can equally be wrong, so the message states the two facts and leaves the diagnosis open. function baseRefMismatchError(refs: { stored: string; requested: string }): Error { return new Error(`Base ref mismatch: stored ${refs.stored}, requested ${refs.requested}`); } +function resolveOperationBaseRef(input: { + storedBaseRef: string | null; + resolvedBaseRef: string | null; + requestedBaseRef?: string; +}): string | null { + if ( + input.storedBaseRef && + input.requestedBaseRef && + !isSameBaseRef(input.storedBaseRef, input.requestedBaseRef) + ) { + throw baseRefMismatchError({ + stored: input.storedBaseRef, + requested: input.requestedBaseRef, + }); + } + return input.storedBaseRef ?? input.requestedBaseRef ?? input.resolvedBaseRef; +} + async function isWorkingTreeDirty(cwd: string, context?: CheckoutContext): Promise { const { stdout } = await runGitCommand(["status", "--porcelain"], { cwd, @@ -1450,29 +1488,25 @@ async function resolveBaseRef(repoRoot: string): Promise { return resolveRepositoryDefaultBranch(repoRoot); } -function normalizeLocalBranchRefName(input: string): string { - if (input.startsWith("refs/remotes/origin/")) { - return input.slice("refs/remotes/origin/".length); - } - if (input.startsWith("refs/heads/")) { - return input.slice("refs/heads/".length); - } - if (input.startsWith("origin/")) { - return input.slice("origin/".length); - } - return input; -} - interface ComparisonBaseRefName { localName: string; originRef: string; } function normalizeComparisonBaseRefName(input: string): ComparisonBaseRefName { - const localName = normalizeLocalBranchRefName(input); + const localName = branchNameFromRef(input); return { localName, originRef: `origin/${localName}` }; } +function resolveMergeTargetBranch(baseRef: string): string { + const remotePrefix = "refs/remotes/"; + const originPrefix = `${remotePrefix}origin/`; + if (baseRef.startsWith(remotePrefix) && !baseRef.startsWith(originPrefix)) { + throw new Error(`No local merge target is recorded for base ref ${baseRef}`); + } + return branchNameFromRef(baseRef); +} + async function doesGitRefExist( cwd: string, fullRef: string, @@ -1487,11 +1521,23 @@ async function doesGitRefExist( return result.exitCode === 0; } +function isQualifiedBranchRef(ref: string): boolean { + return ref.startsWith("refs/heads/") || ref.startsWith("refs/remotes/"); +} + async function resolveBestComparisonBaseRef( cwd: string, baseRef: string, context?: CheckoutContext, ): Promise { + // A fully qualified branch ref names the exact commit stream to compare against. Bare names + // keep going through the local-vs-origin heuristic below for legacy worktree metadata. + if (isQualifiedBranchRef(baseRef)) { + if (await doesGitRefExist(cwd, baseRef, context)) { + return baseRef; + } + throw new Error(`Base ref not found: ${baseRef}`); + } const normalized = normalizeComparisonBaseRefName(baseRef); const [hasLocal, hasOrigin] = await Promise.all([ doesGitRefExist(cwd, `refs/heads/${normalized.localName}`, context), @@ -1512,7 +1558,14 @@ async function resolveBestComparisonBaseRef( throw new Error(`Base branch not found locally or on origin: ${refName}`); } -async function resolveMostAheadBaseRef(cwd: string, normalizedBaseRef: string): Promise { +async function resolveMostAheadBaseRef(cwd: string, baseRef: string): Promise { + if (isQualifiedBranchRef(baseRef)) { + if (await doesGitRefExist(cwd, baseRef)) { + return baseRef; + } + throw new Error(`Base ref not found: ${baseRef}`); + } + const normalizedBaseRef = branchNameFromRef(baseRef); const [hasLocal, hasOrigin] = await Promise.all([ doesGitRefExist(cwd, `refs/heads/${normalizedBaseRef}`), doesGitRefExist(cwd, `refs/remotes/origin/${normalizedBaseRef}`), @@ -1551,7 +1604,7 @@ async function getAheadBehind( currentBranch: string, context?: CheckoutContext, ): Promise { - const normalizedBaseRef = normalizeLocalBranchRefName(baseRef); + const normalizedBaseRef = branchNameFromRef(baseRef); if (!normalizedBaseRef || !currentBranch || normalizedBaseRef === currentBranch) { return null; } @@ -1575,48 +1628,32 @@ async function getAheadBehind( return { ahead, behind }; } -async function getConfiguredUpstreamRef( - cwd: string, - currentBranch: string, - context?: CheckoutContext, -): Promise { - const remoteName = - context?.facts?.isGit && context.facts.currentBranch === currentBranch - ? context.facts.branchRemoteName - : await getGitConfigValue(cwd, `branch.${currentBranch}.remote`, context); - if (!remoteName) { - return null; - } - - const mergeRef = - context?.facts?.isGit && context.facts.currentBranch === currentBranch - ? context.facts.branchMergeRef - : await getGitConfigValue(cwd, `branch.${currentBranch}.merge`, context); - const upstreamBranch = parseBranchMergeHeadRef(mergeRef); - return upstreamBranch ? `${remoteName}/${upstreamBranch}` : null; +interface UpstreamStatus { + ref: string; + aheadBehind: AheadBehind; } -async function getOriginAheadBehind( +async function getUpstreamStatus( cwd: string, currentBranch: string, context?: CheckoutContext, -): Promise { - if (!currentBranch) { - return null; - } - const upstreamRef = await getConfiguredUpstreamRef(cwd, currentBranch, context); - if (!upstreamRef) { - return null; - } +): Promise { try { const { stdout } = await runGitCommand( - ["rev-list", "--left-right", "--count", `${currentBranch}...${upstreamRef}`], + [ + "for-each-ref", + "--format=%(upstream)%00%(upstream:track,nobracket)", + `refs/heads/${currentBranch}`, + ], { cwd, envOverlay: READ_ONLY_GIT_ENV, logger: context?.logger }, ); - const [aheadRaw, behindRaw] = stdout.trim().split(/\s+/); - const ahead = Number.parseInt(aheadRaw ?? "", 10); - const behind = Number.parseInt(behindRaw ?? "", 10); - return Number.isNaN(ahead) || Number.isNaN(behind) ? null : { ahead, behind }; + const [ref = "", track = ""] = stdout.trim().split("\0", 2); + if (!ref || track === "gone") { + return null; + } + const ahead = Number.parseInt(track.match(/ahead (\d+)/)?.[1] ?? "0", 10); + const behind = Number.parseInt(track.match(/behind (\d+)/)?.[1] ?? "0", 10); + return { ref, aheadBehind: { ahead, behind } }; } catch { return null; } @@ -1674,9 +1711,7 @@ function buildPullRequestLookupTargetFromBranchConfig( const originRepo = parseRepositoryIdentityFromRemote(input.originRemoteUrl); const isSameRepo = areSameGitHubRepository(remoteRepo, originRepo); const headRepositoryOwner = remoteRepo && !isSameRepo ? remoteRepo.split("/")[0] : null; - const normalizedBaseRef = input.resolvedBaseRef - ? normalizeLocalBranchRefName(input.resolvedBaseRef) - : null; + const normalizedBaseRef = input.resolvedBaseRef ? branchNameFromRef(input.resolvedBaseRef) : null; if ( trackedHeadRef === normalizedBaseRef && !doesLocalBranchNameIdentifyTrackedHead( @@ -1740,9 +1775,7 @@ function buildPullRequestLookupTargetFromPushConfig( : null; const isSameRepo = areSameGitHubRepository(remoteRepo, originRepo); const headRepositoryOwner = remoteRepo && !isSameRepo ? remoteRepo.split("/")[0] : null; - const normalizedBaseRef = input.resolvedBaseRef - ? normalizeLocalBranchRefName(input.resolvedBaseRef) - : null; + const normalizedBaseRef = input.resolvedBaseRef ? branchNameFromRef(input.resolvedBaseRef) : null; if (pushedHeadRef === normalizedBaseRef && !headRepositoryOwner) { return null; } @@ -1855,7 +1888,7 @@ export async function getCheckoutSnapshotFacts( const paseoWorktreeMetadata = inspected.paseoWorktree.isPaseoOwnedWorktree ? readPaseoWorktreeMetadata(inspected.paseoWorktree.worktreeRoot) : null; - const storedBaseRef = paseoWorktreeMetadata?.baseRefName ?? null; + const storedBaseRef = storedBaseRefFromMetadata(paseoWorktreeMetadata); const resolvedBaseRef = storedBaseRef ?? (await resolveBaseRef(cwd)); const mainRepoRoot = await getMainRepoRootFromCommonDir( cwd, @@ -1866,7 +1899,7 @@ export async function getCheckoutSnapshotFacts( if ( resolvedBaseRef && inspected.currentBranch && - normalizeLocalBranchRefName(resolvedBaseRef) !== inspected.currentBranch + branchNameFromRef(resolvedBaseRef) !== inspected.currentBranch ) { comparisonBaseRef = await resolveBestComparisonBaseRef(cwd, resolvedBaseRef, context).catch( () => null, @@ -1876,6 +1909,9 @@ export async function getCheckoutSnapshotFacts( let branchRemoteName: string | null = null; let branchMergeRef: string | null = null; let branchRemoteUrl: string | null = null; + const upstreamStatusPromise = inspected.currentBranch + ? getUpstreamStatus(cwd, inspected.currentBranch, context) + : Promise.resolve(null); if (inspected.currentBranch) { branchRemoteName = await getGitConfigValue( cwd, @@ -1919,6 +1955,7 @@ export async function getCheckoutSnapshotFacts( pullRequestLookupTarget, context, ); + const upstreamStatus = await upstreamStatusPromise; return { isGit: true, @@ -1934,6 +1971,7 @@ export async function getCheckoutSnapshotFacts( comparisonBaseRef, branchRemoteName, branchMergeRef, + upstreamStatus, pullRequestLookupTarget, }; } @@ -2097,16 +2135,18 @@ export async function getCheckoutStatus( const baseRef = facts.resolvedBaseRef; const mainRepoRoot = facts.mainRepoRoot; const factsContext = { ...context, facts }; - const [aheadBehind, originAheadBehind] = await Promise.all([ + const aheadBehind = baseRef && currentBranch - ? getAheadBehind(cwd, baseRef, currentBranch, factsContext) - : Promise.resolve(null), - hasRemote && currentBranch - ? getOriginAheadBehind(cwd, currentBranch, factsContext) - : Promise.resolve(null), - ]); - const aheadOfOrigin = originAheadBehind?.ahead ?? null; - const behindOfOrigin = originAheadBehind?.behind ?? null; + ? await getAheadBehind(cwd, baseRef, currentBranch, factsContext) + : null; + const upstreamStatus = facts.upstreamStatus; + // The wire carries the display name: clients label the base with it and send it back to + // request diffs and merges. The exact ref stays in worktree.json and in facts, where the + // comparisons and actions read it. + const displayBaseRef = baseRef ? branchNameFromRef(baseRef) : null; + const upstreamRef = upstreamStatus?.ref ?? null; + const aheadOfOrigin = upstreamStatus?.aheadBehind.ahead ?? null; + const behindOfOrigin = upstreamStatus?.aheadBehind.behind ?? null; if (paseoWorktree.isPaseoOwnedWorktree && baseRef) { return { @@ -2115,8 +2155,9 @@ export async function getCheckoutStatus( mainRepoRoot: mainRepoRoot ?? worktreeRoot, currentBranch, isDirty, - baseRef, + baseRef: displayBaseRef ?? baseRef, aheadBehind, + upstreamRef, aheadOfOrigin, behindOfOrigin, hasRemote, @@ -2132,8 +2173,9 @@ export async function getCheckoutStatus( mainRepoRoot && resolve(mainRepoRoot) !== resolve(worktreeRoot) ? mainRepoRoot : null, currentBranch, isDirty, - baseRef, + baseRef: displayBaseRef, aheadBehind, + upstreamRef, aheadOfOrigin, behindOfOrigin, hasRemote, @@ -2345,11 +2387,18 @@ async function tryResolveCheckoutCommitsBaseRef( if (!baseRef) { return null; } - const normalizedBaseRef = normalizeLocalBranchRefName(baseRef); + const normalizedBaseRef = branchNameFromRef(baseRef); if (!normalizedBaseRef || normalizedBaseRef === currentBranch) { return null; } - return resolveMostAheadBaseRef(cwd, normalizedBaseRef).catch(() => null); + try { + return await resolveMostAheadBaseRef(cwd, baseRef); + } catch (error) { + if (isQualifiedBranchRef(baseRef)) { + throw error; + } + return null; + } } export async function listCheckoutCommits({ @@ -2365,7 +2414,7 @@ export async function listCheckoutCommits({ } const { resolvedBaseRef } = await resolveBaseRefForCwd(cwd, context); - const normalizedBaseRef = resolvedBaseRef ? normalizeLocalBranchRefName(resolvedBaseRef) : null; + const normalizedBaseRef = resolvedBaseRef ? branchNameFromRef(resolvedBaseRef) : null; let comparisonBaseRef = await tryResolveCheckoutCommitsBaseRef( cwd, resolvedBaseRef, @@ -2991,13 +3040,14 @@ async function resolveCheckoutDiffRefs( return { baseRef: "HEAD", includeUntracked: true }; } const { storedBaseRef, resolvedBaseRef } = await resolveBaseRefForCwd(cwd, context); - const baseRef = compare.baseRef ?? resolvedBaseRef; + const baseRef = resolveOperationBaseRef({ + storedBaseRef, + resolvedBaseRef, + requestedBaseRef: compare.baseRef, + }); if (!baseRef) { return null; } - if (storedBaseRef && compare.baseRef && compare.baseRef !== storedBaseRef) { - throw baseRefMismatchError({ stored: storedBaseRef, requested: compare.baseRef }); - } const bestBaseRef = await resolveBestComparisonBaseRef(cwd, baseRef); return { baseRef: (await tryResolveMergeBase(cwd, bestBaseRef)) ?? bestBaseRef, @@ -3209,18 +3259,18 @@ export async function mergeToBase( await requireGitRepo(cwd); const currentBranch = await getCurrentBranch(cwd); const { storedBaseRef, resolvedBaseRef } = await resolveBaseRefForCwd(cwd, context); - const baseRef = options.baseRef ?? resolvedBaseRef; + const baseRef = resolveOperationBaseRef({ + storedBaseRef, + resolvedBaseRef, + requestedBaseRef: options.baseRef, + }); if (!baseRef) { throw new Error("Unable to determine base branch for merge"); } - if (storedBaseRef && options.baseRef && options.baseRef !== storedBaseRef) { - throw baseRefMismatchError({ stored: storedBaseRef, requested: options.baseRef }); - } if (!currentBranch) { throw new Error("Unable to determine current branch for merge"); } - let normalizedBaseRef = baseRef; - normalizedBaseRef = normalizeLocalBranchRefName(normalizedBaseRef); + const normalizedBaseRef = resolveMergeTargetBranch(baseRef); const currentWorktreeRoot = (await getWorktreeRoot(cwd, context)) ?? cwd; if (normalizedBaseRef === currentBranch) { return currentWorktreeRoot; @@ -3285,13 +3335,14 @@ export async function mergeFromBase( } const { storedBaseRef, resolvedBaseRef } = await resolveBaseRefForCwd(cwd, context); - const baseRef = options.baseRef ?? resolvedBaseRef; + const baseRef = resolveOperationBaseRef({ + storedBaseRef, + resolvedBaseRef, + requestedBaseRef: options.baseRef, + }); if (!baseRef) { throw new Error("Unable to determine base branch for merge"); } - if (storedBaseRef && options.baseRef && options.baseRef !== storedBaseRef) { - throw baseRefMismatchError({ stored: storedBaseRef, requested: options.baseRef }); - } const requireCleanTarget = options.requireCleanTarget ?? true; if (requireCleanTarget) { @@ -3304,8 +3355,7 @@ export async function mergeFromBase( } } - const normalizedBaseRef = normalizeLocalBranchRefName(baseRef); - const bestBaseRef = await resolveMostAheadBaseRef(cwd, normalizedBaseRef); + const bestBaseRef = await resolveMostAheadBaseRef(cwd, baseRef); if (bestBaseRef === currentBranch) { return; } @@ -3619,17 +3669,18 @@ export async function createPullRequest( const head = options.head ?? (await getCurrentBranch(cwd)); const { storedBaseRef, resolvedBaseRef } = await resolveBaseRefForCwd(cwd, context); - const base = options.base ?? resolvedBaseRef; + const base = resolveOperationBaseRef({ + storedBaseRef, + resolvedBaseRef, + requestedBaseRef: options.base, + }); if (!head) { throw new Error("Unable to determine head branch for PR"); } if (!base) { throw new Error("Unable to determine base branch for PR"); } - const normalizedBase = normalizeLocalBranchRefName(base); - if (storedBaseRef && options.base && options.base !== storedBaseRef) { - throw baseRefMismatchError({ stored: storedBaseRef, requested: options.base }); - } + const normalizedBase = branchNameFromRef(base); // The push deliberately happens before the adapter resolves the target // repository: slug resolution is adapter-internal (e.g. `gh repo view`, which diff --git a/packages/server/src/utils/worktree-metadata.ts b/packages/server/src/utils/worktree-metadata.ts index 7261ea3b2..4c44aaf0f 100644 --- a/packages/server/src/utils/worktree-metadata.ts +++ b/packages/server/src/utils/worktree-metadata.ts @@ -9,15 +9,20 @@ const ChangeRequestLookupTargetSchema = z.object({ localBranchName: z.string().min(1).optional(), }); +// baseRefName is the display name; baseRef is the exact ref the worktree was cut from +// ("refs/remotes/upstream/main"). baseRef is optional because worktrees written before it +// existed only have the name — there are no migrations, so readers fall back. const PaseoWorktreeMetadataV1Schema = z.object({ version: z.literal(1), baseRefName: z.string().min(1), + baseRef: z.string().min(1).optional(), changeRequestLookupTarget: ChangeRequestLookupTargetSchema.optional(), }); const PaseoWorktreeMetadataV2Schema = z.object({ version: z.literal(2), baseRefName: z.string().min(1), + baseRef: z.string().min(1).optional(), changeRequestLookupTarget: ChangeRequestLookupTargetSchema.optional(), firstAgentBranchAutoName: z .discriminatedUnion("status", [ @@ -133,44 +138,72 @@ export function getPaseoWorktreeMetadataPath(worktreeRoot: string): string { return join(gitDir, "paseo", "worktree.json"); } -export function normalizeBaseRefName(input: string): string { - const trimmed = input.trim(); - if (!trimmed) { - throw new Error("Base branch is required"); - } +const REMOTE_TRACKING_PREFIX = "refs/remotes/"; + +/** + * The human-readable branch name behind a ref. Display and legacy identity only — it cannot + * round-trip, so anything that has to resolve to a commit keeps the exact ref instead. + * + * refs/remotes// works for any remote, not just origin. Git allows slashes in + * remote names, so refs/remotes/a/b/c is ambiguous and the first segment is read as the + * remote: slashes are everywhere in branch names and rare in remote names. A remote genuinely + * named "team/upstream" therefore displays as "upstream/main" rather than "main"; the exact + * ref is unaffected, which is why this is display-only. + */ +export function branchNameFromRef(ref: string): string { + const trimmed = ref.trim(); if (trimmed.startsWith("refs/heads/")) { return trimmed.slice("refs/heads/".length); } - if (trimmed.startsWith("refs/remotes/origin/")) { - return trimmed.slice("refs/remotes/origin/".length); + if (trimmed.startsWith(REMOTE_TRACKING_PREFIX)) { + const remainder = trimmed.slice(REMOTE_TRACKING_PREFIX.length); + const separator = remainder.indexOf("/"); + return separator === -1 ? remainder : remainder.slice(separator + 1); } + // Short form. It cannot be generalized to any remote the way the qualified form can: + // without the remote list, "feature/x" is indistinguishable from "/x". if (trimmed.startsWith("origin/")) { return trimmed.slice("origin/".length); } return trimmed; } +export function normalizeBaseRefName(input: string): string { + const trimmed = input.trim(); + if (!trimmed) { + throw new Error("Base branch is required"); + } + return branchNameFromRef(trimmed); +} + +function assertValidBaseRef(value: string): void { + if (value === "HEAD") { + throw new Error("Base branch cannot be HEAD"); + } + if (value.includes("..") || value.includes("@{")) { + throw new Error(`Invalid base branch: ${value}`); + } +} + export function writePaseoWorktreeMetadata( worktreeRoot: string, options: { baseRefName: string; + baseRef?: string; changeRequestLookupTarget?: PaseoWorktreeChangeRequestHint; }, ): void { const baseRefName = normalizeBaseRefName(options.baseRefName); - if (baseRefName === "HEAD") { - throw new Error("Base branch cannot be HEAD"); - } - if (baseRefName.includes("..") || baseRefName.includes("@{")) { - throw new Error(`Invalid base branch: ${baseRefName}`); - } - if (!/^[0-9A-Za-z._/-]+$/.test(baseRefName)) { - throw new Error(`Invalid base branch: ${baseRefName}`); + assertValidBaseRef(baseRefName); + const baseRef = options.baseRef?.trim(); + if (baseRef) { + assertValidBaseRef(baseRef); } const metadata: PaseoWorktreeMetadata = { version: 1, baseRefName, + ...(baseRef ? { baseRef } : {}), ...(options.changeRequestLookupTarget ? { changeRequestLookupTarget: options.changeRequestLookupTarget } : {}), @@ -192,14 +225,8 @@ export function writePaseoWorktreeRuntimeMetadata( } const next: PaseoWorktreeMetadata = { + ...current, version: 2, - baseRefName: current.baseRefName, - ...(current.changeRequestLookupTarget - ? { changeRequestLookupTarget: current.changeRequestLookupTarget } - : {}), - ...(current.version === 2 && current.firstAgentBranchAutoName - ? { firstAgentBranchAutoName: current.firstAgentBranchAutoName } - : {}), runtime: { worktreePort: options.worktreePort, }, @@ -222,16 +249,12 @@ export function writePaseoWorktreeFirstAgentBranchAutoNameMetadata( } writePaseoWorktreeMetadataFile(worktreeRoot, { + ...current, version: 2, - baseRefName: current.baseRefName, - ...(current.changeRequestLookupTarget - ? { changeRequestLookupTarget: current.changeRequestLookupTarget } - : {}), firstAgentBranchAutoName: { status: "pending", placeholderBranchName, }, - ...(current.version === 2 && current.runtime ? { runtime: current.runtime } : {}), }); } @@ -245,17 +268,12 @@ export function markPaseoWorktreeFirstAgentBranchAutoNameAttempted( } const next: PaseoWorktreeMetadata = { - version: 2, - baseRefName: current.baseRefName, - ...(current.changeRequestLookupTarget - ? { changeRequestLookupTarget: current.changeRequestLookupTarget } - : {}), + ...current, firstAgentBranchAutoName: { status: "attempted", placeholderBranchName: current.firstAgentBranchAutoName.placeholderBranchName, attemptedAt: options.attemptedAt ?? new Date().toISOString(), }, - ...(current.runtime ? { runtime: current.runtime } : {}), }; writePaseoWorktreeMetadataFile(worktreeRoot, next); return next; diff --git a/packages/server/src/utils/worktree.posix.test.ts b/packages/server/src/utils/worktree.posix.test.ts index 0cbbb9a04..74e2fcf47 100644 --- a/packages/server/src/utils/worktree.posix.test.ts +++ b/packages/server/src/utils/worktree.posix.test.ts @@ -23,6 +23,13 @@ import { } from "./worktree"; import type { PaseoConfig } from "@getpaseo/protocol/paseo-config-schema"; import { getPaseoWorktreeMetadataPath } from "./worktree-metadata.js"; +import { + getCheckoutDiff, + getCheckoutStatus, + listCheckoutCommits, + mergeFromBase, + mergeToBase, +} from "./checkout-git.js"; import { execFileSync } from "child_process"; import { isPlatform } from "../test-utils/platform.js"; import { @@ -436,7 +443,7 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => { const localResult = await createLegacyWorktreeForTest({ branchName: "prefer-local-feature", cwd: repoDir, - baseBranch: "main", + baseBranch: "refs/heads/main", worktreeSlug: "prefer-local-feature", runSetup: false, paseoHome, @@ -451,6 +458,15 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => { }); expect(readFileSync(join(localResult.worktreePath, "file.txt"), "utf8")).toBe("from-local\n"); + const localStatus = await getCheckoutStatus(localResult.worktreePath, { paseoHome }); + expect(localStatus.isGit).toBe(true); + if (!localStatus.isGit) { + return; + } + expect(localStatus.aheadBehind).toEqual({ ahead: 0, behind: 0 }); + await expect( + getCheckoutDiff(localResult.worktreePath, { mode: "base", baseRef: "main" }, { paseoHome }), + ).resolves.toMatchObject({ diff: "" }); expect(readFileSync(join(originResult.worktreePath, "file.txt"), "utf8")).toBe( "from-origin\n", ); @@ -459,6 +475,137 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => { ).toMatchObject({ baseRefName: "main" }); }); + it("records the branch name when the base is on a remote other than origin", async () => { + const forkDir = join(tempDir, "fork.git"); + const forkCloneDir = join(tempDir, "fork-clone"); + execFileSync("git", ["init", "--bare", forkDir]); + execFileSync("git", ["remote", "add", "upstream", forkDir], { cwd: repoDir }); + execFileSync("git", ["push", "-u", "upstream", "main"], { cwd: repoDir }); + + execFileSync("git", ["clone", "--branch", "main", forkDir, forkCloneDir]); + execFileSync("git", ["config", "user.email", "test@test.com"], { cwd: forkCloneDir }); + execFileSync("git", ["config", "user.name", "Test"], { cwd: forkCloneDir }); + writeFileSync(join(forkCloneDir, "file.txt"), "from-upstream\n"); + execFileSync("git", ["add", "file.txt"], { cwd: forkCloneDir }); + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "advance upstream main"], { + cwd: forkCloneDir, + }); + execFileSync("git", ["push", "origin", "main"], { cwd: forkCloneDir }); + execFileSync("git", ["fetch", "upstream"], { cwd: repoDir }); + const localMainHead = execFileSync("git", ["rev-parse", "refs/heads/main"], { + cwd: repoDir, + }) + .toString() + .trim(); + + const result = await createLegacyWorktreeForTest({ + branchName: "fork-base-feature", + cwd: repoDir, + baseBranch: "refs/remotes/upstream/main", + worktreeSlug: "fork-base-feature", + runSetup: false, + paseoHome, + }); + + expect(readFileSync(join(result.worktreePath, "file.txt"), "utf8")).toBe("from-upstream\n"); + // The display name and the exact ref are both recorded. Only the ref can resolve back + // to the commit the worktree was cut from: "main" resolves local-first, which here is + // a different commit than the fork's upstream. + expect( + JSON.parse(readFileSync(getPaseoWorktreeMetadataPath(result.worktreePath), "utf8")), + ).toMatchObject({ baseRefName: "main", baseRef: "refs/remotes/upstream/main" }); + + // An untouched child must report no work of its own. Comparing against the wrong base + // shows the upstream-only commit as if the workspace had written it. + const status = await getCheckoutStatus(result.worktreePath, { paseoHome }); + expect(status.isGit).toBe(true); + if (!status.isGit) { + return; + } + // The wire keeps the display name; the exact ref above is what the comparison used, + // which is why an untouched child reports no work of its own. + expect(status.baseRef).toBe("main"); + expect(status.aheadBehind).toEqual({ ahead: 0, behind: 0 }); + await expect( + getCheckoutDiff( + result.worktreePath, + { mode: "base", baseRef: "refs/remotes/origin/main" }, + { paseoHome }, + ), + ).rejects.toThrow( + "Base ref mismatch: stored refs/remotes/upstream/main, requested refs/remotes/origin/main", + ); + expect( + await getCheckoutDiff( + result.worktreePath, + { mode: "base", baseRef: "main" }, + { paseoHome }, + ), + ).toMatchObject({ diff: "" }); + const history = await listCheckoutCommits({ + cwd: result.worktreePath, + context: { paseoHome }, + }); + expect(history.commits.every((commit) => commit.isOnBase)).toBe(true); + await expect( + mergeToBase(result.worktreePath, { baseRef: "main" }, { paseoHome }), + ).rejects.toThrow( + "No local merge target is recorded for base ref refs/remotes/upstream/main", + ); + expect( + execFileSync("git", ["rev-parse", "refs/heads/main"], { cwd: repoDir }).toString().trim(), + ).toBe(localMainHead); + + writeFileSync(join(forkCloneDir, "later.txt"), "later\n"); + execFileSync("git", ["add", "later.txt"], { cwd: forkCloneDir }); + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "advance again"], { + cwd: forkCloneDir, + }); + execFileSync("git", ["push", "origin", "main"], { cwd: forkCloneDir }); + execFileSync("git", ["fetch", "upstream"], { cwd: result.worktreePath }); + await mergeFromBase(result.worktreePath, { baseRef: "main" }, { paseoHome }); + expect(readFileSync(join(result.worktreePath, "later.txt"), "utf8")).toBe("later\n"); + + execFileSync("git", ["update-ref", "-d", "refs/remotes/upstream/main"], { + cwd: result.worktreePath, + }); + await expect( + getCheckoutDiff(result.worktreePath, { mode: "base", baseRef: "main" }, { paseoHome }), + ).rejects.toThrow("Base ref not found: refs/remotes/upstream/main"); + await expect( + mergeFromBase(result.worktreePath, { baseRef: "main" }, { paseoHome }), + ).rejects.toThrow("Base ref not found: refs/remotes/upstream/main"); + await expect( + listCheckoutCommits({ cwd: result.worktreePath, context: { paseoHome } }), + ).rejects.toThrow("Base ref not found: refs/remotes/upstream/main"); + }); + + it("records Git-valid characters in a remote base branch", async () => { + const upstreamDir = join(tempDir, "upstream.git"); + execFileSync("git", ["init", "--bare", upstreamDir]); + execFileSync("git", ["remote", "add", "upstream", upstreamDir], { cwd: repoDir }); + execFileSync("git", ["push", "upstream", "refs/heads/main:refs/heads/release+hotfix"], { + cwd: repoDir, + }); + execFileSync("git", ["fetch", "upstream"], { cwd: repoDir }); + + const result = await createLegacyWorktreeForTest({ + branchName: "release-hotfix-feature", + cwd: repoDir, + baseBranch: "refs/remotes/upstream/release+hotfix", + worktreeSlug: "release-hotfix-feature", + runSetup: false, + paseoHome, + }); + + expect( + JSON.parse(readFileSync(getPaseoWorktreeMetadataPath(result.worktreePath), "utf8")), + ).toMatchObject({ + baseRefName: "release+hotfix", + baseRef: "refs/remotes/upstream/release+hotfix", + }); + }); + it("uses a local branch when origin/{branch} does not exist", async () => { writeFileSync(join(repoDir, "file.txt"), "from-local-only\n"); execFileSync("git", ["add", "file.txt"], { cwd: repoDir }); diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index 7b2cf4560..b10fda36f 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -1271,6 +1271,7 @@ export const createWorktree = async ({ writePaseoWorktreeMetadata(worktreePath, { baseRefName: sourcePlan.metadataBaseRefName, + ...(sourcePlan.metadataBaseRef ? { baseRef: sourcePlan.metadataBaseRef } : {}), ...(sourcePlan.changeRequestLookupTarget ? { changeRequestLookupTarget: sourcePlan.changeRequestLookupTarget } : {}), @@ -1300,7 +1301,11 @@ interface ResolveWorktreeSourcePlanOptions { interface WorktreeSourcePlan { branchName: string; + // Display name and exact ref are two different facts. The name cannot round-trip to a + // commit — "main" resolves local-first even when the worktree was cut from a fork's + // upstream — so comparisons and actions read the ref and the UI reads the name. metadataBaseRefName: string; + metadataBaseRef?: string; changeRequestLookupTarget?: PaseoWorktreeChangeRequestHint; addArguments: string[]; pushRemote?: { @@ -1334,6 +1339,7 @@ async function resolveWorktreeSourcePlan({ return { branchName: newBranchName, metadataBaseRefName: normalizedBaseBranch, + metadataBaseRef: resolvedBaseBranch, addArguments: ["-b", newBranchName, "--no-track", base], }; }