From cc675da49cccd8b36cae34023c3747f7459c30ff Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 15 Mar 2026 13:51:04 +0700 Subject: [PATCH] Stabilize git action menu --- .../components/git-action-visibility.test.ts | 62 ----- .../src/components/git-action-visibility.ts | 14 - .../src/components/git-actions-policy.test.ts | 149 ++++++++++ .../app/src/components/git-actions-policy.ts | 256 +++++++++++++++++ .../components/git-actions-split-button.tsx | 8 +- packages/app/src/components/git-diff-pane.tsx | 261 +++++------------ packages/app/src/hooks/use-git-actions.ts | 263 +++++------------- 7 files changed, 549 insertions(+), 464 deletions(-) delete mode 100644 packages/app/src/components/git-action-visibility.test.ts delete mode 100644 packages/app/src/components/git-action-visibility.ts create mode 100644 packages/app/src/components/git-actions-policy.test.ts create mode 100644 packages/app/src/components/git-actions-policy.ts diff --git a/packages/app/src/components/git-action-visibility.test.ts b/packages/app/src/components/git-action-visibility.test.ts deleted file mode 100644 index aafab4c4c..000000000 --- a/packages/app/src/components/git-action-visibility.test.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { shouldShowMergeFromBaseAction } from "./git-action-visibility"; - -describe("git-action-visibility", () => { - describe("shouldShowMergeFromBaseAction", () => { - it("shows on non-base branches", () => { - expect( - shouldShowMergeFromBaseAction({ - isOnBaseBranch: false, - hasRemote: false, - aheadOfOrigin: 0, - behindOfOrigin: 0, - }) - ).toBe(true); - }); - - it("hides on base branch when no remote exists", () => { - expect( - shouldShowMergeFromBaseAction({ - isOnBaseBranch: true, - hasRemote: false, - aheadOfOrigin: 0, - behindOfOrigin: 0, - }) - ).toBe(false); - }); - - it("hides on base branch when local is in sync with origin", () => { - expect( - shouldShowMergeFromBaseAction({ - isOnBaseBranch: true, - hasRemote: true, - aheadOfOrigin: 0, - behindOfOrigin: 0, - }) - ).toBe(false); - }); - - it("shows on base branch when ahead of origin", () => { - expect( - shouldShowMergeFromBaseAction({ - isOnBaseBranch: true, - hasRemote: true, - aheadOfOrigin: 1, - behindOfOrigin: 0, - }) - ).toBe(true); - }); - - it("shows on base branch when behind origin", () => { - expect( - shouldShowMergeFromBaseAction({ - isOnBaseBranch: true, - hasRemote: true, - aheadOfOrigin: 0, - behindOfOrigin: 2, - }) - ).toBe(true); - }); - }); -}); diff --git a/packages/app/src/components/git-action-visibility.ts b/packages/app/src/components/git-action-visibility.ts deleted file mode 100644 index bdd27e873..000000000 --- a/packages/app/src/components/git-action-visibility.ts +++ /dev/null @@ -1,14 +0,0 @@ -export function shouldShowMergeFromBaseAction(input: { - isOnBaseBranch: boolean; - hasRemote: boolean; - aheadOfOrigin: number; - behindOfOrigin: number; -}): boolean { - if (!input.isOnBaseBranch) { - return true; - } - if (!input.hasRemote) { - return false; - } - return input.aheadOfOrigin > 0 || input.behindOfOrigin > 0; -} diff --git a/packages/app/src/components/git-actions-policy.test.ts b/packages/app/src/components/git-actions-policy.test.ts new file mode 100644 index 000000000..5864113ce --- /dev/null +++ b/packages/app/src/components/git-actions-policy.test.ts @@ -0,0 +1,149 @@ +import { describe, expect, it } from "vitest"; + +import { buildGitActions, type BuildGitActionsInput } from "./git-actions-policy"; + +function createInput( + overrides: Partial = {} +): BuildGitActionsInput { + return { + isGit: true, + githubFeaturesEnabled: true, + hasPullRequest: false, + pullRequestUrl: null, + hasRemote: false, + isPaseoOwnedWorktree: false, + isOnBaseBranch: true, + hasUncommittedChanges: false, + baseRefAvailable: true, + baseRefLabel: "main", + aheadCount: 0, + aheadOfOrigin: 0, + behindOfOrigin: 0, + shouldPromoteArchive: false, + shipDefault: "merge", + runtime: { + commit: { + disabled: false, + status: "idle", + handler: () => undefined, + }, + push: { + disabled: false, + status: "idle", + handler: () => undefined, + }, + pr: { + disabled: false, + status: "idle", + handler: () => undefined, + }, + "merge-branch": { + disabled: false, + status: "idle", + handler: () => undefined, + }, + "merge-from-base": { + disabled: false, + status: "idle", + handler: () => undefined, + }, + "archive-worktree": { + disabled: false, + status: "idle", + handler: () => undefined, + }, + }, + ...overrides, + }; +} + +describe("git-actions-policy", () => { + it("keeps the secondary menu order stable while the primary action changes", () => { + const noPrActions = buildGitActions(createInput()); + const withPrActions = buildGitActions( + createInput({ + hasRemote: true, + hasPullRequest: true, + pullRequestUrl: "https://example.com/pr/123", + aheadCount: 3, + aheadOfOrigin: 2, + shipDefault: "pr", + }) + ); + + expect(noPrActions.primary).toBeNull(); + expect(withPrActions.primary?.id).toBe("push"); + expect(noPrActions.secondary.map((action) => action.id)).toEqual([ + "merge-branch", + "pr", + "merge-from-base", + "push", + ]); + expect(withPrActions.secondary.map((action) => action.id)).toEqual([ + "merge-branch", + "pr", + "merge-from-base", + "push", + ]); + }); + + it("disables hidden-before actions with explanations instead", () => { + const actions = buildGitActions(createInput()); + const actionById = new Map(actions.secondary.map((action) => [action.id, action])); + + expect(actionById.get("push")).toMatchObject({ + disabled: true, + description: "No remote configured", + }); + expect(actionById.get("pr")).toMatchObject({ + label: "Create PR", + disabled: true, + description: "Branch has no commits ahead of main", + }); + expect(actionById.get("merge-branch")).toMatchObject({ + disabled: true, + description: "No commits to merge into main", + }); + expect(actionById.get("merge-from-base")).toMatchObject({ + disabled: true, + description: "No remote configured", + }); + expect(actionById.has("archive-worktree")).toBe(false); + }); + + it("keeps the current primary action visible in the menu", () => { + const actions = buildGitActions( + createInput({ + hasRemote: true, + hasPullRequest: true, + pullRequestUrl: "https://example.com/pr/456", + }) + ); + + expect(actions.primary?.id).toBe("pr"); + expect(actions.secondary.some((action) => action.id === "pr" && action.label === "View PR")).toBe(true); + }); + + it("disables sync on the base branch when already up to date", () => { + const actions = buildGitActions( + createInput({ + hasRemote: true, + }) + ); + const syncAction = actions.secondary.find((action) => action.id === "merge-from-base"); + + expect(syncAction).toMatchObject({ + label: "Sync", + disabled: true, + description: "Already up to date", + }); + }); + + it("only shows archive worktree for paseo worktrees", () => { + const hidden = buildGitActions(createInput()); + const shown = buildGitActions(createInput({ isPaseoOwnedWorktree: true })); + + expect(hidden.secondary.some((action) => action.id === "archive-worktree")).toBe(false); + expect(shown.secondary.some((action) => action.id === "archive-worktree")).toBe(true); + }); +}); diff --git a/packages/app/src/components/git-actions-policy.ts b/packages/app/src/components/git-actions-policy.ts new file mode 100644 index 000000000..f9ac556d8 --- /dev/null +++ b/packages/app/src/components/git-actions-policy.ts @@ -0,0 +1,256 @@ +import type { ReactElement } from "react"; + +import type { ActionStatus } from "@/components/ui/dropdown-menu"; + +export type GitActionId = + | "commit" + | "push" + | "pr" + | "merge-branch" + | "merge-from-base" + | "archive-worktree"; + +export interface GitAction { + id: GitActionId; + label: string; + pendingLabel: string; + successLabel: string; + disabled: boolean; + status: ActionStatus; + description?: string; + icon?: ReactElement; + handler: () => void; +} + +export interface GitActions { + primary: GitAction | null; + secondary: GitAction[]; + menu: GitAction[]; +} + +interface GitActionRuntimeState { + disabled: boolean; + status: ActionStatus; + icon?: ReactElement; + handler: () => void; +} + +export interface BuildGitActionsInput { + isGit: boolean; + githubFeaturesEnabled: boolean; + hasPullRequest: boolean; + pullRequestUrl: string | null; + hasRemote: boolean; + isPaseoOwnedWorktree: boolean; + isOnBaseBranch: boolean; + hasUncommittedChanges: boolean; + baseRefAvailable: boolean; + baseRefLabel: string; + aheadCount: number; + aheadOfOrigin: number; + behindOfOrigin: number; + shouldPromoteArchive: boolean; + shipDefault: "merge" | "pr"; + runtime: Record; +} + +const SECONDARY_ACTION_IDS: GitActionId[] = [ + "merge-branch", + "pr", + "merge-from-base", + "push", +]; + +export function buildGitActions(input: BuildGitActionsInput): GitActions { + if (!input.isGit) { + return { primary: null, secondary: [], menu: [] }; + } + + const allActions = new Map(); + + allActions.set("commit", { + id: "commit", + label: "Commit", + pendingLabel: "Committing...", + successLabel: "Committed", + disabled: input.runtime.commit.disabled, + status: input.runtime.commit.status, + icon: input.runtime.commit.icon, + handler: input.runtime.commit.handler, + }); + + allActions.set("push", { + id: "push", + label: "Push", + pendingLabel: "Pushing...", + successLabel: "Pushed", + disabled: input.runtime.push.disabled || !input.hasRemote, + status: input.runtime.push.status, + description: input.hasRemote ? undefined : "No remote configured", + icon: input.runtime.push.icon, + handler: input.runtime.push.handler, + }); + + allActions.set("pr", buildPrAction(input)); + + allActions.set("merge-branch", { + id: "merge-branch", + label: `Merge into ${input.baseRefLabel}`, + pendingLabel: "Merging...", + successLabel: "Merged", + disabled: + input.runtime["merge-branch"].disabled || + !input.baseRefAvailable || + input.hasUncommittedChanges || + input.aheadCount === 0, + status: input.runtime["merge-branch"].status, + description: getMergeBranchDescription(input), + icon: input.runtime["merge-branch"].icon, + handler: input.runtime["merge-branch"].handler, + }); + + allActions.set("merge-from-base", { + id: "merge-from-base", + label: input.isOnBaseBranch ? "Sync" : `Update from ${input.baseRefLabel}`, + pendingLabel: "Updating...", + successLabel: "Updated", + disabled: input.runtime["merge-from-base"].disabled || !canMergeFromBase(input), + status: input.runtime["merge-from-base"].status, + description: getMergeFromBaseDescription(input), + icon: input.runtime["merge-from-base"].icon, + handler: input.runtime["merge-from-base"].handler, + }); + + allActions.set("archive-worktree", { + id: "archive-worktree", + label: "Archive worktree", + pendingLabel: "Archiving...", + successLabel: "Archived", + disabled: input.runtime["archive-worktree"].disabled || !input.isPaseoOwnedWorktree, + status: input.runtime["archive-worktree"].status, + description: input.isPaseoOwnedWorktree ? undefined : "Only available for Paseo worktrees", + icon: input.runtime["archive-worktree"].icon, + handler: input.runtime["archive-worktree"].handler, + }); + + const primaryActionId = getPrimaryActionId(input); + const primary = primaryActionId ? allActions.get(primaryActionId) ?? null : null; + const secondary = SECONDARY_ACTION_IDS.map((id) => allActions.get(id)!); + if (input.isPaseoOwnedWorktree) { + secondary.push(allActions.get("archive-worktree")!); + } + + return { primary, secondary, menu: [] }; +} + +function getPrimaryActionId(input: BuildGitActionsInput): GitActionId | null { + if (input.shouldPromoteArchive && input.isPaseoOwnedWorktree) { + return "archive-worktree"; + } + if (input.hasUncommittedChanges) { + return "commit"; + } + if (input.aheadOfOrigin > 0 && input.hasRemote) { + return "push"; + } + if (input.githubFeaturesEnabled && input.hasPullRequest && input.pullRequestUrl) { + return "pr"; + } + if ( + input.isOnBaseBranch && + input.hasRemote && + (input.aheadOfOrigin > 0 || input.behindOfOrigin > 0) + ) { + return "merge-from-base"; + } + if (input.aheadCount > 0) { + return input.shipDefault === "merge" ? "merge-branch" : "pr"; + } + return null; +} + +function buildPrAction(input: BuildGitActionsInput): GitAction { + if (input.hasPullRequest && input.pullRequestUrl) { + return { + id: "pr", + label: "View PR", + pendingLabel: "View PR", + successLabel: "View PR", + disabled: input.runtime.pr.disabled || !input.githubFeaturesEnabled, + status: input.runtime.pr.status, + description: input.githubFeaturesEnabled ? undefined : "GitHub features unavailable", + icon: input.runtime.pr.icon, + handler: input.runtime.pr.handler, + }; + } + + return { + id: "pr", + label: "Create PR", + pendingLabel: "Creating PR...", + successLabel: "PR Created", + disabled: + input.runtime.pr.disabled || + !input.githubFeaturesEnabled || + input.aheadCount === 0, + status: input.runtime.pr.status, + description: getCreatePrDescription(input), + icon: input.runtime.pr.icon, + handler: input.runtime.pr.handler, + }; +} + +function canMergeFromBase(input: BuildGitActionsInput): boolean { + if (!input.baseRefAvailable || input.hasUncommittedChanges) { + return false; + } + if (!input.isOnBaseBranch) { + return true; + } + if (!input.hasRemote) { + return false; + } + return input.aheadOfOrigin > 0 || input.behindOfOrigin > 0; +} + +function getCreatePrDescription(input: BuildGitActionsInput): string | undefined { + if (!input.githubFeaturesEnabled) { + return "GitHub features unavailable"; + } + if (input.aheadCount === 0) { + return `Branch has no commits ahead of ${input.baseRefLabel}`; + } + return undefined; +} + +function getMergeBranchDescription(input: BuildGitActionsInput): string | undefined { + if (!input.baseRefAvailable) { + return "Base ref unavailable"; + } + if (input.hasUncommittedChanges) { + return "Requires clean working tree"; + } + if (input.aheadCount === 0) { + return `No commits to merge into ${input.baseRefLabel}`; + } + return undefined; +} + +function getMergeFromBaseDescription(input: BuildGitActionsInput): string | undefined { + if (!input.baseRefAvailable) { + return "Base ref unavailable"; + } + if (input.hasUncommittedChanges) { + return "Requires clean working tree"; + } + if (!input.isOnBaseBranch) { + return undefined; + } + if (!input.hasRemote) { + return "No remote configured"; + } + if (input.aheadOfOrigin === 0 && input.behindOfOrigin === 0) { + return "Already up to date"; + } + return undefined; +} diff --git a/packages/app/src/components/git-actions-split-button.tsx b/packages/app/src/components/git-actions-split-button.tsx index c4aad1bff..c746d70ba 100644 --- a/packages/app/src/components/git-actions-split-button.tsx +++ b/packages/app/src/components/git-actions-split-button.tsx @@ -9,7 +9,7 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; -import type { GitAction, GitActions } from "@/hooks/use-git-actions"; +import type { GitAction, GitActions } from "@/components/git-actions-policy"; interface GitActionsSplitButtonProps { gitActions: GitActions; @@ -79,7 +79,11 @@ export function GitActionsSplitButton({ gitActions }: GitActionsSplitButtonProps status={action.status} pendingLabel={action.pendingLabel} successLabel={action.successLabel} - closeOnSelect={action.status === "idle" && action.id === "view-pr"} + closeOnSelect={ + action.status === "idle" && + action.id === "pr" && + action.label === "View PR" + } description={action.description} onSelect={action.handler} > diff --git a/packages/app/src/components/git-diff-pane.tsx b/packages/app/src/components/git-diff-pane.tsx index 95d00cfe8..e157ca86b 100644 --- a/packages/app/src/components/git-diff-pane.tsx +++ b/packages/app/src/components/git-diff-pane.tsx @@ -47,19 +47,19 @@ import { DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; import { GitHubIcon } from "@/components/icons/github-icon"; +import { + buildGitActions, + type GitActions, +} from "@/components/git-actions-policy"; import { WebDesktopScrollbarOverlay, useWebDesktopScrollbarMetrics, } from "@/components/web-desktop-scrollbar"; import { buildNewAgentRoute, resolveNewAgentWorkingDir } from "@/utils/new-agent-routing"; import { openExternalUrl } from "@/utils/open-external-url"; -import { shouldShowMergeFromBaseAction } from "./git-action-visibility"; - -import { type GitActionId, type GitAction, type GitActions } from "@/hooks/use-git-actions"; import { GitActionsSplitButton } from "@/components/git-actions-split-button"; -// Re-export types from shared hook -export type { GitActionId, GitAction, GitActions } from "@/hooks/use-git-actions"; +export type { GitActionId, GitAction, GitActions } from "@/components/git-actions-policy"; function openURLInNewTab(url: string): void { void openExternalUrl(url); @@ -790,19 +790,13 @@ export function GitDiffPane({ serverId, workspaceId, cwd, hideHeaderRow }: GitDi const commitDisabled = actionsDisabled || commitStatus === "pending"; const prDisabled = actionsDisabled || prCreateStatus === "pending"; const mergeDisabled = - actionsDisabled || mergeStatus === "pending" || hasUncommittedChanges || !baseRef; + actionsDisabled || mergeStatus === "pending"; const mergeFromBaseDisabled = - actionsDisabled || - mergeFromBaseStatus === "pending" || - hasUncommittedChanges || - !baseRef || - (isOnBaseBranch && !hasRemote); + actionsDisabled || mergeFromBaseStatus === "pending"; const pushDisabled = - actionsDisabled || pushStatus === "pending" || !(gitStatus?.hasRemote ?? false); + actionsDisabled || pushStatus === "pending"; const archiveDisabled = - actionsDisabled || - archiveStatus === "pending" || - !gitStatus?.isPaseoOwnedWorktree; + actionsDisabled || archiveStatus === "pending"; let bodyContent: ReactElement; @@ -892,184 +886,67 @@ export function GitDiffPane({ serverId, workspaceId, cwd, hideHeaderRow }: GitDi // ========================================================================== const gitActions: GitActions = useMemo(() => { - if (!isGit) { - return { primary: null, secondary: [], menu: [] }; - } - - // Build all possible actions - const allActions = new Map(); - - // Commit - always available - allActions.set("commit", { - id: "commit", - label: "Commit", - pendingLabel: "Committing...", - successLabel: "Committed", - disabled: commitDisabled, - status: commitStatus, - icon: , - handler: handleCommit, + return buildGitActions({ + isGit, + githubFeaturesEnabled, + hasPullRequest, + pullRequestUrl: prStatus?.url ?? null, + hasRemote, + isPaseoOwnedWorktree, + isOnBaseBranch, + hasUncommittedChanges, + baseRefAvailable: Boolean(baseRef), + baseRefLabel, + aheadCount, + aheadOfOrigin, + behindOfOrigin, + shouldPromoteArchive, + shipDefault, + runtime: { + commit: { + disabled: commitDisabled, + status: commitStatus, + icon: , + handler: handleCommit, + }, + push: { + disabled: pushDisabled, + status: pushStatus, + icon: , + handler: handlePush, + }, + pr: { + disabled: prDisabled, + status: hasPullRequest ? "idle" : prCreateStatus, + icon: , + handler: () => { + if (prStatus?.url) { + openURLInNewTab(prStatus.url); + return; + } + handleCreatePr(); + }, + }, + "merge-branch": { + disabled: mergeDisabled, + status: mergeStatus, + icon: , + handler: handleMergeBranch, + }, + "merge-from-base": { + disabled: mergeFromBaseDisabled, + status: mergeFromBaseStatus, + icon: , + handler: handleMergeFromBase, + }, + "archive-worktree": { + disabled: archiveDisabled, + status: archiveStatus, + icon: , + handler: handleArchiveWorktree, + }, + }, }); - - // Push - when has remote - if (hasRemote) { - allActions.set("push", { - id: "push", - label: "Push", - pendingLabel: "Pushing...", - successLabel: "Pushed", - disabled: pushDisabled, - status: pushStatus, - description: !hasRemote ? "No remote configured" : undefined, - icon: , - handler: handlePush, - }); - } - - // View PR - when PR exists - if (githubFeaturesEnabled && hasPullRequest && prStatus?.url) { - const prUrl = prStatus.url; - allActions.set("view-pr", { - id: "view-pr", - label: "View PR", - pendingLabel: "View PR", - successLabel: "View PR", - disabled: false, - status: "idle", - icon: , - handler: () => openURLInNewTab(prUrl), - }); - } - - // Create PR - when ahead of base and no PR - if (githubFeaturesEnabled && aheadCount > 0 && !hasPullRequest) { - allActions.set("create-pr", { - id: "create-pr", - label: "Create PR", - pendingLabel: "Creating PR...", - successLabel: "PR Created", - disabled: prDisabled, - status: prCreateStatus, - icon: , - handler: handleCreatePr, - }); - } - - // Merge branch - when ahead of base - if (aheadCount > 0) { - allActions.set("merge-branch", { - id: "merge-branch", - label: `Merge into ${baseRefLabel}`, - pendingLabel: "Merging...", - successLabel: "Merged", - disabled: mergeDisabled, - status: mergeStatus, - description: hasUncommittedChanges ? "Requires clean working tree" : undefined, - icon: , - handler: handleMergeBranch, - }); - } - - // Update/sync from base - if ( - shouldShowMergeFromBaseAction({ - isOnBaseBranch, - hasRemote, - aheadOfOrigin, - behindOfOrigin, - }) - ) { - allActions.set("merge-from-base", { - id: "merge-from-base", - label: isOnBaseBranch ? "Sync" : `Update from ${baseRefLabel}`, - pendingLabel: "Updating...", - successLabel: "Updated", - disabled: mergeFromBaseDisabled, - status: mergeFromBaseStatus, - description: - hasUncommittedChanges - ? "Requires clean working tree" - : isOnBaseBranch && !hasRemote - ? "No remote configured" - : undefined, - icon: , - handler: handleMergeFromBase, - }); - } - - // Archive worktree - only for Paseo worktrees - if (isPaseoOwnedWorktree) { - allActions.set("archive-worktree", { - id: "archive-worktree", - label: "Archive worktree", - pendingLabel: "Archiving...", - successLabel: "Archived", - disabled: archiveDisabled, - status: archiveStatus, - icon: , - handler: handleArchiveWorktree, - }); - } - - // Select primary action (priority rules) - let primaryActionId: GitActionId | null = null; - - // Rule 0: Post-ship in worktree -> Archive - if (shouldPromoteArchive && allActions.has("archive-worktree")) { - primaryActionId = "archive-worktree"; - } - // Rule 1: Uncommitted changes → Commit - else if (hasUncommittedChanges) { - primaryActionId = "commit"; - } - // Rule 2: Ahead of origin → Push - else if (aheadOfOrigin > 0 && allActions.has("push")) { - primaryActionId = "push"; - } - // Rule 3: Has PR → View PR - else if (hasPullRequest) { - primaryActionId = "view-pr"; - } - // Rule 4: On base branch -> surface sync explicitly - else if (isOnBaseBranch && allActions.has("merge-from-base")) { - primaryActionId = "merge-from-base"; - } - // Rule 5: Ahead of base → Ship action based on preference - else if (aheadCount > 0) { - const preferred: GitActionId = shipDefault === "merge" ? "merge-branch" : "create-pr"; - const fallback: GitActionId = shipDefault === "merge" ? "create-pr" : "merge-branch"; - - const preferredAction = allActions.get(preferred); - const fallbackAction = allActions.get(fallback); - - if (preferredAction && !preferredAction.disabled) { - primaryActionId = preferred; - } else if (fallbackAction && !fallbackAction.disabled) { - primaryActionId = fallback; - } else if (preferredAction) { - primaryActionId = preferred; - } - } - - const primary = primaryActionId ? allActions.get(primaryActionId) ?? null : null; - - // Secondary actions: ship-related + merge from base + push (excluding primary) - const secondaryIds: GitActionId[] = [ - "merge-branch", - "create-pr", - "view-pr", - "merge-from-base", - "push", - "archive-worktree", - ]; - const secondary = secondaryIds - .filter(id => id !== primaryActionId && allActions.has(id)) - .map(id => allActions.get(id)!); - - // Menu actions: none for now (all actionable items are in primary/secondary) - const menu: GitAction[] = []; - - return { primary, secondary, menu }; }, [ isGit, hasRemote, hasPullRequest, prStatus?.url, aheadCount, isPaseoOwnedWorktree, isOnBaseBranch, githubFeaturesEnabled, hasUncommittedChanges, aheadOfOrigin, behindOfOrigin, shipDefault, baseRefLabel, shouldPromoteArchive, diff --git a/packages/app/src/hooks/use-git-actions.ts b/packages/app/src/hooks/use-git-actions.ts index b3ee1e4b2..4ab797d66 100644 --- a/packages/app/src/hooks/use-git-actions.ts +++ b/packages/app/src/hooks/use-git-actions.ts @@ -4,37 +4,14 @@ import AsyncStorage from "@react-native-async-storage/async-storage"; import { useCheckoutGitActionsStore } from "@/stores/checkout-git-actions-store"; import { useCheckoutStatusQuery } from "@/hooks/use-checkout-status-query"; import { useCheckoutPrStatusQuery } from "@/hooks/use-checkout-pr-status-query"; -import { shouldShowMergeFromBaseAction } from "@/components/git-action-visibility"; +import { + buildGitActions, + type GitActions, +} from "@/components/git-actions-policy"; import { buildNewAgentRoute, resolveNewAgentWorkingDir } from "@/utils/new-agent-routing"; import { openExternalUrl } from "@/utils/open-external-url"; -import type { ActionStatus } from "@/components/ui/dropdown-menu"; -export type GitActionId = - | "commit" - | "push" - | "view-pr" - | "create-pr" - | "merge-branch" - | "merge-from-base" - | "archive-worktree"; - -export interface GitAction { - id: GitActionId; - label: string; - pendingLabel: string; - successLabel: string; - disabled: boolean; - status: ActionStatus; - description?: string; - icon?: ReactElement; - handler: () => void; -} - -export interface GitActions { - primary: GitAction | null; - secondary: GitAction[]; - menu: GitAction[]; -} +export type { GitActionId, GitAction, GitActions } from "@/components/git-actions-policy"; function openURLInNewTab(url: string): void { void openExternalUrl(url); @@ -252,19 +229,13 @@ export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): Use const commitDisabled = actionsDisabled || commitStatus === "pending"; const prDisabled = actionsDisabled || prCreateStatus === "pending"; const mergeDisabled = - actionsDisabled || mergeStatus === "pending" || hasUncommittedChanges || !baseRef; + actionsDisabled || mergeStatus === "pending"; const mergeFromBaseDisabled = - actionsDisabled || - mergeFromBaseStatus === "pending" || - hasUncommittedChanges || - !baseRef || - (isOnBaseBranch && !hasRemote); + actionsDisabled || mergeFromBaseStatus === "pending"; const pushDisabled = - actionsDisabled || pushStatus === "pending" || !(gitStatus?.hasRemote ?? false); + actionsDisabled || pushStatus === "pending"; const archiveDisabled = - actionsDisabled || - archiveStatus === "pending" || - !gitStatus?.isPaseoOwnedWorktree; + actionsDisabled || archiveStatus === "pending"; const branchLabel = gitStatus?.currentBranch && gitStatus.currentBranch !== "HEAD" @@ -275,163 +246,67 @@ export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): Use // Build actions const gitActions: GitActions = useMemo(() => { - if (!isGit) { - return { primary: null, secondary: [], menu: [] }; - } - - const allActions = new Map(); - - allActions.set("commit", { - id: "commit", - label: "Commit", - pendingLabel: "Committing...", - successLabel: "Committed", - disabled: commitDisabled, - status: commitStatus, - icon: icons.commit, - handler: handleCommit, + return buildGitActions({ + isGit, + githubFeaturesEnabled, + hasPullRequest, + pullRequestUrl: prStatus?.url ?? null, + hasRemote, + isPaseoOwnedWorktree, + isOnBaseBranch, + hasUncommittedChanges, + baseRefAvailable: Boolean(baseRef), + baseRefLabel, + aheadCount, + aheadOfOrigin, + behindOfOrigin, + shouldPromoteArchive, + shipDefault, + runtime: { + commit: { + disabled: commitDisabled, + status: commitStatus, + icon: icons.commit, + handler: handleCommit, + }, + push: { + disabled: pushDisabled, + status: pushStatus, + icon: icons.push, + handler: handlePush, + }, + pr: { + disabled: prDisabled, + status: hasPullRequest ? "idle" : prCreateStatus, + icon: hasPullRequest ? icons.viewPr : icons.createPr, + handler: () => { + if (prStatus?.url) { + openURLInNewTab(prStatus.url); + return; + } + handleCreatePr(); + }, + }, + "merge-branch": { + disabled: mergeDisabled, + status: mergeStatus, + icon: icons.merge, + handler: handleMergeBranch, + }, + "merge-from-base": { + disabled: mergeFromBaseDisabled, + status: mergeFromBaseStatus, + icon: icons.mergeFromBase, + handler: handleMergeFromBase, + }, + "archive-worktree": { + disabled: archiveDisabled, + status: archiveStatus, + icon: icons.archive, + handler: handleArchiveWorktree, + }, + }, }); - - if (hasRemote) { - allActions.set("push", { - id: "push", - label: "Push", - pendingLabel: "Pushing...", - successLabel: "Pushed", - disabled: pushDisabled, - status: pushStatus, - description: !hasRemote ? "No remote configured" : undefined, - icon: icons.push, - handler: handlePush, - }); - } - - if (githubFeaturesEnabled && hasPullRequest && prStatus?.url) { - const prUrl = prStatus.url; - allActions.set("view-pr", { - id: "view-pr", - label: "View PR", - pendingLabel: "View PR", - successLabel: "View PR", - disabled: false, - status: "idle", - icon: icons.viewPr, - handler: () => openURLInNewTab(prUrl), - }); - } - - if (githubFeaturesEnabled && aheadCount > 0 && !hasPullRequest) { - allActions.set("create-pr", { - id: "create-pr", - label: "Create PR", - pendingLabel: "Creating PR...", - successLabel: "PR Created", - disabled: prDisabled, - status: prCreateStatus, - icon: icons.createPr, - handler: handleCreatePr, - }); - } - - if (aheadCount > 0) { - allActions.set("merge-branch", { - id: "merge-branch", - label: `Merge into ${baseRefLabel}`, - pendingLabel: "Merging...", - successLabel: "Merged", - disabled: mergeDisabled, - status: mergeStatus, - description: hasUncommittedChanges ? "Requires clean working tree" : undefined, - icon: icons.merge, - handler: handleMergeBranch, - }); - } - - if ( - shouldShowMergeFromBaseAction({ - isOnBaseBranch, - hasRemote, - aheadOfOrigin, - behindOfOrigin, - }) - ) { - allActions.set("merge-from-base", { - id: "merge-from-base", - label: isOnBaseBranch ? "Sync" : `Update from ${baseRefLabel}`, - pendingLabel: "Updating...", - successLabel: "Updated", - disabled: mergeFromBaseDisabled, - status: mergeFromBaseStatus, - description: - hasUncommittedChanges - ? "Requires clean working tree" - : isOnBaseBranch && !hasRemote - ? "No remote configured" - : undefined, - icon: icons.mergeFromBase, - handler: handleMergeFromBase, - }); - } - - if (isPaseoOwnedWorktree) { - allActions.set("archive-worktree", { - id: "archive-worktree", - label: "Archive worktree", - pendingLabel: "Archiving...", - successLabel: "Archived", - disabled: archiveDisabled, - status: archiveStatus, - icon: icons.archive, - handler: handleArchiveWorktree, - }); - } - - // Select primary action (priority rules) - let primaryActionId: GitActionId | null = null; - - if (shouldPromoteArchive && allActions.has("archive-worktree")) { - primaryActionId = "archive-worktree"; - } else if (hasUncommittedChanges) { - primaryActionId = "commit"; - } else if (aheadOfOrigin > 0 && allActions.has("push")) { - primaryActionId = "push"; - } else if (hasPullRequest) { - primaryActionId = "view-pr"; - } else if (isOnBaseBranch && allActions.has("merge-from-base")) { - primaryActionId = "merge-from-base"; - } else if (aheadCount > 0) { - const preferred: GitActionId = shipDefault === "merge" ? "merge-branch" : "create-pr"; - const fallback: GitActionId = shipDefault === "merge" ? "create-pr" : "merge-branch"; - - const preferredAction = allActions.get(preferred); - const fallbackAction = allActions.get(fallback); - - if (preferredAction && !preferredAction.disabled) { - primaryActionId = preferred; - } else if (fallbackAction && !fallbackAction.disabled) { - primaryActionId = fallback; - } else if (preferredAction) { - primaryActionId = preferred; - } - } - - const primary = primaryActionId ? allActions.get(primaryActionId) ?? null : null; - - const secondaryIds: GitActionId[] = [ - "merge-branch", - "create-pr", - "view-pr", - "merge-from-base", - "push", - "archive-worktree", - ]; - const secondary = secondaryIds - .filter(id => id !== primaryActionId && allActions.has(id)) - .map(id => allActions.get(id)!); - - const menu: GitAction[] = []; - - return { primary, secondary, menu }; }, [ isGit, hasRemote, hasPullRequest, prStatus?.url, aheadCount, isPaseoOwnedWorktree, isOnBaseBranch, githubFeaturesEnabled, hasUncommittedChanges, aheadOfOrigin, behindOfOrigin, shipDefault, baseRefLabel, shouldPromoteArchive,