Add Update Branch Button to PR Detail View (#1242)
* auto-claude: subtask-1-1 - Add IPC channel constant GITHUB_PR_UPDATE_BRANCH Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * auto-claude: subtask-1-2 - Add IPC handler for update branch in pr-handlers.ts Added GITHUB_PR_UPDATE_BRANCH IPC handler that: - Uses gh CLI to update PR branch with base branch - Validates PR number to prevent command injection - Returns success/error status for UI feedback - Follows existing patterns from GITHUB_PR_MERGE handler Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * auto-claude: subtask-2-1 - Add updatePRBranch method to github-api.ts preload * auto-claude: subtask-3-1 - Add English translation keys to en/common.json Added translation keys for Update Branch feature: - updateBranch: "Update Branch" - updatingBranch: "Updating..." - branchUpdated: "Branch updated" - branchUpdateFailed: "Failed to update branch" Also added missing updatePRBranch mock to browser-mock.ts to fix TypeScript type error. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * auto-claude: subtask-3-2 - Add French translation keys to fr/common.json Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * auto-claude: subtask-4-1 - Add GitBranch icon import to PRDetail.tsx * auto-claude: subtask-4-2 - Add state variables for branch update operation * auto-claude: subtask-4-3 - Add state reset in the PR change useEffect * auto-claude: subtask-4-4 - Add mergeReadinessRefreshKey to checkMergeReadiness useEffect dependency * auto-claude: subtask-4-5 - Add handleUpdateBranch handler function * auto-claude: subtask-4-6 - Add Update Branch button inside the warning banner Added an Update Branch button inside the merge readiness warning banner that displays when the PR branch is behind base. The button: - Only shows when mergeReadiness.isBehind is true - Shows loading state while updating - Displays success/error feedback inline - Uses existing i18n translation keys - Matches the existing styling patterns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address QA issues for Update Branch feature (qa-requested) Fixes: - Add useEffect for auto-dismiss of branchUpdateSuccess after 3 seconds - Change RefreshCw to GitBranch icon in Update Branch button (per spec) - Add user-friendly error messages for permission/conflict/up-to-date cases - Add setIsUpdatingBranch(false) to PR change useEffect for state reset Verified: - All 1761 tests pass - TypeScript build successful - Issues verified locally QA Fix Session: 1 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address PR review issues for Update Branch feature - Remove accidentally committed node_modules symlinks from git index - Update .gitignore to catch symlink files (node_modules without trailing slash) - Replace blocking execFileSync with async execFileAsync pattern - Move success/error messages outside isBehind block for visibility - Expand error message mapping for common failure scenarios * fix: move success/error messages outside Card and fix case-sensitivity - Move branchUpdateSuccess/Error outside blockers Card so they persist - Use toLowerCase() for 'already up to date' error check consistency --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Test User <test@example.com>
This commit is contained in:
Andy
+1
-1
@@ -107,7 +107,7 @@ dmypy.json
|
||||
# ===========================
|
||||
# Node.js (apps/frontend)
|
||||
# ===========================
|
||||
node_modules/
|
||||
node_modules
|
||||
.npm
|
||||
.yarn/
|
||||
.pnp.*
|
||||
|
||||
@@ -2329,6 +2329,65 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
|
||||
}
|
||||
);
|
||||
|
||||
// Update PR branch (sync with base branch)
|
||||
ipcMain.handle(
|
||||
IPC_CHANNELS.GITHUB_PR_UPDATE_BRANCH,
|
||||
async (_, projectId: string, prNumber: number): Promise<{ success: boolean; error?: string }> => {
|
||||
debugLog("updateBranch handler called", { projectId, prNumber });
|
||||
|
||||
const updateResult = await withProjectOrNull(projectId, async (project) => {
|
||||
try {
|
||||
const { execFile } = await import("child_process");
|
||||
const { promisify } = await import("util");
|
||||
const execFileAsync = promisify(execFile);
|
||||
debugLog("Updating PR branch", { prNumber });
|
||||
|
||||
// Validate prNumber to prevent command injection
|
||||
if (!Number.isInteger(prNumber) || prNumber <= 0) {
|
||||
throw new Error("Invalid PR number");
|
||||
}
|
||||
|
||||
// Use gh pr update-branch to sync with base branch (async to avoid blocking main process)
|
||||
// --rebase is not used to avoid force-push requirements
|
||||
await execFileAsync("gh", ["pr", "update-branch", String(prNumber)], {
|
||||
cwd: project.path,
|
||||
env: getAugmentedEnv(),
|
||||
});
|
||||
|
||||
debugLog("PR branch updated successfully", { prNumber });
|
||||
return { success: true };
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
debugLog("Failed to update PR branch", { prNumber, error: errorMessage });
|
||||
|
||||
// Map common error patterns to user-friendly messages
|
||||
let friendlyError = errorMessage;
|
||||
if (errorMessage.includes("permission") || errorMessage.includes("403")) {
|
||||
friendlyError = "You don't have permission to update this branch.";
|
||||
} else if (errorMessage.includes("401") || errorMessage.toLowerCase().includes("auth") || errorMessage.toLowerCase().includes("token")) {
|
||||
friendlyError = "Authentication failed. Try running 'gh auth login' to re-authenticate.";
|
||||
} else if (errorMessage.includes("404") || errorMessage.includes("not found")) {
|
||||
friendlyError = "Pull request not found. It may have been closed or deleted.";
|
||||
} else if (errorMessage.includes("429") || errorMessage.toLowerCase().includes("rate limit")) {
|
||||
friendlyError = "GitHub API rate limit exceeded. Please wait and try again.";
|
||||
} else if (errorMessage.includes("conflict")) {
|
||||
friendlyError = "Cannot update branch due to merge conflicts. Resolve conflicts manually.";
|
||||
} else if (errorMessage.toLowerCase().includes("protected") || errorMessage.toLowerCase().includes("branch protection")) {
|
||||
friendlyError = "Branch protection rules prevent this update.";
|
||||
} else if (errorMessage.includes("ENOTFOUND") || errorMessage.includes("ECONNREFUSED") || errorMessage.includes("ETIMEDOUT")) {
|
||||
friendlyError = "Network error. Check your internet connection and try again.";
|
||||
} else if (errorMessage.toLowerCase().includes("already up to date")) {
|
||||
return { success: true }; // Not an error
|
||||
}
|
||||
|
||||
return { success: false, error: friendlyError };
|
||||
}
|
||||
});
|
||||
|
||||
return updateResult ?? { success: false, error: "Project not found" };
|
||||
}
|
||||
);
|
||||
|
||||
// Run follow-up review
|
||||
ipcMain.on(
|
||||
IPC_CHANNELS.GITHUB_PR_FOLLOWUP_REVIEW,
|
||||
|
||||
@@ -278,6 +278,7 @@ export interface GitHubAPI {
|
||||
// Follow-up review operations
|
||||
checkNewCommits: (projectId: string, prNumber: number) => Promise<NewCommitsCheck>;
|
||||
checkMergeReadiness: (projectId: string, prNumber: number) => Promise<MergeReadiness>;
|
||||
updatePRBranch: (projectId: string, prNumber: number) => Promise<{ success: boolean; error?: string }>;
|
||||
runFollowupReview: (projectId: string, prNumber: number) => void;
|
||||
|
||||
// PR logs
|
||||
@@ -690,6 +691,9 @@ export const createGitHubAPI = (): GitHubAPI => ({
|
||||
checkMergeReadiness: (projectId: string, prNumber: number): Promise<MergeReadiness> =>
|
||||
invokeIpc(IPC_CHANNELS.GITHUB_PR_CHECK_MERGE_READINESS, projectId, prNumber),
|
||||
|
||||
updatePRBranch: (projectId: string, prNumber: number): Promise<{ success: boolean; error?: string }> =>
|
||||
invokeIpc(IPC_CHANNELS.GITHUB_PR_UPDATE_BRANCH, projectId, prNumber),
|
||||
|
||||
runFollowupReview: (projectId: string, prNumber: number): void =>
|
||||
sendIpc(IPC_CHANNELS.GITHUB_PR_FOLLOWUP_REVIEW, projectId, prNumber),
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
Send,
|
||||
XCircle,
|
||||
Loader2,
|
||||
GitBranch,
|
||||
GitMerge,
|
||||
CheckCircle,
|
||||
RefreshCw,
|
||||
@@ -121,6 +122,12 @@ export function PRDetail({
|
||||
const [mergeReadiness, setMergeReadiness] = useState<MergeReadiness | null>(null);
|
||||
const mergeReadinessAbortRef = useRef<AbortController | null>(null);
|
||||
|
||||
// Branch update state (for updating PR branch when behind base)
|
||||
const [isUpdatingBranch, setIsUpdatingBranch] = useState(false);
|
||||
const [branchUpdateError, setBranchUpdateError] = useState<string | null>(null);
|
||||
const [branchUpdateSuccess, setBranchUpdateSuccess] = useState(false);
|
||||
const [mergeReadinessRefreshKey, setMergeReadinessRefreshKey] = useState(0);
|
||||
|
||||
// Workflows awaiting approval state (for fork PRs)
|
||||
const [workflowsAwaiting, setWorkflowsAwaiting] = useState<WorkflowsAwaitingApprovalResult | null>(null);
|
||||
const [isApprovingWorkflow, setIsApprovingWorkflow] = useState<number | null>(null);
|
||||
@@ -208,6 +215,14 @@ export function PRDetail({
|
||||
}
|
||||
}, [postSuccess]);
|
||||
|
||||
// Clear branch update success message after 3 seconds
|
||||
useEffect(() => {
|
||||
if (branchUpdateSuccess) {
|
||||
const timer = setTimeout(() => setBranchUpdateSuccess(false), 3000);
|
||||
return () => clearTimeout(timer);
|
||||
}
|
||||
}, [branchUpdateSuccess]);
|
||||
|
||||
// Auto-expand logs section when review starts
|
||||
useEffect(() => {
|
||||
if (isReviewing) {
|
||||
@@ -278,6 +293,10 @@ export function PRDetail({
|
||||
setBlockedStatusPosted(false);
|
||||
setBlockedStatusError(null);
|
||||
setIsPostingBlockedStatus(false);
|
||||
// Reset branch update state as well
|
||||
setBranchUpdateError(null);
|
||||
setBranchUpdateSuccess(false);
|
||||
setIsUpdatingBranch(false);
|
||||
}, [pr.number]);
|
||||
|
||||
// Check for workflows awaiting approval (fork PRs) when PR changes or review completes
|
||||
@@ -333,7 +352,7 @@ export function PRDetail({
|
||||
mergeReadinessAbortRef.current.abort();
|
||||
}
|
||||
};
|
||||
}, [pr.number, projectId]);
|
||||
}, [pr.number, projectId, mergeReadinessRefreshKey]);
|
||||
|
||||
// Handler to approve a workflow
|
||||
const handleApproveWorkflow = useCallback(async (runId: number) => {
|
||||
@@ -369,6 +388,40 @@ export function PRDetail({
|
||||
setWorkflowsAwaiting(result);
|
||||
}, [pr.number, workflowsAwaiting]);
|
||||
|
||||
// Handler to update PR branch when behind base
|
||||
const handleUpdateBranch = useCallback(async () => {
|
||||
// Capture current PR number to prevent state leaks across PR switches
|
||||
const currentPr = pr.number;
|
||||
|
||||
setIsUpdatingBranch(true);
|
||||
setBranchUpdateError(null);
|
||||
setBranchUpdateSuccess(false);
|
||||
|
||||
try {
|
||||
const result = await window.electronAPI.github.updatePRBranch(projectId, pr.number);
|
||||
|
||||
// Only update state if PR hasn't changed
|
||||
if (pr.number === currentPr) {
|
||||
if (result.success) {
|
||||
setBranchUpdateSuccess(true);
|
||||
// Trigger merge readiness refresh to update the UI
|
||||
setMergeReadinessRefreshKey(prev => prev + 1);
|
||||
} else {
|
||||
setBranchUpdateError(result.error || t('prReview.branchUpdateFailed'));
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
if (pr.number === currentPr) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
setBranchUpdateError(errorMessage);
|
||||
}
|
||||
} finally {
|
||||
if (pr.number === currentPr) {
|
||||
setIsUpdatingBranch(false);
|
||||
}
|
||||
}
|
||||
}, [pr.number, projectId, t]);
|
||||
|
||||
// Count selected findings by type for the button label
|
||||
const selectedCount = selectedFindingIds.size;
|
||||
|
||||
@@ -765,6 +818,29 @@ ${t('prReview.blockedStatusMessageFooter')}`;
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
{mergeReadiness.isBehind && (
|
||||
<div className="flex items-center gap-3 mt-3">
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
className="border-warning/50 text-warning hover:bg-warning/20"
|
||||
onClick={handleUpdateBranch}
|
||||
disabled={isUpdatingBranch}
|
||||
>
|
||||
{isUpdatingBranch ? (
|
||||
<>
|
||||
<Loader2 className="h-4 w-4 mr-2 animate-spin" />
|
||||
{t('prReview.updatingBranch')}
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<GitBranch className="h-4 w-4 mr-2" />
|
||||
{t('prReview.updateBranch')}
|
||||
</>
|
||||
)}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
<p className="text-xs text-warning/70 mt-2">
|
||||
{t('prReview.rerunReviewSuggestion', 'Consider re-running the review after resolving these issues.')}
|
||||
</p>
|
||||
@@ -774,6 +850,18 @@ ${t('prReview.blockedStatusMessageFooter')}`;
|
||||
</Card>
|
||||
)}
|
||||
|
||||
{branchUpdateSuccess && (
|
||||
<div className="flex items-center gap-2 text-xs text-success animate-in fade-in duration-200">
|
||||
<CheckCircle className="h-3 w-3" />
|
||||
{t('prReview.branchUpdated')}
|
||||
</div>
|
||||
)}
|
||||
{branchUpdateError && (
|
||||
<div className="text-xs text-destructive animate-in fade-in duration-200">
|
||||
{branchUpdateError}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Review Status & Actions */}
|
||||
<ReviewStatusTree
|
||||
status={prStatus.status}
|
||||
|
||||
@@ -209,6 +209,7 @@ const browserMockAPI: ElectronAPI = {
|
||||
deletePRReview: async () => true,
|
||||
checkNewCommits: async () => ({ hasNewCommits: false, newCommitCount: 0 }),
|
||||
checkMergeReadiness: async () => ({ isDraft: false, mergeable: 'UNKNOWN' as const, isBehind: false, ciStatus: 'none' as const, blockers: [] }),
|
||||
updatePRBranch: async () => ({ success: true }),
|
||||
runFollowupReview: () => {},
|
||||
getPRLogs: async () => null,
|
||||
getWorkflowsAwaitingApproval: async () => ({ awaiting_approval: 0, workflow_runs: [], can_approve: false }),
|
||||
|
||||
@@ -374,6 +374,7 @@ export const IPC_CHANNELS = {
|
||||
GITHUB_PR_FOLLOWUP_REVIEW: 'github:pr:followupReview',
|
||||
GITHUB_PR_CHECK_NEW_COMMITS: 'github:pr:checkNewCommits',
|
||||
GITHUB_PR_CHECK_MERGE_READINESS: 'github:pr:checkMergeReadiness',
|
||||
GITHUB_PR_UPDATE_BRANCH: 'github:pr:updateBranch',
|
||||
|
||||
// GitHub PR Review events (main -> renderer)
|
||||
GITHUB_PR_REVIEW_PROGRESS: 'github:pr:reviewProgress',
|
||||
|
||||
@@ -320,6 +320,10 @@
|
||||
"initial": "Initial",
|
||||
"rerunFollowup": "Re-run follow-up review",
|
||||
"rerunReview": "Re-run review",
|
||||
"updateBranch": "Update Branch",
|
||||
"updatingBranch": "Updating...",
|
||||
"branchUpdated": "Branch updated",
|
||||
"branchUpdateFailed": "Failed to update branch",
|
||||
"loadingMore": "Loading more PRs...",
|
||||
"scrollForMore": "Scroll for more",
|
||||
"allPRsLoaded": "All PRs loaded",
|
||||
|
||||
@@ -329,6 +329,10 @@
|
||||
"initial": "Initial",
|
||||
"rerunFollowup": "Relancer la revue de suivi",
|
||||
"rerunReview": "Relancer la revue",
|
||||
"updateBranch": "Mettre à jour la branche",
|
||||
"updatingBranch": "Mise à jour...",
|
||||
"branchUpdated": "Branche mise à jour",
|
||||
"branchUpdateFailed": "Échec de la mise à jour de la branche",
|
||||
"loadingMore": "Chargement des PRs...",
|
||||
"scrollForMore": "Défiler pour plus",
|
||||
"allPRsLoaded": "Tous les PRs chargés",
|
||||
|
||||
Reference in New Issue
Block a user