Skip to content

Fix PR List Update on Post Status Click #1207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AndyMik90 merged 14 commits into from
Jan 17, 2026
Merged

Fix PR List Update on Post Status Click #1207

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2920878
auto-claude: subtask-1-1 - Add markReviewPosted function to useGitHub…
Jan 16, 2026
d25fd4d
Merge branch 'develop' into auto-claude/125-fix-pr-list-update-on-pos…
AndyMik90 Jan 16, 2026
a409b92
Merge branch 'develop' into auto-claude/125-fix-pr-list-update-on-pos…
AndyMik90 Jan 16, 2026
e29564b
fix: persist hasPostedFindings flag to disk in markReviewPosted
AndyMik90 Jan 16, 2026
8bf032b
fix: address follow-up review findings for markReviewPosted
AndyMik90 Jan 16, 2026
6c07a7c
fix: update PostCommentFn type in PRDetail tests to match new signature
AndyMik90 Jan 16, 2026
c669b5d
Merge branch 'develop' into auto-claude/125-fix-pr-list-update-on-pos…
AndyMik90 Jan 16, 2026
1dd1649
Merge branch 'develop' into auto-claude/125-fix-pr-list-update-on-pos…
AndyMik90 Jan 17, 2026
3cd6ea2
Merge branch 'develop' into auto-claude/125-fix-pr-list-update-on-pos…
AndyMik90 Jan 17, 2026
82218af
fix: correct mock return value type in PRDetail integration test
AndyMik90 Jan 17, 2026
0c91a5d
fix(security): eliminate TOCTOU race condition in markReviewPosted ha…
AndyMik90 Jan 17, 2026
9f6239a
chore: merge develop and fix additional test type error
Jan 17, 2026
e807cc9
chore: merge develop and resolve conflicts
Jan 17, 2026
04c5f0c
Merge branch 'develop' into auto-claude/125-fix-pr-list-update-on-pos…
AndyMik90 Jan 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
postComment,
mergePR,
assignPR,
markReviewPosted,
refresh,
loadMore,
isConnected,
Expand Down Expand Up @@ -173,6 +174,12 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
return null;
}, [selectedProjectId, selectedPRNumber]);

const handleMarkReviewPosted = useCallback(async () => {
if (selectedPRNumber) {
await markReviewPosted(selectedPRNumber);
}
}, [selectedPRNumber, markReviewPosted]);
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This handler can be simplified to be synchronous. The underlying markReviewPosted function performs a synchronous state update, so there's no need for async/await here. This change improves clarity by accurately reflecting the synchronous nature of the operation.

Suggested change
const handleMarkReviewPosted = useCallback(async () => {
if (selectedPRNumber) {
await markReviewPosted(selectedPRNumber);
}
}, [selectedPRNumber, markReviewPosted]);
const handleMarkReviewPosted = useCallback(() => {
if (selectedPRNumber) {
markReviewPosted(selectedPRNumber);
}
}, [selectedPRNumber, markReviewPosted]);

This comment was marked as outdated.

Sign in to view


// Not connected state
if (!isConnected) {
return <NotConnectedState error={error} onOpenSettings={onOpenSettings} t={t} />;
Expand Down Expand Up @@ -259,6 +266,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
onMergePR={handleMergePR}
onAssignPR={handleAssignPR}
onGetLogs={handleGetLogs}
onMarkReviewPosted={handleMarkReviewPosted}
/>
) : (
<EmptyState message={t("prReview.selectPRToView")} />
Expand Down
4 changes: 4 additions & 0 deletions apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ interface PRDetailProps {
onMergePR: (mergeMethod?: 'merge' | 'squash' | 'rebase') => void;
onAssignPR: (username: string) => void;
onGetLogs: () => Promise<PRLogsType | null>;
onMarkReviewPosted?: () => Promise<void>;
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To align with making markReviewPosted a synchronous function, this prop type should be updated to not return a Promise. The operation is a synchronous state update.

Suggested change
onMarkReviewPosted?: () => Promise<void>;
onMarkReviewPosted?: () => void;

}

function getStatusColor(status: PRReviewResult['overallStatus']): string {
Expand Down Expand Up @@ -88,6 +89,7 @@ export function PRDetail({
onMergePR,
onAssignPR: _onAssignPR,
onGetLogs,
onMarkReviewPosted,
}: PRDetailProps) {
const { t } = useTranslation('common');
// Selection state for findings
Expand Down Expand Up @@ -715,6 +717,8 @@ ${t('prReview.blockedStatusMessageFooter')}`;
if (pr.number === currentPr) {
setBlockedStatusPosted(true);
setBlockedStatusError(null);
// Update the store to mark review as posted so PR list reflects the change
await onMarkReviewPosted?.();
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since onMarkReviewPosted should be a synchronous function that doesn't return a promise, the await keyword is unnecessary and should be removed.

Suggested change
await onMarkReviewPosted?.();
onMarkReviewPosted?.();

}
} catch (err) {
console.error('Failed to post blocked status comment:', err);
Expand Down
19 changes: 19 additions & 0 deletions apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ interface UseGitHubPRsResult {
postComment: (prNumber: number, body: string) => Promise<boolean>;
mergePR: (prNumber: number, mergeMethod?: "merge" | "squash" | "rebase") => Promise<boolean>;
assignPR: (prNumber: number, username: string) => Promise<boolean>;
markReviewPosted: (prNumber: number) => Promise<void>;
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The markReviewPosted function performs a synchronous state update. To make the code clearer and avoid unnecessary asynchronicity, its type should be (prNumber: number) => void; instead of returning a Promise.

Suggested change
markReviewPosted: (prNumber: number) => Promise<void>;
markReviewPosted: (prNumber: number) => void;

getReviewStateForPR: (prNumber: number) => {
isReviewing: boolean;
startedAt: string | null;
Expand Down Expand Up @@ -498,6 +499,23 @@ export function useGitHubPRs(
[projectId, fetchPRs]
);

const markReviewPosted = useCallback(
async (prNumber: number): Promise<void> => {
if (!projectId) return;

const existingState = getPRReviewState(projectId, prNumber);
if (existingState?.result) {
// Update the store with hasPostedFindings: true
usePRReviewStore.getState().setPRReviewResult(
projectId,
{ ...existingState.result, hasPostedFindings: true },
{ preserveNewCommitsCheck: true }
);
}
},
[projectId, getPRReviewState]
);
Comment on lines 561 to 594
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This function is marked as async and returns a Promise, but its body only contains synchronous code (updating the Zustand store). This introduces unnecessary asynchronicity. The function should be made synchronous to more accurately reflect its behavior. This also requires updating its type definition and all call sites to remove async/await.

Suggested change
const markReviewPosted = useCallback(
async (prNumber: number): Promise<void> => {
if (!projectId) return;
const existingState = getPRReviewState(projectId, prNumber);
if (existingState?.result) {
// Update the store with hasPostedFindings: true
usePRReviewStore.getState().setPRReviewResult(
projectId,
{ ...existingState.result, hasPostedFindings: true },
{ preserveNewCommitsCheck: true }
);
}
},
[projectId, getPRReviewState]
);
const markReviewPosted = useCallback(
(prNumber: number): void => {
if (!projectId) return;
const existingState = getPRReviewState(projectId, prNumber);
if (existingState?.result) {
// Update the store with hasPostedFindings: true
usePRReviewStore.getState().setPRReviewResult(
projectId,
{ ...existingState.result, hasPostedFindings: true },
{ preserveNewCommitsCheck: true }
);
}
},
[projectId, getPRReviewState]
);


return {
prs,
isLoading,
Expand Down Expand Up @@ -526,6 +544,7 @@ export function useGitHubPRs(
postComment,
mergePR,
assignPR,
markReviewPosted,
getReviewStateForPR,
};
}
Loading