From 20c03355f99e973894a2e12e0e08b477e0970e13 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 5 Jun 2026 22:56:33 +0700 Subject: [PATCH] Default git shipping to pull requests --- packages/app/src/git/policy.test.ts | 26 ++++++++++++++++++++++---- packages/app/src/git/policy.ts | 19 ++++++++++++++++--- packages/app/src/git/use-actions.ts | 8 ++++++-- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/app/src/git/policy.test.ts b/packages/app/src/git/policy.test.ts index d58549fa8..f9cde8948 100644 --- a/packages/app/src/git/policy.test.ts +++ b/packages/app/src/git/policy.test.ts @@ -49,7 +49,7 @@ function createInput(overrides: Partial = {}): BuildGitAct aheadOfOrigin: 0, behindOfOrigin: 0, shouldPromoteArchive: false, - shipDefault: "merge", + shipDefault: "pr", runtime: { commit: { disabled: false, @@ -387,13 +387,31 @@ describe("git-actions-policy", () => { }); }); - it("promotes local merge over update-from-base", () => { + it("promotes Create PR over push and local merge when PR is the ship default", () => { + const actions = buildGitActions( + createInput({ + hasRemote: true, + isOnBaseBranch: false, + aheadCount: 2, + aheadOfOrigin: 2, + behindBaseCount: 3, + }), + ); + + expect(actions.primary).toMatchObject({ + id: "pr", + label: "Create PR", + }); + }); + + it("uses local merge when merge is the stored ship default", () => { const actions = buildGitActions( createInput({ hasRemote: true, isOnBaseBranch: false, aheadCount: 2, behindBaseCount: 3, + shipDefault: "merge", }), ); @@ -665,7 +683,7 @@ describe("git-actions-policy", () => { }), ); - expect(actions.primary).toMatchObject({ id: "merge-branch", label: "Merge locally" }); + expect(actions.primary).toMatchObject({ id: "pr", label: "View PR" }); expect(actions.secondary.some((action) => action.id.startsWith("enable-pr-auto-merge"))).toBe( false, ); @@ -766,7 +784,7 @@ describe("git-actions-policy", () => { }), ); - expect(actions.primary).toMatchObject({ id: "merge-branch", label: "Merge locally" }); + expect(actions.primary).toMatchObject({ id: "pr", label: "View PR" }); expect( actions.secondary.some((action) => ["merge-pr-squash", "merge-pr-merge", "merge-pr-rebase"].includes(action.id), diff --git a/packages/app/src/git/policy.ts b/packages/app/src/git/policy.ts index f70f24a26..69d09323e 100644 --- a/packages/app/src/git/policy.ts +++ b/packages/app/src/git/policy.ts @@ -333,9 +333,6 @@ function getPrimaryActionId(input: BuildGitActionsInput): GitActionId | null { if (canPull(input)) { return "pull"; } - if (canPush(input)) { - return "push"; - } if (canMergePr(input)) { return getDefaultDirectPullRequestMergeActionId(input); } @@ -345,6 +342,12 @@ function getPrimaryActionId(input: BuildGitActionsInput): GitActionId | null { if (hasEnabledPrAutoMerge(input)) { return "pr"; } + if (input.shipDefault === "pr" && canUsePullRequestActionAsShipDefault(input)) { + return "pr"; + } + if (canPush(input)) { + return "push"; + } if (!input.isOnBaseBranch && input.aheadCount > 0) { return "merge-branch"; } @@ -502,6 +505,16 @@ function canMergeFromBase(input: BuildGitActionsInput): boolean { ); } +function canUsePullRequestActionAsShipDefault(input: BuildGitActionsInput): boolean { + if (input.isOnBaseBranch || !input.githubFeaturesEnabled) { + return false; + } + if (input.hasPullRequest) { + return input.pullRequestUrl !== null; + } + return input.aheadCount > 0; +} + function canMergePr(input: BuildGitActionsInput): boolean { const github = input.pullRequestGithub; const canMergeFromPullRequestStatus = diff --git a/packages/app/src/git/use-actions.ts b/packages/app/src/git/use-actions.ts index ac9b6c4ff..c1ab3f087 100644 --- a/packages/app/src/git/use-actions.ts +++ b/packages/app/src/git/use-actions.ts @@ -155,7 +155,7 @@ interface UseGitActionsResult { export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): UseGitActionsResult { const toast = useToast(); const [postShipArchiveSuggested, setPostShipArchiveSuggested] = useState(false); - const [shipDefault, setShipDefault] = useState<"merge" | "pr">("merge"); + const [shipDefault, setShipDefault] = useState<"merge" | "pr">("pr"); const { status, isLoading: isStatusLoading } = useCheckoutStatusQuery({ serverId, cwd }); const gitStatus = status && status.isGit ? status : null; @@ -186,15 +186,19 @@ export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): Use useEffect(() => { if (!shipDefaultStorageKey) { + setShipDefault("pr"); return; } let isActive = true; + setShipDefault("pr"); AsyncStorage.getItem(shipDefaultStorageKey) .then((value) => { if (!isActive) return; if (value === "pr" || value === "merge") { setShipDefault(value); + return; } + setShipDefault("pr"); return; }) .catch(() => undefined); @@ -210,7 +214,7 @@ export function useGitActions({ serverId, cwd, icons }: UseGitActionsInput): Use try { await AsyncStorage.setItem(shipDefaultStorageKey, next); } catch { - // Ignore persistence failures; default will reset to "merge". + // Ignore persistence failures; default will reset to "pr". } }, [shipDefaultStorageKey],