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 5e8cf6c61..2accf76f1 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 @@ -57,8 +57,9 @@ function createSnapshot(overrides?: { diffStat: { additions: 0, deletions: 0 }, ...overrides?.git, }, - github: { + forge: { featuresEnabled: true, + authState: "authenticated", pullRequest: overrides && "pullRequest" in overrides ? (overrides.pullRequest ?? null) @@ -78,18 +79,13 @@ function createLogger(): Logger { } function createHarness(overrides?: { - autoArchiveAfterMerge?: boolean; autoArchivedChangeRequestUrl?: string | null; - getSnapshot?: () => Promise; + snapshot?: WorkspaceGitRuntimeSnapshot; isPaseoOwnedWorktreeCwd?: ArchiveIfSafeDependencies["isPaseoOwnedWorktreeCwd"]; archiveByScope?: ArchiveIfSafeDependencies["archiveByScope"]; - resolveWorkspaceIdAtPath?: ArchiveIfSafeDependencies["resolveWorkspaceIdAtPath"]; }) { - const getConfig = vi.fn(() => ({ - autoArchiveAfterMerge: overrides?.autoArchiveAfterMerge ?? true, - })); - const getSnapshot = vi.fn( - overrides?.getSnapshot ?? (async () => createSnapshot()), + const getSnapshot = vi.fn(async () => + createSnapshot(), ) as unknown as AutoArchiveArchiveOptions["workspaceGitService"]["getSnapshot"]; const workspaceGitService = { getSnapshot, @@ -97,7 +93,7 @@ function createHarness(overrides?: { const options: AutoArchiveArchiveOptions = { paseoHome: PASEO_HOME, daemonConfigStore: { - get: getConfig, + get: () => ({ autoArchiveAfterMerge: true }), } as unknown as AutoArchiveArchiveOptions["daemonConfigStore"], workspaceGitService, github: {} as AutoArchiveArchiveOptions["github"], @@ -123,9 +119,6 @@ function createHarness(overrides?: { removedDirectory: false, }) satisfies ArchiveResult), ) as unknown as ArchiveIfSafeDependencies["archiveByScope"]; - const resolveWorkspaceIdAtPath = vi.fn( - overrides?.resolveWorkspaceIdAtPath ?? (async () => "ws-auto-archive"), - ) as unknown as ArchiveIfSafeDependencies["resolveWorkspaceIdAtPath"]; const isPaseoOwnedWorktreeCwd = vi.fn( overrides?.isPaseoOwnedWorktreeCwd ?? (async () => ({ @@ -137,37 +130,34 @@ function createHarness(overrides?: { ) as unknown as ArchiveIfSafeDependencies["isPaseoOwnedWorktreeCwd"]; const deps: ArchiveIfSafeDependencies = { archiveByScope, - resolveWorkspaceIdAtPath, isPaseoOwnedWorktreeCwd, killTerminalsForWorkspace: vi.fn(), }; const log = createLogger(); - const inFlight = new Set(); - return { deps, - getConfig, getSnapshot, - inFlight, log, options, + snapshot: overrides?.snapshot ?? createSnapshot(), }; } async function runArchiveIfSafe( harness: ReturnType, overrides?: { - cwd?: string; + workspaceId?: string; + snapshot?: WorkspaceGitRuntimeSnapshot; pullRequest?: WorkspaceGitRuntimeSnapshot["forge"]["pullRequest"]; }, ): Promise { await archiveIfSafe({ - cwd: overrides?.cwd ?? CWD, - pullRequest: - overrides && "pullRequest" in overrides - ? (overrides.pullRequest ?? null) - : createPullRequest(), - inFlight: harness.inFlight, + workspaceId: overrides?.workspaceId ?? "ws-auto-archive", + snapshot: + overrides?.snapshot ?? + (overrides && "pullRequest" in overrides + ? createSnapshot({ pullRequest: overrides.pullRequest ?? null }) + : harness.snapshot), options: harness.options, log: harness.log, deps: harness.deps, @@ -292,8 +282,9 @@ function createRealOutcomeHarness(input: { hasRemote: true, diffStat: { additions: 0, deletions: 0 }, }, - github: { + forge: { featuresEnabled: true, + authState: "authenticated", pullRequest: createPullRequest({ isMerged: true }), error: null, }, @@ -336,7 +327,6 @@ function createRealOutcomeHarness(input: { return { options, log: logger, - inFlight: new Set(), }; } @@ -356,61 +346,13 @@ describe("archiveIfSafe", () => { await runArchiveIfSafe(harness, { pullRequest: createPullRequest({ isMerged: false }) }); - expect(harness.getConfig).not.toHaveBeenCalled(); expect(harness.getSnapshot).not.toHaveBeenCalled(); expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); }); - test("does nothing when auto-archive-after-merge is disabled", async () => { - const harness = createHarness({ autoArchiveAfterMerge: false }); - - await runArchiveIfSafe(harness); - - expect(harness.getConfig).toHaveBeenCalledTimes(1); - expect(harness.getSnapshot).not.toHaveBeenCalled(); - expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); - }); - - test("does nothing when the cwd already has an archive in flight", async () => { - const harness = createHarness(); - harness.inFlight.add(CWD); - - await runArchiveIfSafe(harness); - - expect(harness.getSnapshot).not.toHaveBeenCalled(); - expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); - expect(harness.inFlight.has(CWD)).toBe(true); - }); - - test("logs and skips when reading the snapshot fails", async () => { - const harness = createHarness({ - getSnapshot: async () => { - throw new Error("snapshot failed"); - }, - }); - - await runArchiveIfSafe(harness); - - expect(harness.log.warn).toHaveBeenCalledWith( - { err: expect.any(Error), cwd: CWD }, - "Failed to read snapshot for auto-archive; skipping", - ); - expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); - expect(harness.inFlight.has(CWD)).toBe(false); - }); - - test("does nothing when there is no snapshot", async () => { - const harness = createHarness({ getSnapshot: async () => null }); - - await runArchiveIfSafe(harness); - - expect(harness.deps.isPaseoOwnedWorktreeCwd).not.toHaveBeenCalled(); - expect(harness.deps.archiveByScope).not.toHaveBeenCalled(); - }); - test("does nothing when the worktree is dirty", async () => { const harness = createHarness({ - getSnapshot: async () => createSnapshot({ git: { isDirty: true } }), + snapshot: createSnapshot({ git: { isDirty: true } }), }); await runArchiveIfSafe(harness); @@ -421,7 +363,7 @@ describe("archiveIfSafe", () => { test("does nothing when the worktree is ahead of origin", async () => { const harness = createHarness({ - getSnapshot: async () => createSnapshot({ git: { aheadOfOrigin: 1 } }), + snapshot: createSnapshot({ git: { aheadOfOrigin: 1 } }), }); await runArchiveIfSafe(harness); @@ -432,8 +374,7 @@ describe("archiveIfSafe", () => { test("archives when the PR is merged and the upstream branch was deleted", async () => { const harness = createHarness({ - getSnapshot: async () => - createSnapshot({ git: { aheadOfOrigin: null, behindOfOrigin: null } }), + snapshot: createSnapshot({ git: { aheadOfOrigin: null, behindOfOrigin: null } }), }); await runArchiveIfSafe(harness); @@ -467,7 +408,6 @@ describe("archiveIfSafe", () => { { err: expect.any(Error), cwd: CWD }, "Auto-archive after merge failed", ); - expect(harness.inFlight.has(CWD)).toBe(false); }); test("archives a clean Paseo-owned worktree after merge", async () => { @@ -475,14 +415,6 @@ describe("archiveIfSafe", () => { await runArchiveIfSafe(harness); - expect(harness.deps.resolveWorkspaceIdAtPath).toHaveBeenCalledTimes(1); - expect(harness.deps.resolveWorkspaceIdAtPath).toHaveBeenCalledWith( - { - findWorkspaceIdForCwd: harness.options.findWorkspaceIdForCwd, - listActiveWorkspaces: harness.options.listActiveWorkspaces, - }, - CWD, - ); expect(harness.deps.archiveByScope).toHaveBeenCalledTimes(1); expect(harness.deps.archiveByScope).toHaveBeenCalledWith( expect.objectContaining({ @@ -495,10 +427,14 @@ describe("archiveIfSafe", () => { }, ); expect(harness.log.info).toHaveBeenCalledWith( - { cwd: CWD }, + { + workspaceId: "ws-auto-archive", + cwd: CWD, + branch: "feature", + pullRequestUrl: "https://github.com/acme/repo/pull/123", + }, "Auto-archived worktree after PR merge", ); - expect(harness.inFlight.has(CWD)).toBe(false); }); test("does not archive a merge event already consumed by this workspace", async () => { @@ -540,18 +476,15 @@ describe("archiveIfSafe", () => { }); }); - test("resolves the merged cwd to a single workspace and does not iterate siblings", async () => { - const harness = createHarness({ - resolveWorkspaceIdAtPath: async () => "ws-merged-worktree", - }); + test("archives only the supplied workspace id and does not iterate siblings", async () => { + const harness = createHarness(); harness.options.listActiveWorkspaces = vi.fn(async () => [ { workspaceId: "ws-merged-worktree", cwd: CWD, kind: "worktree" as const }, { workspaceId: "ws-sibling", cwd: CWD, kind: "local_checkout" as const }, ]); - await runArchiveIfSafe(harness); + await runArchiveIfSafe(harness, { workspaceId: "ws-merged-worktree" }); - expect(harness.deps.resolveWorkspaceIdAtPath).toHaveBeenCalledTimes(1); expect(harness.deps.archiveByScope).toHaveBeenCalledTimes(1); expect(harness.deps.archiveByScope).toHaveBeenCalledWith( expect.any(Object), @@ -581,9 +514,8 @@ describe("archiveIfSafe", () => { }); await archiveIfSafe({ - cwd: worktree.worktreePath, - pullRequest: createPullRequest({ isMerged: true }), - inFlight: harness.inFlight, + workspaceId: workspaceA, + snapshot: { ...createSnapshot(), cwd: worktree.worktreePath }, options: harness.options, log: harness.log, }); @@ -609,9 +541,8 @@ describe("archiveIfSafe", () => { }); await archiveIfSafe({ - cwd: worktree.worktreePath, - pullRequest: createPullRequest({ isMerged: true }), - inFlight: harness.inFlight, + workspaceId: workspaceA, + snapshot: { ...createSnapshot(), cwd: worktree.worktreePath }, options: harness.options, log: harness.log, }); diff --git a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts index 02c44bb9b..be8a6f4ce 100644 --- a/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts +++ b/packages/server/src/server/auto-archive-on-merge/archive-if-safe.ts @@ -7,7 +7,6 @@ import { archiveByScope, type ActiveWorkspaceRef, killTerminalsForWorkspace, - resolveWorkspaceIdAtPath, } from "../workspace-archive-service.js"; import type { WorkspaceGitRuntimeSnapshot, @@ -38,124 +37,89 @@ export interface AutoArchiveArchiveOptions { export interface ArchiveIfSafeDependencies { archiveByScope: typeof archiveByScope; - resolveWorkspaceIdAtPath: typeof resolveWorkspaceIdAtPath; isPaseoOwnedWorktreeCwd: typeof isPaseoOwnedWorktreeCwd; killTerminalsForWorkspace: typeof killTerminalsForWorkspace; } const defaultDependencies: ArchiveIfSafeDependencies = { archiveByScope, - resolveWorkspaceIdAtPath, isPaseoOwnedWorktreeCwd, killTerminalsForWorkspace, }; export async function archiveIfSafe(input: { - cwd: string; - pullRequest: WorkspaceGitRuntimeSnapshot["forge"]["pullRequest"]; - inFlight: Set; + workspaceId: string; + snapshot: WorkspaceGitRuntimeSnapshot; options: AutoArchiveArchiveOptions; log: Logger; deps?: ArchiveIfSafeDependencies; }): Promise { - const { cwd, pullRequest, inFlight, options, log } = input; + const { workspaceId, snapshot, options, log } = input; const deps = input.deps ?? defaultDependencies; + const cwd = snapshot.cwd; + const pullRequest = snapshot.forge.pullRequest; if (!pullRequest?.isMerged) { return; } - if (options.daemonConfigStore.get().autoArchiveAfterMerge !== true) { + if (snapshot.git.isDirty === true) { return; } - if (inFlight.has(cwd)) { + if (typeof snapshot.git.aheadOfOrigin === "number" && snapshot.git.aheadOfOrigin > 0) { + return; + } + + const ownership = await deps.isPaseoOwnedWorktreeCwd(cwd, { + paseoHome: options.paseoHome, + worktreesRoot: options.paseoWorktreesBaseRoot, + }); + if (!ownership.allowed) { return; } - inFlight.add(cwd); try { - let snapshot: Awaited> | null; - try { - snapshot = await options.workspaceGitService.getSnapshot(cwd, { - reason: "auto-archive-on-merge", - }); - } catch (error) { - log.warn({ err: error, cwd }, "Failed to read snapshot for auto-archive; skipping"); - return; - } - if (!snapshot) { + const autoArchivedChangeRequestUrl = await options.getAutoArchivedChangeRequestUrl(workspaceId); + if (autoArchivedChangeRequestUrl === pullRequest.url) { return; } - if (snapshot.git.isDirty === true) { - return; - } - if (typeof snapshot.git.aheadOfOrigin === "number" && snapshot.git.aheadOfOrigin > 0) { - return; - } - - const ownership = await deps.isPaseoOwnedWorktreeCwd(cwd, { - paseoHome: options.paseoHome, - worktreesRoot: options.paseoWorktreesBaseRoot, - }); - if (!ownership.allowed) { - return; - } - - try { - const workspaceId = await deps.resolveWorkspaceIdAtPath( - { - findWorkspaceIdForCwd: options.findWorkspaceIdForCwd, - listActiveWorkspaces: options.listActiveWorkspaces, - }, - cwd, - ); - if (!workspaceId) { - log.warn({ cwd }, "Auto-archive could not resolve a workspace for cwd; skipping"); - return; - } - const autoArchivedChangeRequestUrl = - await options.getAutoArchivedChangeRequestUrl(workspaceId); - if (autoArchivedChangeRequestUrl === pullRequest.url) { - return; - } - - await deps.archiveByScope( - { - paseoHome: options.paseoHome, - paseoWorktreesBaseRoot: options.paseoWorktreesBaseRoot, - github: options.github, - workspaceGitService: options.workspaceGitService, - agentManager: options.agentManager, - agentStorage: options.agentStorage, - findWorkspaceIdForCwd: options.findWorkspaceIdForCwd, - listActiveWorkspaces: options.listActiveWorkspaces, - archiveWorkspaceRecord: (workspaceIdToArchive) => - options.archiveWorkspaceRecord(workspaceIdToArchive, { - autoArchivedChangeRequestUrl: pullRequest.url, - }), - emitWorkspaceUpdatesForWorkspaceIds: options.emitWorkspaceUpdatesForWorkspaceIds, - markWorkspaceArchiving: options.markWorkspaceArchiving, - clearWorkspaceArchiving: options.clearWorkspaceArchiving, - killTerminalsForWorkspace: (workspaceIdToKill) => - deps.killTerminalsForWorkspace( - { - terminalManager: options.terminalManager, - sessionLogger: log, - }, - workspaceIdToKill, - ), - sessionLogger: log, - }, - { - scope: { kind: "workspace", workspaceId }, - requestId: "auto-archive-on-merge", - }, - ); - log.info({ cwd }, "Auto-archived worktree after PR merge"); - } catch (error) { - log.warn({ err: error, cwd }, "Auto-archive after merge failed"); - } - } finally { - inFlight.delete(cwd); + await deps.archiveByScope( + { + paseoHome: options.paseoHome, + paseoWorktreesBaseRoot: options.paseoWorktreesBaseRoot, + github: options.github, + workspaceGitService: options.workspaceGitService, + agentManager: options.agentManager, + agentStorage: options.agentStorage, + findWorkspaceIdForCwd: options.findWorkspaceIdForCwd, + listActiveWorkspaces: options.listActiveWorkspaces, + archiveWorkspaceRecord: (workspaceIdToArchive) => + options.archiveWorkspaceRecord(workspaceIdToArchive, { + autoArchivedChangeRequestUrl: pullRequest.url, + }), + emitWorkspaceUpdatesForWorkspaceIds: options.emitWorkspaceUpdatesForWorkspaceIds, + markWorkspaceArchiving: options.markWorkspaceArchiving, + clearWorkspaceArchiving: options.clearWorkspaceArchiving, + killTerminalsForWorkspace: (workspaceIdToKill) => + deps.killTerminalsForWorkspace( + { + terminalManager: options.terminalManager, + sessionLogger: log, + }, + workspaceIdToKill, + ), + sessionLogger: log, + }, + { + scope: { kind: "workspace", workspaceId }, + requestId: "auto-archive-on-merge", + }, + ); + log.info( + { workspaceId, cwd, branch: pullRequest.headRefName, pullRequestUrl: pullRequest.url }, + "Auto-archived worktree after PR merge", + ); + } catch (error) { + log.warn({ err: error, cwd }, "Auto-archive after merge failed"); } } 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 new file mode 100644 index 000000000..1e13df1dd --- /dev/null +++ b/packages/server/src/server/auto-archive-on-merge/index.test.ts @@ -0,0 +1,235 @@ +import { resolve } from "node:path"; +import type { Logger } from "pino"; +import { expect, test, vi } from "vitest"; + +import { + setupAutoArchiveOnMerge, + type AutoArchiveOnMergeDependencies, + type AutoArchiveOnMergeOptions, +} from "./index.js"; +import type { WorkspaceGitRuntimeSnapshot } from "../workspace-git-service.js"; + +function createSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot { + return { + cwd, + git: { + isGit: true, + repoRoot: cwd, + mainRepoRoot: "/repo", + currentBranch: "feature", + remoteUrl: "https://github.com/acme/repo.git", + isPaseoOwnedWorktree: true, + isDirty: false, + baseRef: "main", + aheadBehind: { ahead: 0, behind: 0 }, + aheadOfOrigin: 0, + behindOfOrigin: 0, + hasRemote: true, + diffStat: { additions: 0, deletions: 0 }, + }, + forge: { + featuresEnabled: true, + authState: "authenticated", + pullRequest: { + url: "https://github.com/acme/repo/pull/12", + title: "Merged", + state: "merged", + baseRefName: "main", + headRefName: "feature", + isMerged: true, + }, + error: null, + }, + }; +} + +test("fans one fresh observation out to every workspace attached to its exact cwd", async () => { + 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, + daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: true }) }, + workspaceGitService: { + onSnapshotUpdated: (listener: (next: WorkspaceGitRuntimeSnapshot) => void) => { + onSnapshotUpdated = listener; + return { unsubscribe: vi.fn() }; + }, + getSnapshot, + }, + listActiveWorkspaces: async () => [ + { workspaceId: "workspace-a", cwd: "/repo/worktree" }, + { workspaceId: "workspace-b", cwd: "/repo/worktree/child/.." }, + { workspaceId: "workspace-other", cwd: "/repo/other" }, + ], + } as unknown as AutoArchiveOnMergeOptions; + const calls: Array<{ workspaceId: string; pullRequest: unknown }> = []; + let resolveFinished: (() => void) | null = null; + const finished = new Promise((resolvePromise) => { + resolveFinished = resolvePromise; + }); + const deps: AutoArchiveOnMergeDependencies = { + archiveIfSafe: async (input) => { + calls.push({ + workspaceId: input.workspaceId, + pullRequest: input.snapshot.forge.pullRequest, + }); + if (calls.length === 2) resolveFinished?.(); + }, + resolvePath: resolve, + }; + + setupAutoArchiveOnMerge(options, deps); + if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(eventSnapshot); + await finished; + + expect(getSnapshot).toHaveBeenCalledTimes(1); + expect(calls).toEqual([ + { workspaceId: "workspace-a", pullRequest: freshSnapshot.forge.pullRequest }, + { workspaceId: "workspace-b", pullRequest: freshSnapshot.forge.pullRequest }, + ]); +}); + +test("serializes the complete fan-out for duplicate merge events on one cwd", async () => { + let onSnapshotUpdated: ((snapshot: WorkspaceGitRuntimeSnapshot) => void) | null = null; + const snapshot = createSnapshot("/repo/worktree/."); + const options = { + logger: { child: () => ({ warn: vi.fn() }) } as unknown as Logger, + daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: true }) }, + workspaceGitService: { + onSnapshotUpdated: (listener: (next: WorkspaceGitRuntimeSnapshot) => void) => { + onSnapshotUpdated = listener; + return { unsubscribe: vi.fn() }; + }, + getSnapshot: async () => snapshot, + }, + listActiveWorkspaces: async () => [ + { workspaceId: "workspace-a", cwd: "/repo/worktree" }, + { workspaceId: "workspace-b", cwd: "/repo/worktree/child/.." }, + ], + } as unknown as AutoArchiveOnMergeOptions; + const archivedWorkspaceIds: string[] = []; + let releaseFirstArchive: (() => void) | null = null; + const firstArchivePaused = new Promise((resolvePromise) => { + releaseFirstArchive = resolvePromise; + }); + let resolveFinished: (() => void) | null = null; + const finished = new Promise((resolvePromise) => { + resolveFinished = resolvePromise; + }); + const deps: AutoArchiveOnMergeDependencies = { + archiveIfSafe: async (input) => { + archivedWorkspaceIds.push(input.workspaceId); + if (input.workspaceId === "workspace-a") { + await firstArchivePaused; + } + if (archivedWorkspaceIds.length === 2) resolveFinished?.(); + }, + resolvePath: resolve, + }; + + setupAutoArchiveOnMerge(options, deps); + if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(snapshot); + await vi.waitFor(() => expect(archivedWorkspaceIds).toEqual(["workspace-a"])); + + onSnapshotUpdated(createSnapshot("/repo/worktree/child/..")); + await Promise.resolve(); + expect(archivedWorkspaceIds).toEqual(["workspace-a"]); + + releaseFirstArchive?.(); + await finished; + expect(archivedWorkspaceIds).toEqual(["workspace-a", "workspace-b"]); +}); + +test("does not fan out a stale merged event when the fresh observation has no PR", async () => { + let onSnapshotUpdated: ((snapshot: WorkspaceGitRuntimeSnapshot) => void) | null = null; + const eventSnapshot = createSnapshot("/repo/worktree"); + const freshSnapshot = createSnapshot("/repo/worktree"); + freshSnapshot.forge.pullRequest = null; + const archiveIfSafe = vi.fn(); + const getSnapshot = vi.fn(async () => freshSnapshot); + const options = { + logger: { child: () => ({ warn: vi.fn() }) } as unknown as Logger, + daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: true }) }, + workspaceGitService: { + onSnapshotUpdated: (listener: (next: WorkspaceGitRuntimeSnapshot) => void) => { + onSnapshotUpdated = listener; + return { unsubscribe: vi.fn() }; + }, + getSnapshot, + }, + listActiveWorkspaces: vi.fn(async () => [ + { workspaceId: "workspace-a", cwd: "/repo/worktree" }, + ]), + } as unknown as AutoArchiveOnMergeOptions; + + setupAutoArchiveOnMerge(options, { archiveIfSafe, resolvePath: resolve }); + if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(eventSnapshot); + await vi.waitFor(() => expect(getSnapshot).toHaveBeenCalledTimes(1)); + await Promise.resolve(); + expect(archiveIfSafe).not.toHaveBeenCalled(); + expect(options.listActiveWorkspaces).not.toHaveBeenCalled(); +}); + +test("logs and skips when the fresh observation cannot be read", async () => { + let onSnapshotUpdated: ((snapshot: WorkspaceGitRuntimeSnapshot) => void) | null = null; + const warn = vi.fn(); + const archiveIfSafe = vi.fn(); + const options = { + logger: { child: () => ({ warn }) } as unknown as Logger, + daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: true }) }, + workspaceGitService: { + onSnapshotUpdated: (listener: (next: WorkspaceGitRuntimeSnapshot) => void) => { + onSnapshotUpdated = listener; + return { unsubscribe: vi.fn() }; + }, + getSnapshot: async () => { + throw new Error("snapshot failed"); + }, + }, + listActiveWorkspaces: vi.fn(), + } as unknown as AutoArchiveOnMergeOptions; + + setupAutoArchiveOnMerge(options, { archiveIfSafe, resolvePath: resolve }); + if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(createSnapshot("/repo/worktree")); + await vi.waitFor(() => + expect(warn).toHaveBeenCalledWith( + { err: expect.any(Error), cwd: "/repo/worktree" }, + "Failed to read snapshot for auto-archive; skipping", + ), + ); + expect(archiveIfSafe).not.toHaveBeenCalled(); + expect(options.listActiveWorkspaces).not.toHaveBeenCalled(); +}); + +test("does not read an observation when auto-archive is disabled", async () => { + let onSnapshotUpdated: ((snapshot: WorkspaceGitRuntimeSnapshot) => void) | null = null; + const getSnapshot = vi.fn(); + const archiveIfSafe = vi.fn(); + const options = { + logger: { child: () => ({ warn: vi.fn() }) } as unknown as Logger, + daemonConfigStore: { get: () => ({ autoArchiveAfterMerge: false }) }, + workspaceGitService: { + onSnapshotUpdated: (listener: (next: WorkspaceGitRuntimeSnapshot) => void) => { + onSnapshotUpdated = listener; + return { unsubscribe: vi.fn() }; + }, + getSnapshot, + }, + listActiveWorkspaces: vi.fn(), + } as unknown as AutoArchiveOnMergeOptions; + + setupAutoArchiveOnMerge(options, { archiveIfSafe, resolvePath: resolve }); + if (!onSnapshotUpdated) throw new Error("Snapshot listener was not registered"); + onSnapshotUpdated(createSnapshot("/repo/worktree")); + expect(getSnapshot).not.toHaveBeenCalled(); + expect(archiveIfSafe).not.toHaveBeenCalled(); +}); 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 0f56bdf24..27608d6bd 100644 --- a/packages/server/src/server/auto-archive-on-merge/index.ts +++ b/packages/server/src/server/auto-archive-on-merge/index.ts @@ -1,25 +1,81 @@ +import { resolve } from "node:path"; import type { Logger } from "pino"; import { archiveIfSafe, type AutoArchiveArchiveOptions } from "./archive-if-safe.js"; -import type { WorkspaceGitSubscription } from "../workspace-git-service.js"; +import type { + WorkspaceGitRuntimeSnapshot, + WorkspaceGitSubscription, +} from "../workspace-git-service.js"; export interface AutoArchiveOnMergeOptions extends AutoArchiveArchiveOptions { logger: Logger; } +export interface AutoArchiveOnMergeDependencies { + archiveIfSafe: typeof archiveIfSafe; + resolvePath: typeof resolve; +} + +const defaultDependencies: AutoArchiveOnMergeDependencies = { + archiveIfSafe, + resolvePath: resolve, +}; + export function setupAutoArchiveOnMerge( options: AutoArchiveOnMergeOptions, + deps: AutoArchiveOnMergeDependencies = defaultDependencies, ): WorkspaceGitSubscription { const log = options.logger.child({ module: "auto-archive-on-merge" }); - const inFlight = new Set(); + const inFlightCwds = new Set(); return options.workspaceGitService.onSnapshotUpdated((snapshot) => { - void archiveIfSafe({ - cwd: snapshot.cwd, - pullRequest: snapshot.forge.pullRequest, - inFlight, - options, - log, - }); + if (!snapshot.forge.pullRequest?.isMerged) { + return; + } + if (options.daemonConfigStore.get().autoArchiveAfterMerge !== true) { + return; + } + + const snapshotCwd = deps.resolvePath(snapshot.cwd); + if (inFlightCwds.has(snapshotCwd)) { + return; + } + inFlightCwds.add(snapshotCwd); + + void (async () => { + let freshSnapshot: WorkspaceGitRuntimeSnapshot | null; + try { + freshSnapshot = await options.workspaceGitService.getSnapshot(snapshot.cwd, { + reason: "auto-archive-on-merge", + }); + } catch (error) { + log.warn( + { err: error, cwd: snapshot.cwd }, + "Failed to read snapshot for auto-archive; skipping", + ); + return; + } + if (!freshSnapshot?.forge.pullRequest?.isMerged) { + return; + } + + const attachedWorkspaces = (await options.listActiveWorkspaces()).filter( + (workspace) => deps.resolvePath(workspace.cwd) === snapshotCwd, + ); + for (const workspace of attachedWorkspaces) { + await deps.archiveIfSafe({ + workspaceId: workspace.workspaceId, + snapshot: freshSnapshot, + options, + log, + }); + } + })() + .catch((error) => { + log.warn({ err: error, cwd: snapshot.cwd }, "Failed to auto-archive attached workspaces"); + }) + .finally(() => { + inFlightCwds.delete(snapshotCwd); + }); }); } 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 new file mode 100644 index 000000000..73a695106 --- /dev/null +++ b/packages/server/src/server/daemon-e2e/auto-archive-unrelated-github-merge.real.e2e.test.ts @@ -0,0 +1,189 @@ +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 { afterEach, expect, test } from "vitest"; + +import { createWorktree } from "../../utils/worktree.js"; +import { + createDaemonTestContext, + createTempGithubRepoName, + type DaemonTestContext, +} from "../test-utils/index.js"; + +const cleanupContexts = new Set(); +const cleanupPaths = new Set(); +const cleanupRepos = new Set(); + +function run(cwd: string, command: string, args: string[]): string { + return execFileSync(command, args, { cwd, stdio: "pipe" }).toString().trim(); +} + +function hasGitHubTestAccess(): boolean { + try { + const auth = run(process.cwd(), "gh", ["auth", "status", "-h", "github.com"]); + return auth.length >= 0; + } catch { + return false; + } +} + +const githubTest = hasGitHubTestAccess() ? test : test.skip; + +async function activeWorkspaceIds(context: DaemonTestContext): Promise { + return (await context.client.fetchWorkspaces()).entries.map((workspace) => workspace.id); +} + +async function waitForWorkspaceToArchive( + context: DaemonTestContext, + workspaceId: string, +): Promise { + const deadline = Date.now() + 15_000; + while (Date.now() < deadline) { + if (!(await activeWorkspaceIds(context)).includes(workspaceId)) return; + await new Promise((resolve) => setTimeout(resolve, 250)); + } + throw new Error(`Workspace ${workspaceId} was not archived`); +} + +afterEach(async () => { + await Promise.all( + Array.from(cleanupContexts, (context) => context.cleanup().catch(() => undefined)), + ); + cleanupContexts.clear(); + for (const repo of cleanupRepos) { + try { + run(process.cwd(), "gh", ["repo", "delete", repo, "--yes"]); + } catch { + // Preserve the original test failure; the temp-repo prefix supports bulk cleanup. + } + } + cleanupRepos.clear(); + for (const cleanupPath of cleanupPaths) { + rmSync(cleanupPath, { recursive: true, force: true }); + } + cleanupPaths.clear(); +}); + +githubTest( + "does not retarget a never-pushed workspace when its checkout moves onto a merged PR branch", + async () => { + const repository = realpathSync(mkdtempSync(path.join(tmpdir(), "paseo-github-merge-"))); + cleanupPaths.add(repository); + run(repository, "git", ["init", "-b", "main"]); + run(repository, "git", ["config", "user.email", "paseo-test@example.com"]); + run(repository, "git", ["config", "user.name", "Paseo Test"]); + writeFileSync(path.join(repository, "README.md"), "producer repro\n"); + run(repository, "git", ["add", "README.md"]); + run(repository, "git", ["-c", "commit.gpgsign=false", "commit", "-m", "initial"]); + + const owner = run(repository, "gh", ["api", "user", "--jq", ".login"]); + const repoName = createTempGithubRepoName("unrelated-merge"); + const repoFullName = `${owner}/${repoName}`; + run(repository, "gh", [ + "api", + "-X", + "POST", + "user/repos", + "-f", + `name=${repoName}`, + "-f", + "private=true", + ]); + cleanupRepos.add(repoFullName); + const token = encodeURIComponent(run(repository, "gh", ["auth", "token"])); + run(repository, "git", [ + "remote", + "add", + "origin", + `https://x-access-token:${token}@github.com/${repoFullName}.git`, + ]); + run(repository, "git", ["push", "-u", "origin", "main"]); + + const context = await createDaemonTestContext({ autoArchiveAfterMerge: true }); + cleanupContexts.add(context); + const merged = await createWorktree({ + cwd: repository, + worktreeSlug: "merged-feature", + source: { kind: "branch-off", baseBranch: "main", branchName: "merged-feature" }, + runSetup: false, + paseoHome: context.daemon.paseoHome, + }); + const unrelated = await createWorktree({ + cwd: repository, + worktreeSlug: "never-pushed-unrelated", + source: { kind: "branch-off", baseBranch: "main", branchName: "never-pushed-unrelated" }, + runSetup: false, + paseoHome: context.daemon.paseoHome, + }); + const mergedWorkspace = await context.client.createWorkspace({ + source: { kind: "directory", path: merged.worktreePath }, + }); + const unrelatedWorkspace = await context.client.createWorkspace({ + source: { kind: "directory", path: unrelated.worktreePath }, + }); + if (!mergedWorkspace.workspace || !unrelatedWorkspace.workspace) { + throw new Error("Failed to register both worktrees"); + } + await context.client.fetchWorkspaces({ + subscribe: { subscriptionId: "real-unrelated-merge" }, + }); + + writeFileSync(path.join(merged.worktreePath, "feature.txt"), "merged\n"); + run(merged.worktreePath, "git", ["add", "feature.txt"]); + run(merged.worktreePath, "git", [ + "-c", + "commit.gpgsign=false", + "commit", + "-m", + "merged feature", + ]); + run(merged.worktreePath, "git", ["push", "-u", "origin", "merged-feature"]); + const pullRequestUrl = run(merged.worktreePath, "gh", [ + "pr", + "create", + "--repo", + repoFullName, + "--base", + "main", + "--head", + "merged-feature", + "--title", + "Merged feature", + "--body", + "Producer-level auto-archive reproduction.", + ]); + + const beforeMerge = await activeWorkspaceIds(context); + expect(beforeMerge).toEqual( + expect.arrayContaining([mergedWorkspace.workspace.id, unrelatedWorkspace.workspace.id]), + ); + run(merged.worktreePath, "gh", [ + "pr", + "merge", + pullRequestUrl, + "--repo", + repoFullName, + "--squash", + "--delete-branch", + ]); + + expect((await context.client.checkoutRefresh(merged.worktreePath)).success).toBe(true); + await waitForWorkspaceToArchive(context, mergedWorkspace.workspace.id); + + expect((await context.client.checkoutRefresh(unrelated.worktreePath)).success).toBe(true); + const unrelatedStatus = await context.client.checkoutPrStatus(unrelated.worktreePath); + 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"]); + 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(await activeWorkspaceIds(context)).toContain(unrelatedWorkspace.workspace.id); + }, + 180_000, +); diff --git a/packages/server/src/server/test-utils/paseo-daemon.ts b/packages/server/src/server/test-utils/paseo-daemon.ts index 3891d90ea..cc427463a 100644 --- a/packages/server/src/server/test-utils/paseo-daemon.ts +++ b/packages/server/src/server/test-utils/paseo-daemon.ts @@ -47,6 +47,7 @@ interface TestPaseoDaemonOptions { webUi?: PaseoDaemonConfig["webUi"]; trustedProxies?: PaseoDaemonConfig["trustedProxies"]; agentProfiles?: AgentProfile[]; + autoArchiveAfterMerge?: boolean; } export interface TestPaseoDaemon { @@ -197,6 +198,7 @@ async function prepareTestDaemonConfig( dictationFinalTimeoutMs: options.dictationFinalTimeoutMs, downloadTokenTtlMs: options.downloadTokenTtlMs, agentProfiles: options.agentProfiles, + autoArchiveAfterMerge: options.autoArchiveAfterMerge, }; return { config, paseoHomeRoot, paseoHome, staticDir }; } diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index 6b39ddc1b..52fa6633f 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -2569,6 +2569,9 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { target.latestFacts?.isGit && target.latestFacts.currentBranch === git.currentBranch ? target.latestFacts.pullRequestLookupTarget : null; + if (target.latestFacts?.isGit && target.latestFacts.paseoWorktree.isPaseoOwnedWorktree) { + return lookupTarget; + } if (lookupTarget) { return lookupTarget; } diff --git a/packages/server/src/server/workspace-registry-bootstrap.ts b/packages/server/src/server/workspace-registry-bootstrap.ts index 64bf2d4eb..cf192a44b 100644 --- a/packages/server/src/server/workspace-registry-bootstrap.ts +++ b/packages/server/src/server/workspace-registry-bootstrap.ts @@ -15,6 +15,7 @@ import { type ProjectRegistry, type WorkspaceRegistry, } from "./workspace-registry.js"; +import { pinPaseoWorktreeBranchIdentityIfMissing } from "../utils/worktree-metadata.js"; function minIsoDate(left: string | null, right: string | null): string | null { if (!left) { @@ -60,6 +61,28 @@ export async function bootstrapWorkspaceRegistries(options: { await Promise.all([options.projectRegistry.initialize(), options.workspaceRegistry.initialize()]); + // COMPAT(worktree-branch-identity): added in v0.4.0 on 2026-08-15; remove after + // 2027-02-15. Older worktrees did not pin branch-off/check-out branch identity. + // Seed it from the registry value clients already display, never from live Git. + for (const workspace of await options.workspaceRegistry.list()) { + if ( + workspace.archivedAt || + !workspace.isPaseoOwnedWorktree || + !workspace.worktreeRoot || + !workspace.branch + ) { + continue; + } + try { + pinPaseoWorktreeBranchIdentityIfMissing(workspace.worktreeRoot, workspace.branch); + } catch (error) { + options.logger.warn( + { err: error, workspaceId: workspace.workspaceId }, + "Failed to pin legacy worktree branch identity; PR association remains disabled", + ); + } + } + if (projectsExists && workspacesExists) { await backfillWorkspaceIdForLegacyAgents(options); return; diff --git a/packages/server/src/server/worktree-core.posix.test.ts b/packages/server/src/server/worktree-core.posix.test.ts index 94f654de1..42a846de3 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("derives a deduped PR lookup target from git config when metadata has no target", async () => { + test("does not derive a deduped 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}`], { @@ -1145,7 +1145,7 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => { expect(facts).toMatchObject({ isGit: true, currentBranch: "daemon-shutdown-diagnostics-1", - pullRequestLookupTarget: { headRef: "daemon-shutdown-diagnostics" }, + pullRequestLookupTarget: null, }); }); diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index ba5a7fde1..f38adff41 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -2619,7 +2619,7 @@ const x = 1; expect(lookupTarget?.headSha).toMatch(/^[0-9a-f]{40}$/); }); - it("reconciles a PR worktree lookup from current branch tracking", async () => { + it("does not retarget a PR worktree lookup from current branch tracking", async () => { execFileSync("git", ["remote", "add", "origin", "https://github.com/acme/repo.git"], { cwd: repoDir, }); @@ -2657,8 +2657,7 @@ const x = 1; const switchedTarget = await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome); const commands = stopGitCommandMetrics().commands.map((command) => command.args[0]); - expect(switchedTarget).toMatchObject({ headRef: "new-change" }); - expect(switchedTarget).not.toHaveProperty("headRepositoryOwner"); + expect(switchedTarget).toBeNull(); expect(commands).not.toContain("fetch"); writePaseoWorktreeMetadata(workspaceDir, { @@ -2669,7 +2668,8 @@ const x = 1; }, }); expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ - headRef: "new-change", + headRef: "old-change", + headRepositoryOwner: "contributor", }); execFileSync( @@ -2696,10 +2696,52 @@ const x = 1; cwd: repoDir, }); expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ - headRef: "local-upstream", + headRef: "old-change", + headRepositoryOwner: "contributor", }); }); + it("does not replace a managed branch pin from shared push config", async () => { + execFileSync("git", ["branch", "feature/pinned"], { cwd: repoDir }); + execFileSync("git", ["remote", "add", "origin", "https://github.com/acme/repo.git"], { + cwd: repoDir, + }); + execFileSync("git", ["remote", "add", "fork", "https://github.com/other/repo.git"], { + cwd: repoDir, + }); + const workspaceDir = join(paseoHome, "worktrees", "repo", "pinned-worktree"); + mkdirSync(join(paseoHome, "worktrees", "repo"), { recursive: true }); + execFileSync("git", ["worktree", "add", workspaceDir, "feature/pinned"], { cwd: repoDir }); + writePaseoWorktreeMetadata(workspaceDir, { + baseRefName: "main", + changeRequestLookupTarget: { + headRef: "feature/pinned", + localBranchName: "feature/pinned", + }, + }); + execFileSync("git", ["config", "branch.feature/pinned.pushRemote", "fork"], { + cwd: repoDir, + }); + execFileSync("git", ["config", "remote.fork.push", "HEAD:refs/heads/unrelated"], { + cwd: repoDir, + }); + + expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toMatchObject({ + headRef: "feature/pinned", + }); + }); + + it("fails closed 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 }); + execFileSync("git", ["worktree", "add", workspaceDir, "feature/unpinned"], { + cwd: repoDir, + }); + + expect(await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome)).toBeNull(); + }); + it("does not apply ambiguous legacy PR metadata to a suffixed branch", async () => { execFileSync("git", ["branch", "contributor/old-change-1"], { cwd: repoDir }); const workspaceDir = join(paseoHome, "worktrees", "repo", "legacy-pr-worktree"); @@ -2718,8 +2760,7 @@ const x = 1; const lookupTarget = await readPullRequestLookupTargetFromFacts(workspaceDir, paseoHome); - expect(lookupTarget).toMatchObject({ headRef: "contributor/old-change-1" }); - expect(lookupTarget).not.toHaveProperty("headRepositoryOwner"); + expect(lookupTarget).toBeNull(); }); it("does not apply a legacy fork hint to an ownerless branch with the same head", async () => { @@ -2762,8 +2803,7 @@ const x = 1; { paseoHome, facts: ownerlessFacts }, ); - expect(requestedTargets.at(-1)).toMatchObject({ headRef: "old-change" }); - expect(requestedTargets.at(-1)).not.toHaveProperty("headRepositoryOwner"); + expect(requestedTargets).toHaveLength(1); }); it("recognizes a normalized GitHub owner branch from legacy Enterprise metadata", async () => { @@ -2830,7 +2870,7 @@ const x = 1; expect(requestedTargets.at(-1)).toMatchObject({ headRef: "old-change", - headRepositoryOwner: "OtherOwner", + headRepositoryOwner: "MixedOwner", }); }); @@ -2878,7 +2918,34 @@ const x = 1; { paseoHome, facts: switchedFacts }, ); - expect(requestedTargets.at(-1)).toMatchObject({ headRef: "other-branch" }); + expect(requestedTargets).toHaveLength(1); + }); + + it("moves a managed branch identity pin when its branch is renamed", async () => { + execFileSync("git", ["branch", "feature/placeholder"], { cwd: repoDir }); + const workspaceDir = join(paseoHome, "worktrees", "repo", "renamed-worktree"); + mkdirSync(join(paseoHome, "worktrees", "repo"), { recursive: true }); + execFileSync("git", ["worktree", "add", workspaceDir, "feature/placeholder"], { + cwd: repoDir, + }); + writePaseoWorktreeMetadata(workspaceDir, { + baseRefName: "main", + changeRequestLookupTarget: { + headRef: "feature/placeholder", + localBranchName: "feature/placeholder", + }, + }); + + await renameCurrentBranch(workspaceDir, "feature/generated"); + + expect(readPaseoWorktreeMetadata(workspaceDir)?.changeRequestLookupTarget).toEqual({ + headRef: "feature/generated", + localBranchName: "feature/generated", + }); + const facts = await getCheckoutSnapshotFacts(workspaceDir, { paseoHome }); + expect(facts.isGit && facts.pullRequestLookupTarget).toMatchObject({ + headRef: "feature/generated", + }); }); it("keeps fork identity when the local and tracked branch names match", async () => { diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 16cb20775..628d2716b 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -1824,6 +1824,12 @@ 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), ); @@ -1883,6 +1889,47 @@ async function resolvePullRequestLookupTargetFromPushConfig( }); } +async function resolveFactsPullRequestLookupTarget(input: { + cwd: string; + inspected: CheckoutInspectionContext; + metadata: PaseoWorktreeMetadata | null; + branchRemoteName: string | null; + branchMergeRef: string | null; + branchRemoteUrl: string | null; + resolvedBaseRef: string | null; + context?: CheckoutContext; +}): Promise { + const { cwd, inspected, metadata, context } = input; + if (inspected.paseoWorktree.isPaseoOwnedWorktree) { + return buildPullRequestLookupTargetFromMetadata(metadata, inspected.currentBranch ?? ""); + } + + let target = buildInitialPullRequestLookupTarget({ + currentBranch: inspected.currentBranch, + metadata, + branchRemoteName: input.branchRemoteName, + branchMergeRef: input.branchMergeRef, + branchRemoteUrl: input.branchRemoteUrl, + originRemoteUrl: inspected.remoteUrl, + resolvedBaseRef: input.resolvedBaseRef, + }); + if ( + inspected.currentBranch && + target?.headRef === inspected.currentBranch && + !target.headRepositoryOwner + ) { + target = + (await resolvePullRequestLookupTargetFromPushConfig( + cwd, + inspected.currentBranch, + inspected.remoteUrl, + input.resolvedBaseRef, + context, + )) ?? target; + } + return target; +} + export async function getCheckoutSnapshotFacts( cwd: string, context?: CheckoutContext, @@ -1938,29 +1985,16 @@ export async function getCheckoutSnapshotFacts( ]); } } - let pullRequestLookupTarget = buildInitialPullRequestLookupTarget({ - currentBranch: inspected.currentBranch, + let pullRequestLookupTarget = await resolveFactsPullRequestLookupTarget({ + cwd, + inspected, metadata: paseoWorktreeMetadata, branchRemoteName, branchMergeRef, branchRemoteUrl, - originRemoteUrl: inspected.remoteUrl, resolvedBaseRef, + context, }); - if ( - inspected.currentBranch && - pullRequestLookupTarget?.headRef === inspected.currentBranch && - !pullRequestLookupTarget.headRepositoryOwner - ) { - pullRequestLookupTarget = - (await resolvePullRequestLookupTargetFromPushConfig( - cwd, - inspected.currentBranch, - inspected.remoteUrl, - resolvedBaseRef, - context, - )) ?? pullRequestLookupTarget; - } pullRequestLookupTarget = await addHeadShaToPullRequestLookupTarget( cwd, pullRequestLookupTarget, @@ -3952,9 +3986,8 @@ async function getPullRequestStatusUncached( context?: CheckoutContext, headSha?: string | null, ): Promise { - if (context?.facts?.isGit === false) { - return buildPullRequestStatusResult(null, "no_remote"); - } + const unavailable = getUnavailablePullRequestStatus(context?.facts); + if (unavailable) return unavailable; if (!context?.facts?.isGit) { await requireGitRepo(cwd); } @@ -3995,3 +4028,19 @@ async function getPullRequestStatusUncached( throw error; } } + +function getUnavailablePullRequestStatus( + facts: CheckoutSnapshotFacts | null | undefined, +): PullRequestStatusResult | null { + if (facts?.isGit === false) { + return buildPullRequestStatusResult(null, "no_remote"); + } + if ( + facts?.isGit === true && + facts.paseoWorktree.isPaseoOwnedWorktree && + facts.pullRequestLookupTarget === null + ) { + return buildPullRequestStatusResult(null, "authenticated"); + } + return null; +} diff --git a/packages/server/src/utils/worktree-metadata.ts b/packages/server/src/utils/worktree-metadata.ts index 4c44aaf0f..e07691ba1 100644 --- a/packages/server/src/utils/worktree-metadata.ts +++ b/packages/server/src/utils/worktree-metadata.ts @@ -105,12 +105,36 @@ export function rebindPaseoWorktreeChangeRequestHint( ...metadata, changeRequestLookupTarget: { ...target, + ...(target.headRef === previousBranch && + !target.headRepositoryOwner && + target.changeRequestNumber === undefined + ? { headRef: currentBranch } + : {}), localBranchName: currentBranch, }, }); return true; } +export function pinPaseoWorktreeBranchIdentityIfMissing( + worktreeRoot: string, + branch: string, +): boolean { + const metadata = readPaseoWorktreeMetadata(worktreeRoot); + if (!metadata || metadata.changeRequestLookupTarget) { + return false; + } + const target = createPaseoWorktreeChangeRequestHint({ + headRef: branch, + localBranchName: branch, + }); + writePaseoWorktreeMetadataFile(worktreeRoot, { + ...metadata, + changeRequestLookupTarget: target, + }); + return true; +} + function getGitDirForWorktreeRoot(worktreeRoot: string): string { const gitPath = join(worktreeRoot, ".git"); if (!existsSync(gitPath)) { diff --git a/packages/server/src/utils/worktree.posix.test.ts b/packages/server/src/utils/worktree.posix.test.ts index 36e1642f7..6efae9b50 100644 --- a/packages/server/src/utils/worktree.posix.test.ts +++ b/packages/server/src/utils/worktree.posix.test.ts @@ -21,7 +21,7 @@ import { type WorktreeConfig, } from "./worktree"; import type { PaseoConfig } from "@getpaseo/protocol/paseo-config-schema"; -import { getPaseoWorktreeMetadataPath } from "./worktree-metadata.js"; +import { getPaseoWorktreeMetadataPath, readPaseoWorktreeMetadata } from "./worktree-metadata.js"; import { getCheckoutDiff, getCheckoutStatus, @@ -128,7 +128,11 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => { const metadataPath = getPaseoWorktreeMetadataPath(result.worktreePath); expect(existsSync(metadataPath)).toBe(true); const metadata = JSON.parse(readFileSync(metadataPath, "utf8")); - expect(metadata).toMatchObject({ version: 1, baseRefName: "main" }); + expect(metadata).toMatchObject({ + version: 1, + baseRefName: "main", + changeRequestLookupTarget: { headRef: "hello-world", localBranchName: "hello-world" }, + }); }); it("creates and owns worktrees under a configured root", async () => { @@ -254,7 +258,11 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => { const metadataPath = getPaseoWorktreeMetadataPath(result.worktreePath); const metadata = JSON.parse(readFileSync(metadataPath, "utf8")); - expect(metadata).toMatchObject({ version: 1, baseRefName: "main" }); + expect(metadata).toMatchObject({ + version: 1, + baseRefName: "main", + changeRequestLookupTarget: { headRef: "feature/x", localBranchName: "feature/x" }, + }); }); it("checks out an existing local branch that is not checked out elsewhere", async () => { @@ -278,7 +286,11 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => { const metadataPath = getPaseoWorktreeMetadataPath(result.worktreePath); const metadata = JSON.parse(readFileSync(metadataPath, "utf8")); - expect(metadata).toMatchObject({ version: 1, baseRefName: "dev" }); + expect(metadata).toMatchObject({ + version: 1, + baseRefName: "dev", + changeRequestLookupTarget: { headRef: "dev", localBranchName: "dev" }, + }); }); it("checks out an existing local branch whose name contains uppercase letters and dots", async () => { @@ -312,6 +324,10 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => { expect(result.branchName).toBe("main-1"); expect(existsSync(result.worktreePath)).toBe(true); + expect(readPaseoWorktreeMetadata(result.worktreePath)?.changeRequestLookupTarget).toEqual({ + headRef: "main-1", + localBranchName: "main-1", + }); }); it("fetches a GitHub PR branch, checks it out, writes metadata, and runs setup", async () => { diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index e3cb98aba..594ea0068 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -1322,6 +1322,10 @@ async function resolveWorktreeSourcePlan({ branchName: newBranchName, metadataBaseRefName: normalizedBaseBranch, metadataBaseRef: resolvedBaseBranch, + changeRequestLookupTarget: createPaseoWorktreeChangeRequestHint({ + headRef: newBranchName, + localBranchName: newBranchName, + }), addArguments: ["-b", newBranchName, "--no-track", base], }; } @@ -1342,6 +1346,10 @@ async function resolveWorktreeSourcePlan({ return { branchName, metadataBaseRefName: source.branchName, + changeRequestLookupTarget: createPaseoWorktreeChangeRequestHint({ + headRef: branchName, + localBranchName: branchName, + }), addArguments: ["-b", branchName, "--no-track", source.branchName], }; } @@ -1349,6 +1357,10 @@ async function resolveWorktreeSourcePlan({ return { branchName: source.branchName, metadataBaseRefName: source.branchName, + changeRequestLookupTarget: createPaseoWorktreeChangeRequestHint({ + headRef: source.branchName, + localBranchName: source.branchName, + }), addArguments: [source.branchName], }; }