diff --git a/packages/app/e2e/browser/compact-git-actions-menu.spec.ts b/packages/app/e2e/browser/compact-git-actions-menu.spec.ts new file mode 100644 index 0000000000..8198ecea5f --- /dev/null +++ b/packages/app/e2e/browser/compact-git-actions-menu.spec.ts @@ -0,0 +1,115 @@ +import { execFileSync } from "node:child_process"; +import { mkdtemp, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { test, expect, type Page } from "../support/fixtures"; +import { getServerId } from "../support/helpers/server-id"; +import { connectSeedClient } from "../support/helpers/seed-client"; +import { createTempGitRepo, resolveTempRoot } from "../support/helpers/workspace"; +import { buildHostWorkspaceRoute } from "../../src/utils/host-routes"; + +// The compact toolbar flattens every git action into one menu, and the policy keeps the actions +// it can promote to `primary` in `secondary` as well. A promoted action must still show once. +const cleanupTasks: Array<() => Promise> = []; + +test.afterEach(async () => { + for (const task of cleanupTasks.splice(0)) { + await task(); + } +}); + +async function createRemoteRepo(prefix: string): Promise { + const repo = await createTempGitRepo(prefix, { + withRemote: true, + files: [{ path: "src/app.ts", content: "export const app = 1;\n" }], + }); + cleanupTasks.push(repo.cleanup); + // The helper puts the bare remote inside the checkout; keep it out of the status. + await writeFile(path.join(repo.path, ".git/info/exclude"), "remote.git/\n"); + return repo.path; +} + +async function registerWorkspace(repoPath: string): Promise { + const client = await connectSeedClient(); + cleanupTasks.push(async () => { + await client.close().catch(() => undefined); + }); + const created = await client.createWorkspace({ source: { kind: "directory", path: repoPath } }); + if (!created.workspace) { + throw new Error(created.error ?? `Failed to create workspace ${repoPath}`); + } + return created.workspace.id; +} + +async function openCompactActionsMenu(page: Page, workspaceId: string) { + await page.setViewportSize({ width: 390, height: 844 }); + await page.goto(buildHostWorkspaceRoute(getServerId(), workspaceId)); + await page.reload(); + await page.getByTestId("workspace-explorer-toggle").first().click(); + const changesTab = page.getByTestId("explorer-tab-changes").filter({ visible: true }); + await expect(changesTab).toBeVisible({ timeout: 30_000 }); + await changesTab.click(); + const explorer = page.getByTestId("explorer-content-area").filter({ visible: true }); + await expect(explorer.getByTestId("changes-header")).toBeVisible({ timeout: 30_000 }); + + const trigger = explorer.getByTestId("changes-actions-menu-trigger"); + await expect(trigger).toBeVisible({ timeout: 30_000 }); + await trigger.click(); + const menu = page.getByTestId("changes-primary-cta-menu"); + await expect(menu).toBeVisible(); + return menu; +} + +test("compact git actions menu lists a promoted Pull once", async ({ page }) => { + const repoPath = await createRemoteRepo("compact-menu-pull-"); + cleanupTasks.push(async () => { + execFileSync("rm", ["-rf", repoPath]); + }); + + // Advance origin so the clean checkout is behind it and Pull is the promoted action. + const other = await mkdtemp(path.join(await resolveTempRoot(), "compact-menu-other-")); + cleanupTasks.push(async () => { + execFileSync("rm", ["-rf", other]); + }); + execFileSync("git", ["clone", path.join(repoPath, "remote.git"), other], { stdio: "ignore" }); + execFileSync("git", ["config", "user.email", "e2e@paseo.test"], { cwd: other, stdio: "ignore" }); + execFileSync("git", ["config", "user.name", "Paseo E2E"], { cwd: other, stdio: "ignore" }); + await writeFile(path.join(other, "src/incoming.ts"), "export const incoming = 1;\n"); + execFileSync("git", ["add", "--all"], { cwd: other, stdio: "ignore" }); + execFileSync("git", ["commit", "-m", "Incoming commit"], { cwd: other, stdio: "ignore" }); + execFileSync("git", ["push", "origin", "main"], { cwd: other, stdio: "ignore" }); + execFileSync("git", ["fetch", "origin"], { cwd: repoPath, stdio: "ignore" }); + + const workspaceId = await registerWorkspace(repoPath); + const menu = await openCompactActionsMenu(page, workspaceId); + + await expect(menu.getByTestId("changes-menu-pull")).toHaveCount(1); + await expect(menu.getByRole("menuitem")).toHaveText([ + "Pull", + "Push", + "Pull and push", + "Archive workspace", + ]); +}); + +test("compact git actions menu lists a promoted Push once", async ({ page }) => { + const repoPath = await createRemoteRepo("compact-menu-push-"); + cleanupTasks.push(async () => { + execFileSync("rm", ["-rf", repoPath]); + }); + const workspaceId = await registerWorkspace(repoPath); + + // Commit so the clean checkout is ahead of origin and Push is the promoted action. + await writeFile(path.join(repoPath, "src/mine.ts"), "export const mine = 1;\n"); + execFileSync("git", ["add", "--all"], { cwd: repoPath, stdio: "ignore" }); + execFileSync("git", ["commit", "-m", "Local commit"], { cwd: repoPath, stdio: "ignore" }); + + const menu = await openCompactActionsMenu(page, workspaceId); + + await expect(menu.getByTestId("changes-menu-push")).toHaveCount(1); + await expect(menu.getByRole("menuitem")).toHaveText([ + "Push", + "Pull", + "Pull and push", + "Archive workspace", + ]); +}); diff --git a/packages/app/src/command-center/workspace-contributions.ts b/packages/app/src/command-center/workspace-contributions.ts index afc484de57..538905cb4a 100644 --- a/packages/app/src/command-center/workspace-contributions.ts +++ b/packages/app/src/command-center/workspace-contributions.ts @@ -1,4 +1,4 @@ -import type { GitAction, GitActions } from "@/git/policy"; +import { actionsWithoutPrimary, type GitAction, type GitActions } from "@/git/policy"; import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher"; import type { WorkspacePanelPlacement, @@ -581,9 +581,7 @@ export function buildWorkspaceCommandCenterContributions( ...(source.capabilities.canSplitPanes ? buildPaneContributions(source) : []), ]; - const primary = source.gitActions.primary; - for (const [index, action] of source.gitActions.secondary.entries()) { - if (action.id === primary?.id) continue; + for (const [index, action] of actionsWithoutPrimary(source.gitActions).entries()) { contributions.push(buildGitContribution(source, action, 100 + index, "query")); } diff --git a/packages/app/src/git/actions-split-button.tsx b/packages/app/src/git/actions-split-button.tsx index 193154b377..64a550f144 100644 --- a/packages/app/src/git/actions-split-button.tsx +++ b/packages/app/src/git/actions-split-button.tsx @@ -15,7 +15,7 @@ import { Shortcut } from "@/components/ui/shortcut"; import { useShortcutKeys } from "@/hooks/use-shortcut-keys"; import { inlineUnistylesStyle } from "@/styles/unistyles-inline-style"; import type { ShortcutKey } from "@/utils/format-shortcut"; -import type { GitAction, GitActions } from "@/git/policy"; +import { actionsWithoutPrimary, type GitAction, type GitActions } from "@/git/policy"; import { useGitActionRunner } from "@/git/use-actions"; import { buttonControlHeight, HEADER_CONTROL_HEIGHT } from "@/components/ui/control-geometry"; @@ -129,14 +129,10 @@ export function GitActionsSplitButton({ [theme.colors.surface2], ); - const menuOnlyActions = useMemo( - () => [ - ...(gitActions.primary ? [gitActions.primary] : []), - ...gitActions.secondary, - ...gitActions.menu, - ], - [gitActions.menu, gitActions.primary, gitActions.secondary], - ); + const menuOnlyActions = [ + ...(gitActions.primary ? [gitActions.primary] : []), + ...actionsWithoutPrimary(gitActions), + ]; if (menuOnly) { if (menuOnlyActions.length === 0) { diff --git a/packages/app/src/git/policy.test.ts b/packages/app/src/git/policy.test.ts index 561bbc75e6..efbd61efa6 100644 --- a/packages/app/src/git/policy.test.ts +++ b/packages/app/src/git/policy.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { CheckoutPrStatusSchema } from "@getpaseo/protocol/messages"; import { i18n } from "@/i18n/i18next"; -import { buildGitActions, type BuildGitActionsInput } from "./policy"; +import { actionsWithoutPrimary, buildGitActions, type BuildGitActionsInput } from "./policy"; import { deriveMergeCapability, type ForgeSpecificStatusFacts } from "./merge-capability"; type GithubMergeFactsFixture = ForgeSpecificStatusFacts & { @@ -190,6 +190,59 @@ describe("git-actions-policy", () => { expect(actions.primary).toMatchObject({ id: "pull", label: "Pull" }); }); + it("drops a promoted remote action from the remaining menu actions", () => { + const pull = buildGitActions(createInput({ hasRemote: true, behindOfOrigin: 2 })); + expect(pull.primary?.id).toBe("pull"); + expect(actionsWithoutPrimary(pull).map((action) => action.id)).toEqual([ + "push", + "pull-and-push", + "archive-workspace", + ]); + + const push = buildGitActions(createInput({ hasRemote: true, aheadOfOrigin: 1 })); + expect(push.primary?.id).toBe("push"); + expect(actionsWithoutPrimary(push).map((action) => action.id)).toEqual([ + "pull", + "pull-and-push", + "archive-workspace", + ]); + }); + + it("drops a promoted merge action from the remaining menu actions", () => { + const actions = buildGitActions( + createInput({ + hasRemote: true, + isOnBaseBranch: false, + aheadCount: 2, + hasPullRequest: true, + pullRequestUrl: "https://example.com/pr/456", + pullRequestState: "open", + pullRequestMergeable: "MERGEABLE", + pullRequestGithub: githubStatus({ + repository: { + autoMergeAllowed: true, + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: false, + viewerDefaultMergeMethod: "SQUASH", + }, + }), + shipDefault: "pr", + }), + ); + + expect(actions.primary).toMatchObject({ id: "merge-pr-merge", label: "Merge PR (merge)" }); + expect(actionsWithoutPrimary(actions).map((action) => action.id)).toEqual([ + "pull", + "push", + "pull-and-push", + "merge-from-base", + "merge-branch", + "pr", + "archive-workspace", + ]); + }); + it("keeps push clickable with a clearer message when the branch diverged", () => { const actions = buildGitActions( createInput({ diff --git a/packages/app/src/git/policy.ts b/packages/app/src/git/policy.ts index eb6e28399b..71d63eae37 100644 --- a/packages/app/src/git/policy.ts +++ b/packages/app/src/git/policy.ts @@ -312,6 +312,18 @@ export function buildGitActions(input: BuildGitActionsInput): GitActions { }; } +/** + * The actions a flattened list shows after the primary. `secondary` keeps the actions the policy + * can promote to `primary`, and the translator rebuilds those objects, so a flattening consumer + * has to drop the promoted one by id or it renders it twice. + */ +export function actionsWithoutPrimary(gitActions: GitActions): GitAction[] { + const primaryActionId = gitActions.primary?.id; + return [...gitActions.secondary, ...gitActions.menu].filter( + (action) => action.id !== primaryActionId, + ); +} + function getPrimaryActionId(input: BuildGitActionsInput): GitActionId | null { if (input.shouldPromoteArchive) { return "archive-workspace";