diff --git a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts index 2accf76f1a..cc0bf08250 100644 --- a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts +++ b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.test.ts @@ -253,6 +253,7 @@ function createRealOutcomeHarness(input: { archivedWorkspaceIds: Set; }) { const active = [...input.activeWorkspaces]; + const autoArchivedChangeRequestUrls = new Map(); const logger = pino({ level: "silent" }); vi.spyOn(logger, "info").mockImplementation(() => undefined); vi.spyOn(logger, "warn").mockImplementation(() => undefined); @@ -311,8 +312,12 @@ function createRealOutcomeHarness(input: { }, listActiveWorkspaces: async () => active.filter((workspace) => !input.archivedWorkspaceIds.has(workspace.workspaceId)), - getAutoArchivedChangeRequestUrl: async () => null, - archiveWorkspaceRecord: async (workspaceId: string) => { + getAutoArchivedChangeRequestUrl: async (workspaceId: string) => + autoArchivedChangeRequestUrls.get(workspaceId) ?? null, + archiveWorkspaceRecord: async (workspaceId: string, context) => { + if (context?.autoArchivedChangeRequestUrl) { + autoArchivedChangeRequestUrls.set(workspaceId, context.autoArchivedChangeRequestUrl); + } input.archivedWorkspaceIds.add(workspaceId); const index = active.findIndex((workspace) => workspace.workspaceId === workspaceId); if (index !== -1) { @@ -327,6 +332,10 @@ function createRealOutcomeHarness(input: { return { options, log: logger, + unarchiveWorkspace(workspace: ActiveWorkspaceRef) { + input.archivedWorkspaceIds.delete(workspace.workspaceId); + active.push(workspace); + }, }; } @@ -550,4 +559,48 @@ describe("archiveIfSafe", () => { expect(archivedWorkspaceIds.has(workspaceA)).toBe(true); expect(existsSync(worktree.worktreePath)).toBe(false); }); + + test("real outcome: an unarchived workspace is not archived again for the same merged PR", async () => { + const { tempDir, repoDir } = createGitRepo(); + const paseoHome = path.join(tempDir, ".paseo"); + const worktree = await createPaseoOwnedWorktree(repoDir, paseoHome, "merged-then-unarchived"); + const workspace = { + workspaceId: "ws-merged-then-unarchived", + cwd: worktree.worktreePath, + kind: "worktree" as const, + }; + const sibling = { + workspaceId: "ws-directory-preserving-sibling", + cwd: worktree.worktreePath, + kind: "local_checkout" as const, + }; + const archivedWorkspaceIds = new Set(); + const harness = createRealOutcomeHarness({ + paseoHome, + repoDir, + worktreePath: worktree.worktreePath, + activeWorkspaces: [workspace, sibling], + archivedWorkspaceIds, + }); + const mergedSnapshot = { ...createSnapshot(), cwd: worktree.worktreePath }; + + await archiveIfSafe({ + workspaceId: workspace.workspaceId, + snapshot: mergedSnapshot, + options: harness.options, + log: harness.log, + }); + expect(archivedWorkspaceIds.has(workspace.workspaceId)).toBe(true); + + harness.unarchiveWorkspace(workspace); + await archiveIfSafe({ + workspaceId: workspace.workspaceId, + snapshot: mergedSnapshot, + options: harness.options, + log: harness.log, + }); + + expect(archivedWorkspaceIds.has(workspace.workspaceId)).toBe(false); + expect(existsSync(worktree.worktreePath)).toBe(true); + }); }); diff --git a/packages/server/src/server/auto-archive-on-merge/behavior.test.ts b/packages/server/src/server/auto-archive-on-merge/behavior.test.ts new file mode 100644 index 0000000000..c0e20e36f2 --- /dev/null +++ b/packages/server/src/server/auto-archive-on-merge/behavior.test.ts @@ -0,0 +1,246 @@ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import type { Logger } from "pino"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { + setupAutoArchiveOnMerge, + type AutoArchiveOnMergeDependencies, + type AutoArchiveOnMergeOptions, +} from "./index.js"; +import type { WorkspaceGitRuntimeSnapshot } from "../workspace-git-service.js"; +import { createWorktree } from "../../utils/worktree.js"; + +type PullRequestState = "open" | "closed" | "merged"; + +interface PullRequestObservation { + url: string; + headRefName: string; + state: PullRequestState; +} + +const cleanupPaths: string[] = []; + +function run(cwd: string, args: string[]): void { + execFileSync("git", args, { cwd, stdio: "pipe" }); +} + +async function settleObservation(): Promise { + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setImmediate(resolve)); +} + +async function createWorkspaceJourney() { + const tempDir = realpathSync(mkdtempSync(path.join(tmpdir(), "auto-archive-behavior-"))); + cleanupPaths.push(tempDir); + const repoDir = path.join(tempDir, "repo"); + run(tempDir, ["init", "-b", "main", repoDir]); + run(repoDir, ["config", "user.email", "test@getpaseo.local"]); + run(repoDir, ["config", "user.name", "Paseo Test"]); + writeFileSync(path.join(repoDir, "README.md"), "workspace journey\n"); + run(repoDir, ["add", "README.md"]); + run(repoDir, ["-c", "commit.gpgsign=false", "commit", "-m", "initial"]); + + const paseoHome = path.join(tempDir, ".paseo"); + const worktree = await createWorktree({ + cwd: repoDir, + worktreeSlug: "workspace", + source: { kind: "branch-off", baseBranch: "main", branchName: "workspace" }, + runSetup: false, + paseoHome, + }); + const workspaceId = "workspace-under-test"; + let active = true; + let currentPullRequest: PullRequestObservation | null = null; + let listener: ((snapshot: WorkspaceGitRuntimeSnapshot) => void) | null = null; + let subscription: { unsubscribe: () => void } | null = null; + + function snapshot(): WorkspaceGitRuntimeSnapshot { + const branch = execFileSync("git", ["branch", "--show-current"], { + cwd: worktree.worktreePath, + encoding: "utf8", + }).trim(); + return { + cwd: worktree.worktreePath, + git: { + isGit: true, + repoRoot: worktree.worktreePath, + mainRepoRoot: repoDir, + currentBranch: branch, + remoteUrl: "https://github.com/acme/repo.git", + isPaseoOwnedWorktree: true, + isDirty: false, + baseRef: "main", + aheadBehind: { ahead: 0, behind: 0 }, + upstreamRef: `origin/${branch}`, + aheadOfOrigin: 0, + behindOfOrigin: 0, + hasRemote: true, + diffStat: { additions: 0, deletions: 0 }, + }, + forge: { + featuresEnabled: true, + authState: "authenticated", + pullRequest: currentPullRequest + ? { + url: currentPullRequest.url, + title: currentPullRequest.headRefName, + state: currentPullRequest.state, + baseRefName: "main", + headRefName: currentPullRequest.headRefName, + isMerged: currentPullRequest.state === "merged", + } + : null, + error: null, + }, + }; + } + + const options = { + logger: { child: () => ({ warn: vi.fn() }) } as unknown as Logger, + daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: true }) }, + workspaceGitService: { + onSnapshotUpdated: (nextListener: (next: WorkspaceGitRuntimeSnapshot) => void) => { + listener = nextListener; + return { + unsubscribe: () => { + if (listener === nextListener) listener = null; + }, + }; + }, + getSnapshot: async () => snapshot(), + }, + listActiveWorkspaces: async () => + active ? [{ workspaceId, cwd: worktree.worktreePath, kind: "worktree" as const }] : [], + } as unknown as AutoArchiveOnMergeOptions; + const dependencies: AutoArchiveOnMergeDependencies = { + archiveIfSafe: async () => { + active = false; + }, + resolvePath: path.resolve, + }; + + function startDaemon(): void { + subscription?.unsubscribe(); + subscription = setupAutoArchiveOnMerge(options, dependencies); + } + + async function observe(observation: PullRequestObservation | null): Promise { + currentPullRequest = observation; + const activeListener = listener; + if (!activeListener) throw new Error("Daemon is not observing the workspace"); + activeListener(snapshot()); + await settleObservation(); + } + + function checkout(branch: string): void { + run(worktree.worktreePath, ["checkout", "-B", branch]); + } + + startDaemon(); + + return { + checkout, + isActive: () => active, + observe, + restartDaemon: startDaemon, + }; +} + +function pullRequest( + headRefName: string, + state: PullRequestState, + number = 1, +): PullRequestObservation { + return { + url: `https://github.com/acme/repo/pull/${number}`, + headRefName, + state, + }; +} + +afterEach(() => { + for (const target of cleanupPaths.splice(0)) { + rmSync(target, { recursive: true, force: true }); + } +}); + +describe("workspace auto-archive behavior", () => { + test("archives when the checked-out PR changes from open to merged", async () => { + const workspace = await createWorkspaceJourney(); + workspace.checkout("feature"); + + await workspace.observe(pullRequest("feature", "open")); + expect(workspace.isActive()).toBe(true); + + await workspace.observe(pullRequest("feature", "merged")); + expect(workspace.isActive()).toBe(false); + }); + + test("archives an existing open PR checked out after workspace creation when it later merges", async () => { + const workspace = await createWorkspaceJourney(); + workspace.checkout("existing-open-pr"); + + await workspace.observe(pullRequest("existing-open-pr", "open")); + expect(workspace.isActive()).toBe(true); + + await workspace.observe(pullRequest("existing-open-pr", "merged")); + expect(workspace.isActive()).toBe(false); + }); + + test("does not archive when an already-merged PR is checked out", async () => { + const workspace = await createWorkspaceJourney(); + workspace.checkout("already-merged-pr"); + + await workspace.observe(pullRequest("already-merged-pr", "merged")); + + expect(workspace.isActive()).toBe(true); + }); + + test("does not archive when an open PR is replaced by a different already-merged PR", async () => { + const workspace = await createWorkspaceJourney(); + workspace.checkout("open-pr"); + await workspace.observe(pullRequest("open-pr", "open", 1)); + + workspace.checkout("already-merged-pr"); + await workspace.observe(pullRequest("already-merged-pr", "merged", 2)); + + expect(workspace.isActive()).toBe(true); + }); + + test("does not archive after leaving an open PR and later checking it out already merged", async () => { + const workspace = await createWorkspaceJourney(); + workspace.checkout("eventually-merged-pr"); + await workspace.observe(pullRequest("eventually-merged-pr", "open")); + + workspace.checkout("unrelated-work"); + await workspace.observe(null); + workspace.checkout("eventually-merged-pr"); + await workspace.observe(pullRequest("eventually-merged-pr", "merged")); + + expect(workspace.isActive()).toBe(true); + }); + + test("does not archive when an open PR closes without merging", async () => { + const workspace = await createWorkspaceJourney(); + workspace.checkout("closed-pr"); + + await workspace.observe(pullRequest("closed-pr", "open")); + await workspace.observe(pullRequest("closed-pr", "closed")); + + expect(workspace.isActive()).toBe(true); + }); + + test("does not infer a merge transition from the first observation after daemon restart", async () => { + const workspace = await createWorkspaceJourney(); + workspace.checkout("merged-while-offline"); + await workspace.observe(pullRequest("merged-while-offline", "open")); + + workspace.restartDaemon(); + await workspace.observe(pullRequest("merged-while-offline", "merged")); + + expect(workspace.isActive()).toBe(true); + }); +}); diff --git a/packages/server/src/server/auto-archive-on-merge/index.test.ts b/packages/server/src/server/auto-archive-on-merge/index.test.ts index 1e13df1dd5..885d8250e8 100644 --- a/packages/server/src/server/auto-archive-on-merge/index.test.ts +++ b/packages/server/src/server/auto-archive-on-merge/index.test.ts @@ -9,7 +9,10 @@ import { } from "./index.js"; import type { WorkspaceGitRuntimeSnapshot } from "../workspace-git-service.js"; -function createSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot { +function createSnapshot( + cwd: string, + state: "open" | "merged" = "merged", +): WorkspaceGitRuntimeSnapshot { return { cwd, git: { @@ -32,11 +35,11 @@ function createSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot { authState: "authenticated", pullRequest: { url: "https://github.com/acme/repo/pull/12", - title: "Merged", - state: "merged", + title: "Feature", + state, baseRefName: "main", headRefName: "feature", - isMerged: true, + isMerged: state === "merged", }, error: null, }, @@ -47,9 +50,6 @@ test("fans one fresh observation out to every workspace attached to its exact cw let onSnapshotUpdated: ((snapshot: WorkspaceGitRuntimeSnapshot) => void) | null = null; const eventSnapshot = createSnapshot("/repo/worktree/."); const freshSnapshot = createSnapshot("/repo/worktree"); - if (freshSnapshot.forge.pullRequest) { - freshSnapshot.forge.pullRequest.url = "https://github.com/acme/repo/pull/13"; - } const getSnapshot = vi.fn(async () => freshSnapshot); const options = { logger: { child: () => ({ warn: vi.fn() }) } as unknown as Logger, @@ -85,6 +85,7 @@ test("fans one fresh observation out to every workspace attached to its exact cw setupAutoArchiveOnMerge(options, deps); if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(createSnapshot("/repo/worktree/.", "open")); onSnapshotUpdated(eventSnapshot); await finished; @@ -135,6 +136,7 @@ test("serializes the complete fan-out for duplicate merge events on one cwd", as setupAutoArchiveOnMerge(options, deps); if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(createSnapshot("/repo/worktree/.", "open")); onSnapshotUpdated(snapshot); await vi.waitFor(() => expect(archivedWorkspaceIds).toEqual(["workspace-a"])); @@ -171,6 +173,7 @@ test("does not fan out a stale merged event when the fresh observation has no PR setupAutoArchiveOnMerge(options, { archiveIfSafe, resolvePath: resolve }); if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(createSnapshot("/repo/worktree", "open")); onSnapshotUpdated(eventSnapshot); await vi.waitFor(() => expect(getSnapshot).toHaveBeenCalledTimes(1)); await Promise.resolve(); @@ -199,6 +202,7 @@ test("logs and skips when the fresh observation cannot be read", async () => { setupAutoArchiveOnMerge(options, { archiveIfSafe, resolvePath: resolve }); if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(createSnapshot("/repo/worktree", "open")); onSnapshotUpdated(createSnapshot("/repo/worktree")); await vi.waitFor(() => expect(warn).toHaveBeenCalledWith( diff --git a/packages/server/src/server/auto-archive-on-merge/index.ts b/packages/server/src/server/auto-archive-on-merge/index.ts index 27608d6bda..be950bf367 100644 --- a/packages/server/src/server/auto-archive-on-merge/index.ts +++ b/packages/server/src/server/auto-archive-on-merge/index.ts @@ -1,4 +1,5 @@ import { resolve } from "node:path"; +import { LRUCache } from "lru-cache"; import type { Logger } from "pino"; import { archiveIfSafe, type AutoArchiveArchiveOptions } from "./archive-if-safe.js"; @@ -16,6 +17,8 @@ export interface AutoArchiveOnMergeDependencies { resolvePath: typeof resolve; } +const OPEN_PULL_REQUEST_LATCH_MAX = 1_024; + const defaultDependencies: AutoArchiveOnMergeDependencies = { archiveIfSafe, resolvePath: resolve, @@ -27,16 +30,30 @@ export function setupAutoArchiveOnMerge( ): WorkspaceGitSubscription { const log = options.logger.child({ module: "auto-archive-on-merge" }); const inFlightCwds = new Set(); + const openPullRequestUrlsByCwd = new LRUCache({ + max: OPEN_PULL_REQUEST_LATCH_MAX, + }); return options.workspaceGitService.onSnapshotUpdated((snapshot) => { - if (!snapshot.forge.pullRequest?.isMerged) { - return; - } + const snapshotCwd = deps.resolvePath(snapshot.cwd); if (options.daemonConfigStore.get().autoArchiveAfterMerge !== true) { + openPullRequestUrlsByCwd.delete(snapshotCwd); return; } - const snapshotCwd = deps.resolvePath(snapshot.cwd); + const pullRequest = snapshot.forge.pullRequest; + if (!pullRequest?.isMerged) { + if (pullRequest?.state.toLowerCase() === "open") { + openPullRequestUrlsByCwd.set(snapshotCwd, pullRequest.url); + } else { + openPullRequestUrlsByCwd.delete(snapshotCwd); + } + return; + } + if (openPullRequestUrlsByCwd.get(snapshotCwd) !== pullRequest.url) { + openPullRequestUrlsByCwd.delete(snapshotCwd); + return; + } if (inFlightCwds.has(snapshotCwd)) { return; } @@ -55,7 +72,15 @@ export function setupAutoArchiveOnMerge( ); return; } - if (!freshSnapshot?.forge.pullRequest?.isMerged) { + const freshPullRequest = freshSnapshot?.forge.pullRequest; + if ( + !freshPullRequest?.isMerged || + freshPullRequest.url !== pullRequest.url || + openPullRequestUrlsByCwd.get(snapshotCwd) !== pullRequest.url + ) { + if (openPullRequestUrlsByCwd.get(snapshotCwd) === pullRequest.url) { + openPullRequestUrlsByCwd.delete(snapshotCwd); + } return; } diff --git a/packages/server/src/server/daemon-e2e/auto-archive-unrelated-github-merge.real.e2e.test.ts b/packages/server/src/server/daemon-e2e/auto-archive-unrelated-github-merge.real.e2e.test.ts index 73a6951061..b3a3d4854f 100644 --- a/packages/server/src/server/daemon-e2e/auto-archive-unrelated-github-merge.real.e2e.test.ts +++ b/packages/server/src/server/daemon-e2e/auto-archive-unrelated-github-merge.real.e2e.test.ts @@ -66,7 +66,7 @@ afterEach(async () => { }); githubTest( - "does not retarget a never-pushed workspace when its checkout moves onto a merged PR branch", + "shows but does not archive an already-merged PR checked out in another workspace", async () => { const repository = realpathSync(mkdtempSync(path.join(tmpdir(), "paseo-github-merge-"))); cleanupPaths.add(repository); @@ -154,6 +154,13 @@ githubTest( "Producer-level auto-archive reproduction.", ]); + expect((await context.client.checkoutRefresh(merged.worktreePath)).success).toBe(true); + expect((await context.client.checkoutPrStatus(merged.worktreePath)).status).toMatchObject({ + url: pullRequestUrl, + state: "open", + isMerged: false, + }); + const beforeMerge = await activeWorkspaceIds(context); expect(beforeMerge).toEqual( expect.arrayContaining([mergedWorkspace.workspace.id, unrelatedWorkspace.workspace.id]), @@ -176,13 +183,17 @@ githubTest( expect(unrelatedStatus.status).toBeNull(); expect(await activeWorkspaceIds(context)).toContain(unrelatedWorkspace.workspace.id); - // The workspace remains identified by the branch it was created for. Moving the - // checkout does not silently retarget its PR association or archive identity. - run(merged.worktreePath, "git", ["checkout", "--detach"]); + // The current checkout's PR remains visible, but arriving after its merge is not + // a merge transition and must not archive this workspace. run(unrelated.worktreePath, "git", ["checkout", "merged-feature"]); expect((await context.client.checkoutRefresh(unrelated.worktreePath)).success).toBe(true); const switchedStatus = await context.client.checkoutPrStatus(unrelated.worktreePath); - expect(switchedStatus.status).toBeNull(); + expect(switchedStatus.status).toMatchObject({ + url: pullRequestUrl, + headRefName: "merged-feature", + state: "merged", + isMerged: true, + }); expect(await activeWorkspaceIds(context)).toContain(unrelatedWorkspace.workspace.id); }, 180_000, diff --git a/packages/server/src/server/worktree-core.posix.test.ts b/packages/server/src/server/worktree-core.posix.test.ts index 42a846de35..527b989d7e 100644 --- a/packages/server/src/server/worktree-core.posix.test.ts +++ b/packages/server/src/server/worktree-core.posix.test.ts @@ -1087,7 +1087,7 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => { expect(dedupedRemoteBranch.status).toBe(1); }); - test("does not derive a deduped PR lookup target when managed metadata has no target", async () => { + test("derives the tracked PR lookup target when managed metadata has no target", async () => { const { tempDir, repoDir, remoteDir, paseoHome } = createSameRepoGitHubPrRemoteRepo(); cleanupPaths.push(tempDir); execFileSync("git", ["remote", "set-url", "origin", `file://${remoteDir}`], { @@ -1139,13 +1139,20 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => { "utf8", ); + const currentHead = execFileSync("git", ["rev-parse", "HEAD"], { + cwd: second.worktree.worktreePath, + encoding: "utf8", + }).trim(); const facts = await getCheckoutSnapshotFacts(second.worktree.worktreePath, { paseoHome }); expect(second.worktree.branchName).toBe("daemon-shutdown-diagnostics-1"); expect(facts).toMatchObject({ isGit: true, currentBranch: "daemon-shutdown-diagnostics-1", - pullRequestLookupTarget: null, + pullRequestLookupTarget: { + headRef: "daemon-shutdown-diagnostics", + headSha: currentHead, + }, }); }); diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index f38adff412..182cc7ce98 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -2619,85 +2619,90 @@ const x = 1; expect(lookupTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); }); - it("does not retarget a PR worktree lookup from current branch tracking", async () => { + it.each([ + { state: "open", isMerged: false }, + { state: "closed", isMerged: false }, + { state: "merged", isMerged: true }, + ])( + "shows a $state PR for the branch currently checked out after workspace creation", + async ({ state, isMerged }) => { + execFileSync("git", ["remote", "add", "origin", "https://github.com/acme/repo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["branch", "contributor/old-change"], { cwd: repoDir }); + execFileSync("git", ["branch", "new-change"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.new-change.remote", "origin"], { cwd: repoDir }); + execFileSync("git", ["config", "branch.new-change.merge", "refs/heads/new-change"], { + cwd: repoDir, + }); + const workspaceDir = join(paseoHome, "worktrees", "repo", "pr-worktree"); + mkdirSync(join(paseoHome, "worktrees", "repo"), { recursive: true }); + execFileSync("git", ["worktree", "add", workspaceDir, "contributor/old-change"], { + cwd: repoDir, + }); + writePaseoWorktreeMetadata(workspaceDir, { + baseRefName: "main", + changeRequestLookupTarget: { + headRef: "old-change", + headRepositoryOwner: "contributor", + changeRequestNumber: 41, + localBranchName: "contributor/old-change", + }, + }); + + execFileSync("git", ["checkout", "new-change"], { cwd: workspaceDir }); + const requestedTargets: RequestedPullRequestTarget[] = []; + const facts = await getCheckoutSnapshotFacts(workspaceDir, { paseoHome }); + const result = await getPullRequestStatus( + workspaceDir, + createGitHubServiceRecordingPullRequestTargets({ + requestedTargets, + statusOverrides: { state, isMerged }, + }), + { force: true, reason: "current-checkout-pr" }, + { paseoHome, facts }, + ); + + expect(requestedTargets).toEqual([expect.objectContaining({ headRef: "new-change" })]); + expect(result.status).toMatchObject({ + headRefName: "new-change", + state, + isMerged, + }); + }, + ); + + it("shows the PR after an agent renames the branch directly with git", async () => { execFileSync("git", ["remote", "add", "origin", "https://github.com/acme/repo.git"], { cwd: repoDir, }); - execFileSync("git", ["branch", "contributor/old-change"], { cwd: repoDir }); - execFileSync("git", ["branch", "new-change"], { cwd: repoDir }); - execFileSync("git", ["config", "branch.new-change.remote", "origin"], { cwd: repoDir }); - execFileSync("git", ["config", "branch.new-change.merge", "refs/heads/new-change"], { - cwd: repoDir, - }); - const workspaceDir = join(paseoHome, "worktrees", "repo", "pr-worktree"); + execFileSync("git", ["branch", "placeholder"], { cwd: repoDir }); + const workspaceDir = join(paseoHome, "worktrees", "repo", "renamed-by-agent"); mkdirSync(join(paseoHome, "worktrees", "repo"), { recursive: true }); - execFileSync("git", ["worktree", "add", workspaceDir, "contributor/old-change"], { - cwd: repoDir, - }); - const staleLookupTarget = { - headRef: "old-change", - headRepositoryOwner: "contributor", - changeRequestNumber: 41, - }; + execFileSync("git", ["worktree", "add", workspaceDir, "placeholder"], { cwd: repoDir }); writePaseoWorktreeMetadata(workspaceDir, { baseRefName: "main", changeRequestLookupTarget: { - ...staleLookupTarget, - localBranchName: "contributor/old-change", + headRef: "placeholder", + localBranchName: "placeholder", }, }); - expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ - headRef: "old-change", - headRepositoryOwner: "contributor", - }); - - execFileSync("git", ["checkout", "new-change"], { cwd: workspaceDir }); - startGitCommandMetrics(); - const switchedTarget = await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome); - const commands = stopGitCommandMetrics().commands.map((command) => command.args[0]); - - expect(switchedTarget).toBeNull(); - expect(commands).not.toContain("fetch"); - - writePaseoWorktreeMetadata(workspaceDir, { - baseRefName: "main", - changeRequestLookupTarget: { - ...staleLookupTarget, - localBranchName: "new-change", - }, - }); - expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ - headRef: "old-change", - headRepositoryOwner: "contributor", - }); - - execFileSync( - "git", - ["remote", "add", "enterprise-fork", "git@github.acme.internal:contributor/repo.git"], - { - cwd: repoDir, - }, + execFileSync("git", ["branch", "-m", "agent-chosen-name"], { cwd: workspaceDir }); + const requestedTargets: RequestedPullRequestTarget[] = []; + const facts = await getCheckoutSnapshotFacts(workspaceDir, { paseoHome }); + const result = await getPullRequestStatus( + workspaceDir, + createGitHubServiceRecordingPullRequestTargets({ requestedTargets }), + { force: true, reason: "agent-renamed-branch-pr" }, + { paseoHome, facts }, ); - execFileSync("git", ["config", "branch.new-change.remote", "enterprise-fork"], { - cwd: repoDir, - }); - execFileSync("git", ["config", "branch.new-change.merge", "refs/heads/old-change"], { - cwd: repoDir, - }); - expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ - headRef: "old-change", - headRepositoryOwner: "contributor", - }); - execFileSync("git", ["branch", "local-upstream"], { cwd: repoDir }); - execFileSync("git", ["config", "branch.new-change.remote", "."], { cwd: repoDir }); - execFileSync("git", ["config", "branch.new-change.merge", "refs/heads/local-upstream"], { - cwd: repoDir, - }); - expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ - headRef: "old-change", - headRepositoryOwner: "contributor", + expect(requestedTargets).toEqual([expect.objectContaining({ headRef: "agent-chosen-name" })]); + expect(result.status).toMatchObject({ + headRefName: "agent-chosen-name", + state: "open", + isMerged: false, }); }); @@ -2731,7 +2736,7 @@ const x = 1; }); }); - it("fails closed when a managed worktree has no metadata", async () => { + it("uses the checked-out branch when a managed worktree has no metadata", async () => { execFileSync("git", ["branch", "feature/unpinned"], { cwd: repoDir }); const workspaceDir = join(paseoHome, "worktrees", "repo", "unpinned-worktree"); mkdirSync(join(paseoHome, "worktrees", "repo"), { recursive: true }); @@ -2739,10 +2744,12 @@ const x = 1; cwd: repoDir, }); - expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toBeNull(); + expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ + headRef: "feature/unpinned", + }); }); - it("does not apply ambiguous legacy PR metadata to a suffixed branch", async () => { + it("uses the checked-out branch instead of ambiguous legacy PR metadata", async () => { execFileSync("git", ["branch", "contributor/old-change-1"], { cwd: repoDir }); const workspaceDir = join(paseoHome, "worktrees", "repo", "legacy-pr-worktree"); mkdirSync(join(paseoHome, "worktrees", "repo"), { recursive: true }); @@ -2760,7 +2767,7 @@ const x = 1; const lookupTarget = await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome); - expect(lookupTarget).toBeNull(); + expect(lookupTarget).toMatchObject({ headRef: "contributor/old-change-1" }); }); it("does not apply a legacy fork hint to an ownerless branch with the same head", async () => { @@ -2803,7 +2810,14 @@ const x = 1; { paseoHome, facts: ownerlessFacts }, ); - expect(requestedTargets).toHaveLength(1); + expect(requestedTargets).toEqual([ + expect.objectContaining({ + headRef: "old-change", + headRepositoryOwner: "contributor", + }), + expect.objectContaining({ headRef: "old-change" }), + ]); + expect(requestedTargets[1]).not.toHaveProperty("headRepositoryOwner"); }); it("recognizes a normalized GitHub owner branch from legacy Enterprise metadata", async () => { @@ -2874,7 +2888,7 @@ const x = 1; }); }); - it("keeps a ref-only change request bound across rename but not branch switch", async () => { + it("keeps a ref-only change request across rename and follows a later branch switch", async () => { execFileSync("git", ["branch", "feature/gitlab-mr"], { cwd: repoDir }); const workspaceDir = join(paseoHome, "worktrees", "repo", "gitlab-mr-worktree"); mkdirSync(join(paseoHome, "worktrees", "repo"), { recursive: true }); @@ -2918,7 +2932,10 @@ const x = 1; { paseoHome, facts: switchedFacts }, ); - expect(requestedTargets).toHaveLength(1); + expect(requestedTargets).toEqual([ + expect.objectContaining({ headRef: "feature/gitlab-mr" }), + expect.objectContaining({ headRef: "other-branch" }), + ]); }); it("moves a managed branch identity pin when its branch is renamed", async () => { diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 628d2716bf..febf226c8f 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -1813,7 +1813,6 @@ function buildPullRequestLookupTargetFromMetadata( function buildInitialPullRequestLookupTarget(input: { currentBranch: string | null; - metadata: PaseoWorktreeMetadata | null; branchRemoteName: string | null; branchMergeRef: string | null; branchRemoteUrl: string | null; @@ -1824,12 +1823,6 @@ function buildInitialPullRequestLookupTarget(input: { return null; } - // Paseo worktree metadata owns PR identity. A checkout drift must not fall - // through to branch config and silently retarget the workspace. - if (input.metadata) { - return buildPullRequestLookupTargetFromMetadata(input.metadata, input.currentBranch); - } - const hasConfiguredBranchTarget = Boolean( input.branchRemoteName && parseBranchMergeHeadRef(input.branchMergeRef), ); @@ -1844,17 +1837,14 @@ function buildInitialPullRequestLookupTarget(input: { }); } - return ( - buildPullRequestLookupTargetFromMetadata(input.metadata, input.currentBranch) ?? - buildPullRequestLookupTargetFromBranchConfig({ - currentBranch: input.currentBranch, - branchRemoteName: input.branchRemoteName, - branchMergeRef: input.branchMergeRef, - branchRemoteUrl: input.branchRemoteUrl, - originRemoteUrl: input.originRemoteUrl, - resolvedBaseRef: input.resolvedBaseRef, - }) - ); + return buildPullRequestLookupTargetFromBranchConfig({ + currentBranch: input.currentBranch, + branchRemoteName: input.branchRemoteName, + branchMergeRef: input.branchMergeRef, + branchRemoteUrl: input.branchRemoteUrl, + originRemoteUrl: input.originRemoteUrl, + resolvedBaseRef: input.resolvedBaseRef, + }); } async function resolvePullRequestLookupTargetFromPushConfig( @@ -1900,13 +1890,15 @@ async function resolveFactsPullRequestLookupTarget(input: { context?: CheckoutContext; }): Promise { const { cwd, inspected, metadata, context } = input; - if (inspected.paseoWorktree.isPaseoOwnedWorktree) { - return buildPullRequestLookupTargetFromMetadata(metadata, inspected.currentBranch ?? ""); + const metadataTarget = inspected.currentBranch + ? buildPullRequestLookupTargetFromMetadata(metadata, inspected.currentBranch) + : null; + if (metadataTarget) { + return metadataTarget; } let target = buildInitialPullRequestLookupTarget({ currentBranch: inspected.currentBranch, - metadata, branchRemoteName: input.branchRemoteName, branchMergeRef: input.branchMergeRef, branchRemoteUrl: input.branchRemoteUrl,