Merge pull request #57 from getpaseo/investigate/git-workflow-ux-improvements
Improve post-ship worktree flow and merged PR handling
This commit is contained in:
@@ -394,7 +394,7 @@ test('checkout-first Changes panel ship loop', async ({ page }) => {
|
||||
await expect(page.getByTestId('changes-menu-archive-worktree')).toBeVisible();
|
||||
await page.getByTestId('changes-menu-archive-worktree').click();
|
||||
// Archiving a worktree deletes agents and redirects to home
|
||||
await expect(page).toHaveURL(/\/agent\/?$/, { timeout: 30000 });
|
||||
await expect(page).toHaveURL(/\/agent\/?(?:\?.*)?$/, { timeout: 30000 });
|
||||
await setWorkingDirectory(page, repo.path);
|
||||
await ensureHostSelected(page);
|
||||
// Repo inspection is async; wait until git options are interactive again.
|
||||
|
||||
@@ -50,7 +50,7 @@ import {
|
||||
type ActionStatus,
|
||||
} from "@/components/ui/dropdown-menu";
|
||||
import { GitHubIcon } from "@/components/icons/github-icon";
|
||||
import { buildHostAgentDraftRoute } from "@/utils/host-routes";
|
||||
import { buildNewAgentRoute, resolveNewAgentWorkingDir } from "@/utils/new-agent-routing";
|
||||
import { openExternalUrl } from "@/utils/open-external-url";
|
||||
|
||||
// =============================================================================
|
||||
@@ -472,6 +472,7 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
const router = useRouter();
|
||||
const [diffModeOverride, setDiffModeOverride] = useState<"uncommitted" | "base" | null>(null);
|
||||
const [actionError, setActionError] = useState<string | null>(null);
|
||||
const [postShipArchiveSuggested, setPostShipArchiveSuggested] = useState(false);
|
||||
const [shipDefault, setShipDefault] = useState<"merge" | "pr">("merge");
|
||||
const { status, isLoading: isStatusLoading, isFetching: isStatusFetching, isError: isStatusError, error: statusError, refresh: refreshStatus } =
|
||||
useCheckoutStatusQuery({ serverId, cwd });
|
||||
@@ -796,10 +797,14 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
}
|
||||
void persistShipDefault("merge");
|
||||
setActionError(null);
|
||||
void runMergeBranch({ serverId, cwd, baseRef }).catch((err) => {
|
||||
const message = err instanceof Error ? err.message : "Failed to merge";
|
||||
setActionError(message);
|
||||
});
|
||||
void runMergeBranch({ serverId, cwd, baseRef })
|
||||
.then(() => {
|
||||
setPostShipArchiveSuggested(true);
|
||||
})
|
||||
.catch((err) => {
|
||||
const message = err instanceof Error ? err.message : "Failed to merge";
|
||||
setActionError(message);
|
||||
});
|
||||
}, [baseRef, persistShipDefault, runMergeBranch, serverId, cwd]);
|
||||
|
||||
const handleMergeFromBase = useCallback(() => {
|
||||
@@ -821,15 +826,16 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
return;
|
||||
}
|
||||
setActionError(null);
|
||||
const targetWorkingDir = resolveNewAgentWorkingDir(cwd, status ?? null);
|
||||
void runArchiveWorktree({ serverId, cwd, worktreePath })
|
||||
.then(() => {
|
||||
router.replace(buildHostAgentDraftRoute(serverId) as any);
|
||||
router.replace(buildNewAgentRoute(serverId, targetWorkingDir) as any);
|
||||
})
|
||||
.catch((err) => {
|
||||
const message = err instanceof Error ? err.message : "Failed to archive worktree";
|
||||
setActionError(message);
|
||||
});
|
||||
}, [runArchiveWorktree, router, serverId, cwd, status?.cwd]);
|
||||
}, [runArchiveWorktree, router, serverId, cwd, status]);
|
||||
|
||||
const renderFlatItem = useCallback(
|
||||
({ item }: { item: DiffFlatItem }) => {
|
||||
@@ -887,12 +893,27 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
? undefined
|
||||
: `${branchLabel} -> ${baseRefLabel}`;
|
||||
}, [baseRefLabel, branchLabel]);
|
||||
const hasPullRequest = Boolean(prStatus?.url);
|
||||
const hasRemote = gitStatus?.hasRemote ?? false;
|
||||
const isPaseoOwnedWorktree = gitStatus?.isPaseoOwnedWorktree ?? false;
|
||||
const isMergedPullRequest = Boolean(prStatus?.isMerged);
|
||||
const currentBranch = gitStatus?.currentBranch;
|
||||
const isOnBaseBranch = currentBranch === baseRefLabel;
|
||||
const shouldPromoteArchive =
|
||||
isPaseoOwnedWorktree &&
|
||||
!hasUncommittedChanges &&
|
||||
(postShipArchiveSuggested || isMergedPullRequest);
|
||||
|
||||
const commitDisabled = actionsDisabled || commitStatus === "pending";
|
||||
const prDisabled = actionsDisabled || prCreateStatus === "pending";
|
||||
const mergeDisabled =
|
||||
actionsDisabled || mergeStatus === "pending" || hasUncommittedChanges || !baseRef;
|
||||
const mergeFromBaseDisabled =
|
||||
actionsDisabled || mergeFromBaseStatus === "pending" || hasUncommittedChanges || !baseRef;
|
||||
actionsDisabled ||
|
||||
mergeFromBaseStatus === "pending" ||
|
||||
hasUncommittedChanges ||
|
||||
!baseRef ||
|
||||
(isOnBaseBranch && !hasRemote);
|
||||
const pushDisabled =
|
||||
actionsDisabled || pushStatus === "pending" || !(gitStatus?.hasRemote ?? false);
|
||||
const archiveDisabled =
|
||||
@@ -969,11 +990,9 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
);
|
||||
}
|
||||
|
||||
const hasPullRequest = Boolean(prStatus?.url);
|
||||
const hasRemote = gitStatus?.hasRemote ?? false;
|
||||
const isPaseoOwnedWorktree = gitStatus?.isPaseoOwnedWorktree ?? false;
|
||||
const currentBranch = gitStatus?.currentBranch;
|
||||
const isOnBaseBranch = currentBranch === baseRefLabel;
|
||||
useEffect(() => {
|
||||
setPostShipArchiveSuggested(false);
|
||||
}, [cwd]);
|
||||
|
||||
// ==========================================================================
|
||||
// Git Actions (Data-Oriented)
|
||||
@@ -1063,16 +1082,21 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
});
|
||||
}
|
||||
|
||||
// Update from base - only when not on base branch
|
||||
if (!isOnBaseBranch) {
|
||||
// Update/sync from base
|
||||
if (!isOnBaseBranch || hasRemote) {
|
||||
allActions.set("merge-from-base", {
|
||||
id: "merge-from-base",
|
||||
label: `Update from ${baseRefLabel}`,
|
||||
label: isOnBaseBranch ? `Sync ${baseRefLabel} from origin` : `Update from ${baseRefLabel}`,
|
||||
pendingLabel: "Updating...",
|
||||
successLabel: "Updated",
|
||||
disabled: mergeFromBaseDisabled,
|
||||
status: mergeFromBaseStatus,
|
||||
description: hasUncommittedChanges ? "Requires clean working tree" : undefined,
|
||||
description:
|
||||
hasUncommittedChanges
|
||||
? "Requires clean working tree"
|
||||
: isOnBaseBranch && !hasRemote
|
||||
? "No remote configured"
|
||||
: undefined,
|
||||
icon: <RefreshCcw size={16} color={theme.colors.foregroundMuted} />,
|
||||
handler: handleMergeFromBase,
|
||||
});
|
||||
@@ -1095,8 +1119,12 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
// 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
|
||||
if (hasUncommittedChanges) {
|
||||
else if (hasUncommittedChanges) {
|
||||
primaryActionId = "commit";
|
||||
}
|
||||
// Rule 2: Ahead of origin → Push
|
||||
@@ -1107,7 +1135,11 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
else if (hasPullRequest) {
|
||||
primaryActionId = "view-pr";
|
||||
}
|
||||
// Rule 4: Ahead of base → Ship action based on preference
|
||||
// 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";
|
||||
@@ -1127,20 +1159,25 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
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"];
|
||||
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: archive worktree only
|
||||
const menu = allActions.has("archive-worktree")
|
||||
? [allActions.get("archive-worktree")!]
|
||||
: [];
|
||||
// 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, shipDefault, baseRefLabel,
|
||||
hasUncommittedChanges, aheadOfOrigin, shipDefault, baseRefLabel, shouldPromoteArchive,
|
||||
commitDisabled, pushDisabled, prDisabled, mergeDisabled, mergeFromBaseDisabled, archiveDisabled,
|
||||
commitStatus, pushStatus, prCreateStatus, mergeStatus, mergeFromBaseStatus, archiveStatus,
|
||||
handleCommit, handlePush, handleCreatePr, handleMergeBranch, handleMergeFromBase, handleArchiveWorktree,
|
||||
|
||||
@@ -1690,6 +1690,7 @@ const CheckoutPrStatusSchema = z.object({
|
||||
state: z.string(),
|
||||
baseRefName: z.string(),
|
||||
headRefName: z.string(),
|
||||
isMerged: z.boolean(),
|
||||
});
|
||||
|
||||
export const CheckoutPrStatusResponseSchema = z.object({
|
||||
|
||||
@@ -471,6 +471,109 @@ describe("checkout git utilities", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("returns merged PR status when no open PR exists for the current branch", async () => {
|
||||
execSync("git checkout -b feature", { cwd: repoDir });
|
||||
execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir });
|
||||
|
||||
const fakeBinDir = join(tempDir, "fake-bin-gh-merged");
|
||||
mkdirSync(fakeBinDir);
|
||||
const gitPath = execSync("command -v git", { stdio: "pipe" }).toString().trim();
|
||||
symlinkSync(gitPath, join(fakeBinDir, "git"));
|
||||
writeFileSync(
|
||||
join(fakeBinDir, "gh"),
|
||||
[
|
||||
"#!/usr/bin/env bash",
|
||||
"set -euo pipefail",
|
||||
"if [[ \"${1-}\" == \"--version\" ]]; then",
|
||||
" echo \"gh version 2.0.0\"",
|
||||
" exit 0",
|
||||
"fi",
|
||||
"args=\"$*\"",
|
||||
"if [[ \"$args\" == *\"state=open\"* ]]; then",
|
||||
" echo \"[]\"",
|
||||
" exit 0",
|
||||
"fi",
|
||||
"if [[ \"$args\" == *\"state=closed\"* ]]; then",
|
||||
" echo '[{\"html_url\":\"https://github.com/getpaseo/paseo/pull/123\",\"title\":\"Ship feature\",\"state\":\"closed\",\"merged_at\":\"2026-02-18T00:00:00Z\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature\"}}]'",
|
||||
" exit 0",
|
||||
"fi",
|
||||
"echo \"unexpected gh args: $args\" >&2",
|
||||
"exit 1",
|
||||
"",
|
||||
].join("\n"),
|
||||
"utf8"
|
||||
);
|
||||
execSync(`chmod +x ${join(fakeBinDir, "gh")}`);
|
||||
|
||||
const originalPath = process.env.PATH;
|
||||
process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`;
|
||||
try {
|
||||
const status = await getPullRequestStatus(repoDir);
|
||||
expect(status.githubFeaturesEnabled).toBe(true);
|
||||
expect(status.status).not.toBeNull();
|
||||
expect(status.status?.url).toContain("/pull/123");
|
||||
expect(status.status?.baseRefName).toBe("main");
|
||||
expect(status.status?.headRefName).toBe("feature");
|
||||
expect(status.status?.isMerged).toBe(true);
|
||||
expect(status.status?.state).toBe("merged");
|
||||
} finally {
|
||||
if (originalPath === undefined) {
|
||||
delete process.env.PATH;
|
||||
} else {
|
||||
process.env.PATH = originalPath;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("does not treat closed-unmerged PRs as shipped status", async () => {
|
||||
execSync("git checkout -b feature", { cwd: repoDir });
|
||||
execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir });
|
||||
|
||||
const fakeBinDir = join(tempDir, "fake-bin-gh-closed");
|
||||
mkdirSync(fakeBinDir);
|
||||
const gitPath = execSync("command -v git", { stdio: "pipe" }).toString().trim();
|
||||
symlinkSync(gitPath, join(fakeBinDir, "git"));
|
||||
writeFileSync(
|
||||
join(fakeBinDir, "gh"),
|
||||
[
|
||||
"#!/usr/bin/env bash",
|
||||
"set -euo pipefail",
|
||||
"if [[ \"${1-}\" == \"--version\" ]]; then",
|
||||
" echo \"gh version 2.0.0\"",
|
||||
" exit 0",
|
||||
"fi",
|
||||
"args=\"$*\"",
|
||||
"if [[ \"$args\" == *\"state=open\"* ]]; then",
|
||||
" echo \"[]\"",
|
||||
" exit 0",
|
||||
"fi",
|
||||
"if [[ \"$args\" == *\"state=closed\"* ]]; then",
|
||||
" echo '[{\"html_url\":\"https://github.com/getpaseo/paseo/pull/999\",\"title\":\"Closed without merge\",\"state\":\"closed\",\"merged_at\":null,\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature\"}}]'",
|
||||
" exit 0",
|
||||
"fi",
|
||||
"echo \"unexpected gh args: $args\" >&2",
|
||||
"exit 1",
|
||||
"",
|
||||
].join("\n"),
|
||||
"utf8"
|
||||
);
|
||||
execSync(`chmod +x ${join(fakeBinDir, "gh")}`);
|
||||
|
||||
const originalPath = process.env.PATH;
|
||||
process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`;
|
||||
try {
|
||||
const status = await getPullRequestStatus(repoDir);
|
||||
expect(status.githubFeaturesEnabled).toBe(true);
|
||||
expect(status.status).toBeNull();
|
||||
} finally {
|
||||
if (originalPath === undefined) {
|
||||
delete process.env.PATH;
|
||||
} else {
|
||||
process.env.PATH = originalPath;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("returns typed MergeConflictError on merge conflicts", async () => {
|
||||
const conflictFile = join(repoDir, "conflict.txt");
|
||||
writeFileSync(conflictFile, "base\n");
|
||||
|
||||
@@ -1517,6 +1517,7 @@ export interface PullRequestStatus {
|
||||
state: string;
|
||||
baseRefName: string;
|
||||
headRefName: string;
|
||||
isMerged: boolean;
|
||||
}
|
||||
|
||||
export interface PullRequestStatusResult {
|
||||
@@ -1592,6 +1593,62 @@ async function resolveGitHubRepo(cwd: string): Promise<string | null> {
|
||||
return null;
|
||||
}
|
||||
|
||||
async function listPullRequestsForHead(options: {
|
||||
cwd: string;
|
||||
repo: string;
|
||||
owner: string;
|
||||
head: string;
|
||||
state: "open" | "closed";
|
||||
}): Promise<any[]> {
|
||||
const { stdout } = await execFileAsync(
|
||||
"gh",
|
||||
[
|
||||
"api",
|
||||
`repos/${options.repo}/pulls`,
|
||||
"-X",
|
||||
"GET",
|
||||
"-F",
|
||||
`head=${options.owner}:${options.head}`,
|
||||
"-F",
|
||||
`state=${options.state}`,
|
||||
],
|
||||
{ cwd: options.cwd }
|
||||
);
|
||||
const parsed = JSON.parse(stdout.trim());
|
||||
return Array.isArray(parsed) ? parsed : [];
|
||||
}
|
||||
|
||||
function buildPullRequestStatus(current: any, fallbackHead: string): PullRequestStatus | null {
|
||||
if (!current || typeof current !== "object") {
|
||||
return null;
|
||||
}
|
||||
const url = current.html_url ?? current.url;
|
||||
const title = current.title;
|
||||
if (typeof url !== "string" || typeof title !== "string" || !url || !title) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const mergedAt =
|
||||
typeof current.merged_at === "string" && current.merged_at.trim().length > 0
|
||||
? current.merged_at
|
||||
: null;
|
||||
const state =
|
||||
mergedAt !== null
|
||||
? "merged"
|
||||
: typeof current.state === "string" && current.state.trim().length > 0
|
||||
? current.state
|
||||
: "";
|
||||
|
||||
return {
|
||||
url,
|
||||
title,
|
||||
state,
|
||||
baseRefName: current.base?.ref ?? "",
|
||||
headRefName: current.head?.ref ?? fallbackHead,
|
||||
isMerged: mergedAt !== null,
|
||||
};
|
||||
}
|
||||
|
||||
export async function createPullRequest(
|
||||
cwd: string,
|
||||
options: CreatePullRequestOptions
|
||||
@@ -1652,22 +1709,15 @@ export async function getPullRequestStatus(cwd: string): Promise<PullRequestStat
|
||||
};
|
||||
}
|
||||
const owner = repo.split("/")[0];
|
||||
let stdout: string;
|
||||
let openPulls: any[];
|
||||
try {
|
||||
({ stdout } = await execFileAsync(
|
||||
"gh",
|
||||
[
|
||||
"api",
|
||||
`repos/${repo}/pulls`,
|
||||
"-X",
|
||||
"GET",
|
||||
"-F",
|
||||
`head=${owner}:${head}`,
|
||||
"-F",
|
||||
"state=open",
|
||||
],
|
||||
{ cwd }
|
||||
));
|
||||
openPulls = await listPullRequestsForHead({
|
||||
cwd,
|
||||
repo,
|
||||
owner,
|
||||
head,
|
||||
state: "open",
|
||||
});
|
||||
} catch (error) {
|
||||
if (isGhAuthError(error)) {
|
||||
return {
|
||||
@@ -1677,22 +1727,50 @@ export async function getPullRequestStatus(cwd: string): Promise<PullRequestStat
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
const parsed = JSON.parse(stdout.trim());
|
||||
const current = Array.isArray(parsed) && parsed.length > 0 ? parsed[0] : null;
|
||||
if (!current) {
|
||||
const openPull = openPulls[0] ?? null;
|
||||
const openStatus = buildPullRequestStatus(openPull, head);
|
||||
if (openStatus) {
|
||||
return {
|
||||
status: openStatus,
|
||||
githubFeaturesEnabled: true,
|
||||
};
|
||||
}
|
||||
|
||||
let closedPulls: any[];
|
||||
try {
|
||||
closedPulls = await listPullRequestsForHead({
|
||||
cwd,
|
||||
repo,
|
||||
owner,
|
||||
head,
|
||||
state: "closed",
|
||||
});
|
||||
} catch (error) {
|
||||
if (isGhAuthError(error)) {
|
||||
return {
|
||||
status: null,
|
||||
githubFeaturesEnabled: false,
|
||||
};
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
const mergedClosedPull =
|
||||
closedPulls.find(
|
||||
(entry) =>
|
||||
entry &&
|
||||
typeof entry === "object" &&
|
||||
typeof entry.merged_at === "string" &&
|
||||
entry.merged_at.trim().length > 0
|
||||
) ?? null;
|
||||
const mergedStatus = buildPullRequestStatus(mergedClosedPull, head);
|
||||
if (!mergedStatus) {
|
||||
return {
|
||||
status: null,
|
||||
githubFeaturesEnabled: true,
|
||||
};
|
||||
}
|
||||
return {
|
||||
status: {
|
||||
url: current.html_url ?? current.url,
|
||||
title: current.title,
|
||||
state: current.state,
|
||||
baseRefName: current.base?.ref ?? "",
|
||||
headRefName: current.head?.ref ?? head,
|
||||
},
|
||||
status: mergedStatus,
|
||||
githubFeaturesEnabled: true,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user