diff --git a/docs/data-model.md b/docs/data-model.md index 5bd96f5252..c95fced196 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -488,7 +488,7 @@ Array of workspace records. A workspace is a specific working directory within a | `title` | `string \| null` | User-set name override layered over `displayName`. Null means "use `displayName`". | | `branch` | `string \| null` | The current Git branch for git-backed workspaces. Separate from `displayName`/`title`; a background branch refresh never rewrites the name. | | `worktreeRoot` | `string \| null` | Backing checkout/worktree root. May differ from `cwd` for exact subprojects and remains persisted after the worktree is deleted so restore can reproduce the placement. | -| `baseBranch` | `string \| null` | Normalized branch the Paseo worktree was created from; null for directories, local checkouts, and checkout-branch worktrees | +| `baseBranch` | `string \| null` | Comparison base retained across archive and restore. Branch-off creation stores the resolved ref; legacy and PR-checkout records hold a bare name. Null means no recorded base. | | `isPaseoOwnedWorktree` | `boolean` | Whether Paseo owns and may remove/recreate the backing `worktreeRoot` | | `mainRepoRoot` | `string \| null` | Main repository root for worktree checkouts, independent of both exact `cwd` and backing `worktreeRoot` | | `createdAt` | `string` (ISO 8601) | | diff --git a/packages/app/e2e/browser/workspace-draft-thinking-preferences.spec.ts b/packages/app/e2e/browser/workspace-draft-thinking-preferences.spec.ts index 27b0bb98bb..cd8f3b0c03 100644 --- a/packages/app/e2e/browser/workspace-draft-thinking-preferences.spec.ts +++ b/packages/app/e2e/browser/workspace-draft-thinking-preferences.spec.ts @@ -1,5 +1,5 @@ import { expect, test, type Page } from "../support/fixtures"; -import { clickNewChat, gotoWorkspace } from "../support/helpers/launcher"; +import { gotoWorkspace } from "../support/helpers/launcher"; import { seedWorkspace } from "../support/helpers/seed-client"; import { chooseCommandCenterAgentControl, @@ -7,13 +7,15 @@ import { waitForDraftComposer, } from "../support/helpers/command-center-agent-controls"; import { openCommandCenter } from "../support/helpers/command-center"; +import { runWorkspaceActionFromCommandCenter } from "../support/helpers/command-center-workspace-actions"; import { selectModel } from "../support/helpers/app"; const DISABLE_DEFAULT_SEED_ONCE_KEY = "@paseo:e2e-disable-default-seed-once"; const SEED_NONCE_KEY = "@paseo:e2e-seed-nonce"; async function openNewAgentTab(page: Page): Promise { - await clickNewChat(page); + // Preference persistence does not depend on the animated tab-creation menu. + await runWorkspaceActionFromCommandCenter(page, "New agent"); await waitForDraftComposer(page); } diff --git a/packages/app/e2e/browser/worktree-restore-after-restart.spec.ts b/packages/app/e2e/browser/worktree-restore-after-restart.spec.ts index 7061d14576..ea04da4a55 100644 --- a/packages/app/e2e/browser/worktree-restore-after-restart.spec.ts +++ b/packages/app/e2e/browser/worktree-restore-after-restart.spec.ts @@ -1,5 +1,11 @@ import { randomUUID } from "node:crypto"; -import { existsSync } from "node:fs"; +import { existsSync, writeFileSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { join } from "node:path"; +import { + restoreWorkspaceFromHistory, + expectCommittedFileAfterReload, +} from "../support/helpers/workspace-recovery"; import { expect, type Page } from "@playwright/test"; import { metroTest as test } from "../support/fixtures"; import { gotoAppShell } from "../support/helpers/app"; @@ -80,7 +86,7 @@ test.describe("Worktree restore after daemon restart", () => { ); } - test("after archiving a worktree and restarting the daemon, History shows the worktree branch (not main) before any restore", async ({ + test("after archiving a worktree and restarting the daemon, History shows the worktree branch (not main) and restores its committed changes", async ({ page, }) => { // A paseo worktree is cut on its own branch named after the slug, and the @@ -97,6 +103,10 @@ test.describe("Worktree restore after daemon restart", () => { createdProjectIds.add(worktree.projectKey); createdWorktreeDirectories.add(worktree.workspaceDirectory); + writeFileSync(join(worktree.workspaceDirectory, "restart-change.txt"), "persisted base\n"); + execFileSync("git", ["add", "restart-change.txt"], { cwd: worktree.workspaceDirectory }); + execFileSync("git", ["commit", "-m", "Survive restart"], { cwd: worktree.workspaceDirectory }); + const agent = await createMockIdleAgent(client, { cwd: worktree.workspaceDirectory, workspaceId: worktree.workspaceId, @@ -138,5 +148,7 @@ test.describe("Worktree restore after daemon restart", () => { await expect(branchCell).toBeVisible({ timeout: 60_000 }); await expect(branchCell).toHaveText(worktreeSlug, { timeout: 60_000 }); await expect(workspaceCell).toHaveText(worktree.workspaceName, { timeout: 60_000 }); + await restoreWorkspaceFromHistory(page, serverId, agent.id); + await expectCommittedFileAfterReload(page, "restart-change.txt"); }); }); diff --git a/packages/app/e2e/browser/worktree-restore.spec.ts b/packages/app/e2e/browser/worktree-restore.spec.ts index 3926b43a97..479a69ffb8 100644 --- a/packages/app/e2e/browser/worktree-restore.spec.ts +++ b/packages/app/e2e/browser/worktree-restore.spec.ts @@ -1,6 +1,13 @@ import { randomUUID } from "node:crypto"; import { execFileSync } from "node:child_process"; -import { existsSync } from "node:fs"; +import { existsSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { + restoreArchivedWorkspace, + expectCommittedFile, + expectCommittedFileAfterReload, +} from "../support/helpers/workspace-recovery"; +import { archiveWorkspaceFromSidebar } from "../support/helpers/sidebar"; import { rename } from "node:fs/promises"; import type { Page } from "@playwright/test"; import { buildHostWorkspaceRoute } from "@/utils/host-routes"; @@ -139,6 +146,33 @@ test.describe("Worktree restore", () => { await tempRepo?.cleanup().catch(() => undefined); }); + async function createWorkspaceWithCommittedChange() { + const worktree = await createWorktreeViaDaemon(worktreeClient, { + cwd: tempRepo.path, + slug: `restore-diff-${randomUUID().slice(0, 8)}`, + }); + createdProjectIds.add(worktree.projectKey); + createdWorktreeDirectories.add(worktree.workspaceDirectory); + commitRestorableChange(worktree.workspaceDirectory); + const agent = await createIdleAgent(client, { + cwd: worktree.workspaceDirectory, + workspaceId: worktree.workspaceId, + title: "Restore committed changes", + }); + const route = `${buildHostWorkspaceRoute(getServerId(), worktree.workspaceId)}?open=${encodeURIComponent(`agent:${agent.id}`)}`; + + return { worktree, route }; + } + + test("committed changes survive archive, restore and page reload", async ({ page }) => { + const { worktree, route } = await createWorkspaceWithCommittedChange(); + await openCommittedWorkspace(page, route); + await archiveCommittedWorkspace(page, worktree); + await restoreCommittedWorkspace(page, route); + await expectCommittedFileAfterReload(page, "restored-change.txt"); + await expectRestoredCommitDiff(page); + }); + test("opening an active History agent navigates without restoring or unarchiving", async ({ page, }) => { @@ -387,3 +421,40 @@ test.describe("Worktree restore", () => { } }); }); + +function commitRestorableChange(cwd: string): void { + writeFileSync(path.join(cwd, "restored-change.txt"), "preserved after restore\n"); + execFileSync("git", ["add", "restored-change.txt"], { cwd, stdio: "pipe" }); + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "Preserve restored diff"], { + cwd, + stdio: "pipe", + }); +} + +async function openCommittedWorkspace(page: Page, route: string): Promise { + await page.goto(route); + await expectCommittedFile(page, "restored-change.txt"); +} + +async function archiveCommittedWorkspace( + page: Page, + worktree: { workspaceId: string; workspaceDirectory: string }, +): Promise { + await archiveWorkspaceFromSidebar(page, worktree.workspaceId); + await expect.poll(() => existsSync(worktree.workspaceDirectory)).toBe(false); +} + +async function restoreCommittedWorkspace(page: Page, route: string): Promise { + await page.goto(route); + await restoreArchivedWorkspace(page); +} + +async function expectRestoredCommitDiff(page: Page): Promise { + await page.getByRole("button", { name: /Commits/i }).click(); + await page.getByText("Preserve restored diff", { exact: true }).click(); + // Diff file headers have no semantic role; their accessible names identify the file and counts. + await expect( + page.getByTestId("commit-diff-panel").filter({ visible: true }).getByTestId("diff-file-0"), + ).toHaveAccessibleName("restored-change.txt, +1, -0"); + await page.screenshot({ path: test.info().outputPath("restored-commit.png") }); +} diff --git a/packages/app/e2e/support/helpers/workspace-recovery.ts b/packages/app/e2e/support/helpers/workspace-recovery.ts new file mode 100644 index 0000000000..17c90fddb3 --- /dev/null +++ b/packages/app/e2e/support/helpers/workspace-recovery.ts @@ -0,0 +1,38 @@ +import { expect, type Page } from "@playwright/test"; +import { openChangesPanel } from "./branch-switcher"; + +export async function restoreArchivedWorkspace(page: Page): Promise { + await expect(page.getByText("Workspace archived", { exact: true })).toBeVisible({ + timeout: 30_000, + }); + await page.getByRole("button", { name: "Restore", exact: true }).click(); + await expect(page.getByText("Workspace archived", { exact: true })).toHaveCount(0, { + timeout: 30_000, + }); +} + +export async function restoreWorkspaceFromHistory( + page: Page, + serverId: string, + agentId: string, +): Promise { + // History rows have no unique accessible name; their stable ID identifies the session. + await page.getByTestId(`agent-row-${serverId}-${agentId}`).click(); + await restoreArchivedWorkspace(page); +} + +export async function expectCommittedFile(page: Page, filename: string): Promise { + await openChangesPanel(page); + await expect(page.getByText(filename, { exact: true }).filter({ visible: true })).toBeVisible({ + timeout: 30_000, + }); + await expect( + page.getByText("No changes to display", { exact: true }).filter({ visible: true }), + ).toHaveCount(0); +} + +export async function expectCommittedFileAfterReload(page: Page, filename: string): Promise { + await expectCommittedFile(page, filename); + await page.reload(); + await expectCommittedFile(page, filename); +} diff --git a/packages/server/src/server/paseo-worktree-service.test.ts b/packages/server/src/server/paseo-worktree-service.test.ts index c40f710d38..21f9951998 100644 --- a/packages/server/src/server/paseo-worktree-service.test.ts +++ b/packages/server/src/server/paseo-worktree-service.test.ts @@ -71,7 +71,7 @@ test("creates a worktree and registers it in the source workspace project withou expect(result.workspace.workspaceId).toMatch(/^wks_[0-9a-f]{16}$/); expect(result.workspace.projectId).toBe("remote:github.com/acme/repo"); expect(result.workspace.displayName).toBe("feature-one"); - expect(result.workspace.baseBranch).toBe("main"); + expect(result.workspace.baseBranch).toBe("refs/heads/main"); expect(result.workspace.title).toBe("Feature One"); expect(deps.workspaceGitService.getSnapshot).not.toHaveBeenCalled(); expect(deps.projects.get(sourceProject.projectId)).toEqual({ diff --git a/packages/server/src/server/paseo-worktree-service.ts b/packages/server/src/server/paseo-worktree-service.ts index dc93e30298..247fe46c18 100644 --- a/packages/server/src/server/paseo-worktree-service.ts +++ b/packages/server/src/server/paseo-worktree-service.ts @@ -20,7 +20,6 @@ import { import { getCurrentBranch, localBranchExists, renameCurrentBranch } from "../utils/checkout-git.js"; import { markPaseoWorktreeFirstAgentBranchAutoNameAttempted, - normalizeBaseRefName, readPaseoWorktreeMetadata, writePaseoWorktreeFirstAgentBranchAutoNameMetadata, } from "../utils/worktree-metadata.js"; @@ -99,7 +98,7 @@ async function createPaseoWorktreeWithPriority( cwd: workspaceCwd, worktreeRoot: createdWorktree.worktree.worktreePath, branch: createdWorktree.worktree.branchName || null, - baseBranch: resolveIntentBaseBranch(createdWorktree.intent), + baseBranch: createdWorktree.worktree.comparisonBaseRef, title: input.title?.trim() || resolveFirstAgentPromptTitle(input.firstAgentContext), expectsInitialAgent: Boolean(input.firstAgentContext), ...(createdWorktree.intent.kind === "checkout-change-request" && @@ -273,18 +272,3 @@ function maybeMarkFirstAgentBranchAutoNameEligible(options: { placeholderBranchName: createdWorktree.worktree.branchName, }); } - -// The base branch is normalized to match worktree.json's baseRefName (origin/ -// stripped). checkout-branch worktrees have no distinct base, so they stay null. -function resolveIntentBaseBranch(intent: WorktreeCreationIntent): string | null { - switch (intent.kind) { - case "branch-off": - return normalizeBaseRefName(intent.baseBranch); - case "checkout-change-request": - return normalizeBaseRefName(intent.baseRefName); - case "checkout-github-pr": - return normalizeBaseRefName(intent.baseRefName); - case "checkout-branch": - return null; - } -} diff --git a/packages/server/src/server/resolve-worktree-creation-intent.ts b/packages/server/src/server/resolve-worktree-creation-intent.ts index 1d6ae13677..b966a6b8aa 100644 --- a/packages/server/src/server/resolve-worktree-creation-intent.ts +++ b/packages/server/src/server/resolve-worktree-creation-intent.ts @@ -1,7 +1,7 @@ import type { ForgeService, PullRequestCheckoutTarget } from "../services/forge-service.js"; import type { WorktreeSource } from "../utils/worktree.js"; -export type WorktreeCreationIntent = WorktreeSource; +export type WorktreeCreationIntent = Exclude; export interface ResolveWorktreeCreationIntentInput { worktreeSlug?: string; diff --git a/packages/server/src/server/session.test.ts b/packages/server/src/server/session.test.ts index 7a6384f9a6..fd4d650e0b 100644 --- a/packages/server/src/server/session.test.ts +++ b/packages/server/src/server/session.test.ts @@ -2457,10 +2457,14 @@ describe("session checkout merge handling", () => { requestId: "request-merge-from-base-success", }); - expect(checkoutGitMocks.mergeFromBase).toHaveBeenCalledWith("/tmp/request-worktree", { - baseRef: "main", - requireCleanTarget: true, - }); + expect(checkoutGitMocks.mergeFromBase).toHaveBeenCalledWith( + "/tmp/request-worktree", + { + baseRef: "main", + requireCleanTarget: true, + }, + { paseoHome: "/tmp/paseo-home", worktreesRoot: undefined }, + ); expect(workspaceGitService.getSnapshot).toHaveBeenCalledWith("/tmp/request-worktree", { force: true, reason: "merge-from-base", @@ -2917,6 +2921,7 @@ diff --git a/file.txt b/file.txt base: "main", }, expect.anything(), + { paseoHome: "/tmp/paseo-home", worktreesRoot: undefined }, ); expect(messages).toContainEqual({ type: "checkout_pr_create_response", @@ -3041,6 +3046,7 @@ diff --git a/file.txt b/file.txt base: "main", }, expect.anything(), + { paseoHome: "/tmp/paseo-home", worktreesRoot: undefined }, ); expect(messages).toContainEqual({ type: "checkout_pr_create_response", diff --git a/packages/server/src/server/session/checkout/checkout-session.test.ts b/packages/server/src/server/session/checkout/checkout-session.test.ts index 98185ddc08..bc65b01d87 100644 --- a/packages/server/src/server/session/checkout/checkout-session.test.ts +++ b/packages/server/src/server/session/checkout/checkout-session.test.ts @@ -27,6 +27,7 @@ import { createNoGitWorkspaceRuntimeSnapshot, createNoopWorkspaceGitService, } from "../../test-utils/workspace-git-service-stub.js"; +import { createWorktree, deletePaseoWorktree } from "../../../utils/worktree.js"; import { expandTilde } from "../../../utils/path.js"; import type { GitMetadataGenerator } from "./git-metadata-generator.js"; @@ -106,6 +107,7 @@ interface RecordedGeneratorCalls { } function makeCheckoutSession(options?: { + paseoHome?: string; git?: Partial; diff?: CheckoutDiffSubscriber; github?: Partial; @@ -171,7 +173,7 @@ function makeCheckoutSession(options?: { checkoutDiffManager: options?.diff ?? createFakeDiffSubscriber({ cwd: "", files: [], error: null }).subscriber, gitMetadataGenerator, - paseoHome: "/tmp/paseo-home", + paseoHome: options?.paseoHome ?? "/tmp/paseo-home", worktreesRoot: undefined, logger: pino({ level: "silent" }), }); @@ -1722,3 +1724,91 @@ describe("CheckoutSession", () => { }); }); }); + +it("creates a PR from a restored exact base using the host metadata and a forge branch name", async () => { + const root = realpathSync(mkdtempSync(join(tmpdir(), "paseo-restored-pr-"))); + const repo = join(root, "repo"); + const remote = join(root, "remote.git"); + const paseoHome = join(root, "home"); + const git = (cwd: string, ...args: string[]) => + execFileSync("git", args, { cwd, encoding: "utf8", stdio: "pipe" }).trim(); + try { + git(root, "init", "-b", "main", repo); + git(repo, "config", "user.name", "Test"); + git(repo, "config", "user.email", "test@example.com"); + git(repo, "commit", "--allow-empty", "-m", "base"); + git(root, "init", "--bare", remote); + git(repo, "remote", "add", "origin", remote); + git(repo, "push", "-u", "origin", "main"); + const baseRef = "refs/remotes/origin/main"; + const created = await createWorktree({ + cwd: repo, + paseoHome, + worktreeSlug: "restored-pr", + source: { kind: "branch-off", baseBranch: baseRef, branchName: "restored-pr" }, + runSetup: false, + }); + writeFileSync(join(created.worktreePath, "feature.txt"), "feature\n"); + git(created.worktreePath, "add", "."); + git(created.worktreePath, "commit", "-m", "feature"); + await deletePaseoWorktree({ cwd: repo, paseoHome, worktreePath: created.worktreePath }); + const restored = await createWorktree({ + cwd: repo, + paseoHome, + worktreeSlug: "restored-pr", + source: { kind: "restore", branchName: created.branchName, baseRef }, + runSetup: false, + }); + const requests: Array[0]> = []; + const service: ForgeService = { + ...createGitHubService(), + createPullRequest: async (input) => { + requests.push(input); + return { url: "https://example.com/pull/1", number: 1 }; + }, + }; + const { checkout, emitted } = makeCheckoutSession({ + paseoHome, + git: { resolveForge: async () => ({ forge: "github", host: "github.com", service }) }, + }); + await checkout.handleCheckoutPrCreateRequest({ + type: "checkout_pr_create_request", + cwd: restored.worktreePath, + baseRef: "refs/heads/main", + title: "Feature", + body: "Description", + requestId: "wrong-base", + }); + expect(emitted.at(-1)).toMatchObject({ + type: "checkout_pr_create_response", + payload: { error: { message: expect.stringContaining("Base ref mismatch") } }, + }); + expect(requests).toHaveLength(0); + await checkout.handleCheckoutPrCreateRequest({ + type: "checkout_pr_create_request", + cwd: restored.worktreePath, + baseRef: "main", + title: "Feature", + body: "Description", + requestId: "right-base", + }); + expect(emitted.at(-1)).toMatchObject({ + type: "checkout_pr_create_response", + payload: { error: null, number: 1 }, + }); + expect(requests).toEqual([ + { + cwd: restored.worktreePath, + base: "main", + head: created.branchName, + title: "Feature", + body: "Description", + }, + ]); + expect(git(remote, "rev-parse", `refs/heads/${created.branchName}`)).toBe( + git(restored.worktreePath, "rev-parse", "HEAD"), + ); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/packages/server/src/server/session/checkout/checkout-session.ts b/packages/server/src/server/session/checkout/checkout-session.ts index 152734603c..419d418050 100644 --- a/packages/server/src/server/session/checkout/checkout-session.ts +++ b/packages/server/src/server/session/checkout/checkout-session.ts @@ -274,7 +274,10 @@ export class CheckoutSession { const { cwd, requestId } = msg; try { - const { baseRef, commits } = await listCheckoutCommits({ cwd: expandTilde(cwd) }); + const { baseRef, commits } = await listCheckoutCommits({ + cwd: expandTilde(cwd), + context: { paseoHome: this.paseoHome, worktreesRoot: this.worktreesRoot }, + }); this.host.emit({ type: "checkout.commits.list.response", payload: { cwd, baseRef, commits, error: null, requestId }, @@ -850,10 +853,14 @@ export class CheckoutSession { } } - await mergeFromBase(cwd, { - baseRef: msg.baseRef, - requireCleanTarget: msg.requireCleanTarget ?? true, - }); + await mergeFromBase( + cwd, + { + baseRef: msg.baseRef, + requireCleanTarget: msg.requireCleanTarget ?? true, + }, + { paseoHome: this.paseoHome, worktreesRoot: this.worktreesRoot }, + ); await this.gitMutation.notifyGitMutation(cwd, "merge-from-base", { invalidateForge: true }); this.scheduleDiffRefresh(cwd); @@ -965,6 +972,7 @@ export class CheckoutSession { base: msg.baseRef, }, service, + { paseoHome: this.paseoHome, worktreesRoot: this.worktreesRoot }, ); await this.gitMutation.notifyGitMutation(cwd, "create-pr", { invalidateForge: true }); diff --git a/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.test.ts b/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.test.ts index 91631a5631..82ffafa4d7 100644 --- a/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.test.ts +++ b/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.test.ts @@ -13,8 +13,16 @@ import { join } from "node:path"; import { afterEach, describe, expect, test } from "vitest"; -import { createWorktree } from "../../../utils/worktree.js"; +import pino from "pino"; +import { archivePersistedWorkspaceRecord } from "../../workspace-archive-service.js"; +import { getCheckoutStatus } from "../../../utils/checkout-git.js"; import { + readPaseoWorktreeMetadata, + writePaseoWorktreeMetadata, +} from "../../../utils/worktree-metadata.js"; +import { createWorktree, deletePaseoWorktree } from "../../../utils/worktree.js"; +import { + FileBackedWorkspaceRegistry, createPersistedProjectRecord, createPersistedWorkspaceRecord, type PersistedProjectRecord, @@ -286,3 +294,125 @@ function createGitRepository(): { tempDir: string; repoDir: string } { execFileSync("git", ["commit", "-m", "initial"], { cwd: repoDir, stdio: "pipe" }); return { tempDir, repoDir }; } + +async function createBaseRecoveryFixture(baseBranch: string | null) { + const { tempDir, repoDir } = createGitRepository(); + const paseoHome = join(tempDir, "paseo-home"); + const created = await createWorktree({ + cwd: repoDir, + paseoHome, + worktreeSlug: "base-recovery", + source: { kind: "branch-off", branchName: "feature", baseBranch: "main" }, + runSetup: false, + }); + writeFileSync(join(created.worktreePath, "feature.txt"), "feature\n"); + execFileSync("git", ["add", "."], { cwd: created.worktreePath }); + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "feature"], { + cwd: created.worktreePath, + stdio: "pipe", + }); + const workspace = createWorkspace({ + workspaceId: "base-recovery", + cwd: created.worktreePath, + worktreeRoot: created.worktreePath, + mainRepoRoot: repoDir, + branch: "feature", + baseBranch, + archivedAt: null, + }); + const project = createProject({ rootPath: repoDir }); + const registry = new FileBackedWorkspaceRegistry( + join(tempDir, "workspaces.json"), + pino({ level: "silent" }), + ); + await registry.upsert(workspace); + const service = createWorkspaceRecoveryService({ + paseoHome, + getWorkspace: (id) => registry.get(id), + getProject: async () => project, + isDirectory: async (target) => existsSync(target) && statSync(target).isDirectory(), + unarchiveWorkspace: async (record) => { + await registry.update(record.workspaceId, (value) => ({ ...value, archivedAt: null })); + }, + }); + async function archiveAndRemove() { + await archivePersistedWorkspaceRecord({ + workspaceId: workspace.workspaceId, + workspaceRegistry: registry, + }); + await deletePaseoWorktree({ cwd: repoDir, worktreePath: workspace.cwd, paseoHome }); + expect(existsSync(workspace.cwd)).toBe(false); + } + return { workspace, registry, service, paseoHome, repoDir, tempDir, archiveAndRemove }; +} + +test("preserves ordinary checkout behavior when no separate base was recorded", async () => { + const fixture = await createBaseRecoveryFixture(null); + writePaseoWorktreeMetadata(fixture.workspace.cwd, { baseRefName: "feature" }); + await fixture.archiveAndRemove(); + await fixture.service.restore(fixture.workspace.workspaceId); + expect((await fixture.registry.get(fixture.workspace.workspaceId))?.baseBranch).toBeNull(); + expect( + await getCheckoutStatus(fixture.workspace.cwd, { paseoHome: fixture.paseoHome }), + ).toMatchObject({ currentBranch: "feature", baseRef: "feature" }); +}); + +test("restores the branch HEAD with a base name when the exact base is missing", async () => { + const fixture = await createBaseRecoveryFixture("refs/remotes/upstream/main"); + const headBeforeArchive = execFileSync("git", ["rev-parse", "HEAD"], { + cwd: fixture.workspace.cwd, + encoding: "utf8", + }).trim(); + await fixture.archiveAndRemove(); + await expect(fixture.service.restore(fixture.workspace.workspaceId)).resolves.toEqual({ + workspaceId: fixture.workspace.workspaceId, + action: "restore", + }); + expect(existsSync(fixture.workspace.cwd)).toBe(true); + expect((await fixture.registry.get(fixture.workspace.workspaceId))?.archivedAt).toBeNull(); + const metadata = readPaseoWorktreeMetadata(fixture.workspace.cwd); + expect(metadata).toMatchObject({ baseRefName: "main" }); + expect(metadata?.baseRef).toBeUndefined(); + expect( + execFileSync("git", ["rev-parse", "HEAD"], { + cwd: fixture.workspace.cwd, + encoding: "utf8", + }).trim(), + ).toBe(headBeforeArchive); +}); + +test("restore fetches a deleted local branch from origin and retains its base", async () => { + const fixture = await createBaseRecoveryFixture("refs/heads/main"); + const remote = join(fixture.tempDir, "remote.git"); + execFileSync("git", ["init", "--bare", remote], { stdio: "pipe" }); + execFileSync("git", ["remote", "add", "origin", remote], { cwd: fixture.repoDir }); + execFileSync("git", ["push", "origin", "feature"], { cwd: fixture.repoDir, stdio: "pipe" }); + await fixture.archiveAndRemove(); + execFileSync("git", ["branch", "-D", "feature"], { cwd: fixture.repoDir }); + await fixture.service.restore(fixture.workspace.workspaceId); + expect(readPaseoWorktreeMetadata(fixture.workspace.cwd)?.baseRef).toBe("refs/heads/main"); + expect( + await getCheckoutStatus(fixture.workspace.cwd, { paseoHome: fixture.paseoHome }), + ).toMatchObject({ + currentBranch: "feature", + baseRef: "main", + aheadBehind: { ahead: 1, behind: 0 }, + }); +}); + +test("restore rejects a branch checked out elsewhere without inventing another branch", async () => { + const fixture = await createBaseRecoveryFixture("refs/heads/main"); + await fixture.archiveAndRemove(); + execFileSync("git", ["switch", "feature"], { cwd: fixture.repoDir, stdio: "pipe" }); + await expect(fixture.service.restore(fixture.workspace.workspaceId)).rejects.toThrow( + /already checked out/, + ); + expect(existsSync(fixture.workspace.cwd)).toBe(false); + expect((await fixture.registry.get(fixture.workspace.workspaceId))?.archivedAt).not.toBeNull(); + expect( + execFileSync("git", ["branch", "--list", "feature*"], { + cwd: fixture.repoDir, + encoding: "utf8", + }).trim(), + ).toBe("* feature"); +}); diff --git a/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.ts b/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.ts index 1d98a9128c..f40e0e8afc 100644 --- a/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.ts +++ b/packages/server/src/server/session/workspace-recovery/workspace-recovery-service.ts @@ -189,7 +189,7 @@ export function createWorkspaceRecoveryService(deps: { const result = await createWorktree({ cwd: sourceRepoRoot, worktreeSlug: basename(previousWorktreePath), - source: { kind: "checkout-branch", branchName: branch }, + source: { kind: "restore", branchName: branch, baseRef: workspace.baseBranch }, runSetup: false, paseoHome: deps.paseoHome, worktreesRoot: deps.worktreesRoot, diff --git a/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts b/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts index a63d2e4a82..2e1f2c4e49 100644 --- a/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts +++ b/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts @@ -1,8 +1,8 @@ import { execFileSync } from "node:child_process"; -import { mkdtempSync, rmSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; -import { expect, test } from "vitest"; +import { expect, onTestFinished, test } from "vitest"; import { DaemonClient } from "./test-utils/index.js"; import { createTestPaseoDaemon } from "./test-utils/paseo-daemon.js"; @@ -258,3 +258,173 @@ test("workspace.create suffixes an occupied checkout branch", async () => { rmSync(tempRoot, { recursive: true, force: true }); } }, 180000); + +function runGit(cwd: string, ...args: string[]): string { + return execFileSync("git", args, { cwd, encoding: "utf8", stdio: "pipe" }).trim(); +} +function commitFile(cwd: string, name: string, content: string): void { + writeFileSync(path.join(cwd, name), content); + runGit(cwd, "add", name); + runGit(cwd, "-c", "commit.gpgsign=false", "commit", "-m", name); +} + +test.each([ + ["refs/heads/main", "refs/remotes/origin/main"], + ["refs/remotes/origin/main", "refs/heads/main"], +])( + "restore preserves %s through comparison, update and local merge", + async (baseRef, conflictingRef) => { + const workspace = await createRestorableWorkspace(baseRef); + await archiveAndRestoreWorkspace(workspace); + await expectRestoredComparison(workspace, conflictingRef); + await updateFromOriginalBase(workspace); + await expectMergeIntoLocalBase(workspace); + }, + 180000, +); + +test("restore preserves upstream comparison and update but rejects a missing local merge target", async () => { + const workspace = await createRestorableWorkspace("refs/remotes/upstream/main"); + await archiveAndRestoreWorkspace(workspace); + await expectRestoredComparison(workspace, "refs/heads/main"); + await updateFromOriginalBase(workspace); + await expectMissingLocalMergeTarget(workspace); +}, 180000); + +async function createRestorableWorkspace(baseRef: string) { + const daemon = await createTestPaseoDaemon(); + const { repoDir, tempRoot } = createGitRepoWithBranch(); + const client = new DaemonClient({ url: `ws://127.0.0.1:${daemon.port}/ws`, appVersion: "0.8.0" }); + onTestFinished(async () => { + await client.close().catch(() => undefined); + await daemon.close(); + rmSync(tempRoot, { recursive: true, force: true }); + }); + createDivergentBases(repoDir); + await client.connect(); + const created = await client.createWorkspace({ + source: { + kind: "worktree", + cwd: repoDir, + action: "branch-off", + baseBranch: baseRef, + branchName: "feature/restore", + worktreeSlug: "restore-base", + }, + }); + expect(created.error).toBeNull(); + const workspace = created.workspace!; + const cwd = workspace.workspaceDirectory; + commitFile(cwd, "feature.txt", "feature change\n"); + const head = runGit(cwd, "rev-parse", "HEAD"); + await client.checkoutRefresh(cwd); + const before = await client.getCheckoutDiff(cwd, { mode: "base", baseRef: "main" }); + expect(before.error).toBeNull(); + expect(before.files.map((file) => file.path)).toEqual(["feature.txt"]); + expect((await client.getCheckoutStatus(cwd)).baseRef).toBe("main"); + return { client, daemon, repoDir, workspace, cwd, head, before, baseRef }; +} + +type RestorableWorkspace = Awaited>; + +function createDivergentBases(repoDir: string): void { + const root = runGit(repoDir, "rev-parse", "HEAD"); + // Same displayed name, three different commit streams. + commitFile(repoDir, "local.txt", "local base\n"); + runGit(repoDir, "switch", "--detach", root); + commitFile(repoDir, "origin.txt", "origin base\n"); + runGit(repoDir, "update-ref", "refs/remotes/origin/main", "HEAD"); + runGit(repoDir, "switch", "--detach", root); + commitFile(repoDir, "upstream.txt", "upstream base\n"); + runGit(repoDir, "update-ref", "refs/remotes/upstream/main", "HEAD"); + runGit(repoDir, "switch", "main"); +} + +async function archiveAndRestoreWorkspace({ client, workspace, cwd }: RestorableWorkspace) { + expect((await client.archiveWorkspace(workspace.id)).error).toBeNull(); + expect(existsSync(cwd)).toBe(false); + expect(await client.inspectWorkspaceRecovery(workspace.id)).toMatchObject({ action: "restore" }); + await client.restoreWorkspace(workspace.id); +} + +async function expectRestoredComparison( + { client, daemon, workspace, cwd, head, before, baseRef }: RestorableWorkspace, + conflictingRef: string, +) { + const restoredStatus = await client.getCheckoutStatus(cwd); + await client.checkoutRefresh(cwd); + const after = await client.getCheckoutDiff(cwd, { + mode: "base", + baseRef: restoredStatus.baseRef ?? undefined, + }); + expect(after.error).toBeNull(); + expect(after.files).toEqual(before.files); + expect(runGit(cwd, "rev-parse", "HEAD")).toBe(head); + expect(runGit(cwd, "branch", "--show-current")).toBe("feature/restore"); + expect(await client.getCheckoutStatus(cwd)).toMatchObject({ + baseRef: "main", + aheadBehind: { ahead: 1, behind: 0 }, + }); + const history = await client.listCheckoutCommits(cwd); + expect(history.baseRef).toBe(baseRef); + expect(history.commits.filter((commit) => !commit.isOnBase).map((commit) => commit.sha)).toEqual([ + head, + ]); + expect((await client.getCommitFileDiff(cwd, head, "feature.txt")).file?.path).toBe("feature.txt"); + const records = JSON.parse( + readFileSync(path.join(daemon.paseoHome, "projects/workspaces.json"), "utf8"), + ); + expect( + records.find((record: { workspaceId: string }) => record.workspaceId === workspace.id) + .baseBranch, + ).toBe(baseRef); + expect( + (await client.getCheckoutDiff(cwd, { mode: "base", baseRef: conflictingRef })).error?.message, + ).toContain("Base ref mismatch"); +} + +async function updateFromOriginalBase({ client, repoDir, cwd, baseRef }: RestorableWorkspace) { + // Advance only the chosen base; Update must not merge a same-named alternative. + runGit(repoDir, "switch", "--detach", baseRef); + commitFile(repoDir, "base-update.txt", `${baseRef}\n`); + const baseUpdate = runGit(repoDir, "rev-parse", "HEAD"); + runGit(repoDir, "update-ref", baseRef, baseUpdate); + runGit(repoDir, "switch", "main"); + await client.checkoutRefresh(cwd); + expect(await client.getCheckoutStatus(cwd)).toMatchObject({ + aheadBehind: { ahead: 1, behind: 1 }, + }); + expect((await client.checkoutMergeFromBase(cwd, { baseRef: "main" })).error).toBeNull(); + expect(readFileSync(path.join(cwd, "base-update.txt"), "utf8")).toBe(`${baseRef}\n`); + expect(runGit(cwd, "merge-base", baseUpdate, "HEAD")).toBe(baseUpdate); + expect( + (await client.getCheckoutDiff(cwd, { mode: "base", baseRef: "main" })).files.map( + (file) => file.path, + ), + ).toEqual(["feature.txt"]); +} + +async function expectMergeIntoLocalBase({ client, cwd, repoDir }: RestorableWorkspace) { + expect( + ( + await client.checkoutMerge(cwd, { + baseRef: "main", + strategy: "merge", + requireCleanTarget: true, + }) + ).error, + ).toBeNull(); + expect(runGit(repoDir, "show", "main:feature.txt")).toBe("feature change"); +} + +async function expectMissingLocalMergeTarget({ client, cwd }: RestorableWorkspace) { + expect( + ( + await client.checkoutMerge(cwd, { + baseRef: "main", + strategy: "merge", + requireCleanTarget: true, + }) + ).error?.message, + ).toContain("No local merge target is recorded"); +} diff --git a/packages/server/src/server/workspace-registry.ts b/packages/server/src/server/workspace-registry.ts index b4b8d5cdd5..64f6f1e4a1 100644 --- a/packages/server/src/server/workspace-registry.ts +++ b/packages/server/src/server/workspace-registry.ts @@ -75,9 +75,9 @@ const PersistedWorkspaceRecordSchema = z.object({ // archive and recovery do not need the directory to still exist in order to // recover placement. worktreeRoot: z.string().nullable().default(null), - // The base branch the worktree was created from (normalized like worktree.json's - // baseRefName). Only worktree workspaces carry a base branch; checkout-branch - // worktrees and directory/local_checkout workspaces leave it null. + // Comparison base preserved across worktree deletion and restore. New branch-off + // workspaces store the resolved ref (e.g. refs/remotes/upstream/main); legacy and + // PR checkout records may contain a bare branch name. Ordinary checkouts use null. baseBranch: z .string() .nullable() diff --git a/packages/server/src/server/worktree-core.ts b/packages/server/src/server/worktree-core.ts index f4b51e0554..c66e91c5dd 100644 --- a/packages/server/src/server/worktree-core.ts +++ b/packages/server/src/server/worktree-core.ts @@ -5,7 +5,7 @@ import { createWorktree, slugify, validateBranchSlug, - type WorktreeConfig, + type CreatedWorktree, } from "../utils/worktree.js"; import { resolveWorktreeCreationIntent, @@ -41,7 +41,7 @@ export interface CreateWorktreeCoreDeps { } export interface CreateWorktreeCoreResult { - worktree: WorktreeConfig; + worktree: CreatedWorktree; intent: WorktreeCreationIntent; repoRoot: string; created: boolean; diff --git a/packages/server/src/server/worktree-session.test.ts b/packages/server/src/server/worktree-session.test.ts index 384a74a54d..0fb32bf558 100644 --- a/packages/server/src/server/worktree-session.test.ts +++ b/packages/server/src/server/worktree-session.test.ts @@ -1904,6 +1904,7 @@ describe("handleCreatePaseoWorktreeRequest", () => { requestCwd: repoDir, worktree: { branchName: "response-after-create", + comparisonBaseRef: "refs/heads/main", worktreePath: registeredWorktreePath, }, shouldBootstrap: true, diff --git a/packages/server/src/utils/worktree-metadata.ts b/packages/server/src/utils/worktree-metadata.ts index e07691ba1c..b977f7952b 100644 --- a/packages/server/src/utils/worktree-metadata.ts +++ b/packages/server/src/utils/worktree-metadata.ts @@ -162,6 +162,10 @@ export function getPaseoWorktreeMetadataPath(worktreeRoot: string): string { return join(gitDir, "paseo", "worktree.json"); } +export function isQualifiedRef(ref: string): boolean { + return ref.trim().startsWith("refs/"); +} + const REMOTE_TRACKING_PREFIX = "refs/remotes/"; /** diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index 987867c0d3..ad5ba0f8b3 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -31,6 +31,7 @@ import { PaseoConfigSchema, type PaseoConfig } from "@getpaseo/protocol/paseo-co import { createPaseoWorktreeChangeRequestHint, normalizeBaseRefName, + isQualifiedRef, type PaseoWorktreeChangeRequestHint, readPaseoWorktreeMetadata, readPaseoWorktreeRuntimePort, @@ -57,6 +58,10 @@ export interface WorktreeConfig { worktreePath: string; } +export interface CreatedWorktree extends WorktreeConfig { + comparisonBaseRef: string | null; +} + export interface WorktreeRuntimeEnv { [key: string]: string; PASEO_SOURCE_CHECKOUT_PATH: string; @@ -180,6 +185,7 @@ export interface WorktreeCheckoutRef { export type WorktreeSource = | { kind: "branch-off"; baseBranch: string; branchName: string } | { kind: "checkout-branch"; branchName: string } + | { kind: "restore"; branchName: string; baseRef: string | null } | { kind: "checkout-change-request"; forge: string; @@ -1216,7 +1222,7 @@ export const createWorktree = async ({ runSetup, paseoHome, worktreesRoot, -}: CreateWorktreeOptions): Promise => { +}: CreateWorktreeOptions): Promise => { const sourcePlan = await resolveWorktreeSourcePlan({ cwd, source, desiredSlug: worktreeSlug }); let worktreePath = join(await getPaseoWorktreesRoot(cwd, paseoHome, worktreesRoot), worktreeSlug); mkdirSync(dirname(worktreePath), { recursive: true }); @@ -1272,6 +1278,10 @@ export const createWorktree = async ({ return { branchName: sourcePlan.branchName, worktreePath, + comparisonBaseRef: + source.kind === "checkout-branch" + ? null + : (sourcePlan.metadataBaseRef ?? sourcePlan.metadataBaseRefName), }; }; @@ -1302,45 +1312,74 @@ interface WorktreeSourcePlan { }; } +async function resolveRestoredWorktreeSourcePlan( + cwd: string, + source: Extract, +): Promise { + await validateGitBranchName(cwd, source.branchName); + await ensureLocalBranch(cwd, source.branchName); + if (await isBranchCheckedOut(cwd, source.branchName)) { + throw new BranchAlreadyCheckedOutError(source.branchName); + } + let exactBase: string | undefined; + if (source.baseRef && isQualifiedRef(source.baseRef)) { + try { + exactBase = await resolveBaseBranchForWorktree(cwd, source.baseRef); + } catch { + // Restore the saved branch even when its comparison ref no longer exists. + } + } + return { + branchName: source.branchName, + metadataBaseRefName: normalizeRequiredBaseBranch(source.baseRef ?? source.branchName), + ...(exactBase ? { metadataBaseRef: exactBase } : {}), + changeRequestLookupTarget: createPaseoWorktreeChangeRequestHint({ + headRef: source.branchName, + localBranchName: source.branchName, + }), + addArguments: [source.branchName], + }; +} + +async function resolveBranchOffWorktreeSourcePlan( + cwd: string, + source: Extract, + desiredSlug: string, +): Promise { + const branchName = source.branchName; + await validateGitBranchName(cwd, branchName); + const normalizedBaseBranch = normalizeRequiredBaseBranch(source.baseBranch); + const resolvedBaseBranch = await resolveBaseBranchForWorktree(cwd, source.baseBranch); + const branchExists = await localBranchExists(cwd, branchName); + const base = branchExists ? branchName : resolvedBaseBranch; + const candidateBranch = branchExists ? desiredSlug : branchName; + const newBranchName = await resolveUniqueLocalBranchName(cwd, candidateBranch); + + return { + branchName: newBranchName, + metadataBaseRefName: normalizedBaseBranch, + metadataBaseRef: resolvedBaseBranch, + changeRequestLookupTarget: createPaseoWorktreeChangeRequestHint({ + headRef: newBranchName, + localBranchName: newBranchName, + }), + addArguments: ["-b", newBranchName, "--no-track", base], + }; +} + async function resolveWorktreeSourcePlan({ cwd, source, desiredSlug, }: ResolveWorktreeSourcePlanOptions): Promise { switch (source.kind) { - case "branch-off": { - const branchName = source.branchName; - await validateGitBranchName(cwd, branchName); - const normalizedBaseBranch = normalizeRequiredBaseBranch(source.baseBranch); - const resolvedBaseBranch = await resolveBaseBranchForWorktree(cwd, source.baseBranch); - const branchExists = await localBranchExists(cwd, branchName); - const base = branchExists ? branchName : resolvedBaseBranch; - const candidateBranch = branchExists ? desiredSlug : branchName; - const newBranchName = await resolveUniqueLocalBranchName(cwd, candidateBranch); - - return { - branchName: newBranchName, - metadataBaseRefName: normalizedBaseBranch, - metadataBaseRef: resolvedBaseBranch, - changeRequestLookupTarget: createPaseoWorktreeChangeRequestHint({ - headRef: newBranchName, - localBranchName: newBranchName, - }), - addArguments: ["-b", newBranchName, "--no-track", base], - }; - } + case "branch-off": + return resolveBranchOffWorktreeSourcePlan(cwd, source, desiredSlug); + case "restore": + return resolveRestoredWorktreeSourcePlan(cwd, source); case "checkout-branch": { await validateGitBranchName(cwd, source.branchName); - if (!(await localBranchExists(cwd, source.branchName))) { - try { - await runGitCommand(["fetch", "origin", `${source.branchName}:${source.branchName}`], { - cwd, - timeout: 120_000, - }); - } catch { - throw new UnknownBranchError({ branchName: source.branchName, cwd }); - } - } + await ensureLocalBranch(cwd, source.branchName); if (await isBranchCheckedOut(cwd, source.branchName)) { const branchName = await resolveUniqueLocalBranchName(cwd, source.branchName); return { @@ -1618,7 +1657,7 @@ async function resolveBaseBranchForWorktree( const requested = requestedBaseBranch.trim(); const normalized = normalizeRequiredBaseBranch(requested); let exactRef: string | null = null; - if (requested.startsWith("refs/")) { + if (isQualifiedRef(requested)) { exactRef = requested; } else if (requested.startsWith("origin/")) { exactRef = `refs/remotes/${requested}`; @@ -1629,7 +1668,7 @@ async function resolveBaseBranchForWorktree( await runGitCommand(["rev-parse", "--verify", exactRef], { cwd }); return exactRef; } catch { - throw new Error(`Base branch not found: ${normalized}`); + throw new Error(`Base branch not found: ${exactRef}`); } } @@ -1645,6 +1684,20 @@ async function resolveBaseBranchForWorktree( throw new Error(`Base branch not found: ${normalized}`); } +async function ensureLocalBranch(cwd: string, branchName: string): Promise { + if (await localBranchExists(cwd, branchName)) { + return; + } + try { + await runGitCommand(["fetch", "origin", `${branchName}:${branchName}`], { + cwd, + timeout: 120_000, + }); + } catch { + throw new UnknownBranchError({ branchName, cwd }); + } +} + async function localBranchExists(cwd: string, branchName: string): Promise { try { await runGitCommand(["show-ref", "--verify", "--quiet", `refs/heads/${branchName}`], {