From 1c1677eaadd2fb23e67dd1bb200089e5b5c32b23 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 30 May 2026 01:52:07 -0700 Subject: [PATCH] Hydrate task PR merge methods Co-authored-by: Orca --- src/main/github/client-work-items.test.ts | 64 ++++++++++++++++++- src/main/github/client.ts | 32 ++++++++-- .../right-sidebar/HostedReviewActions.tsx | 3 +- 3 files changed, 89 insertions(+), 10 deletions(-) diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index e8e3dcba802..86138028a5f 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -58,7 +58,12 @@ vi.mock('./rate-limit', () => ({ noteRateLimitSpend: noteRateLimitSpendMock })) -import { countWorkItems, listWorkItems, _resetOwnerRepoCache } from './client' +import { + countWorkItems, + listWorkItems, + _resetMergeQueueCacheForTests, + _resetOwnerRepoCache +} from './client' describe('listWorkItems', () => { beforeEach(() => { @@ -84,6 +89,7 @@ describe('listWorkItems', () => { })) getOwnerRepoForRemoteMock.mockResolvedValue(null) _resetOwnerRepoCache() + _resetMergeQueueCacheForTests() }) it('runs both issue and PR GitHub searches for a mixed query and merges the results by recency', async () => { @@ -221,6 +227,58 @@ describe('listWorkItems', () => { ]) }) + it('hydrates PR list rows with repository merge method settings', async () => { + getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 42, + title: 'Add feature', + state: 'OPEN', + url: 'https://github.com/acme/widgets/pull/42', + labels: [], + updatedAt: '2026-03-28T00:00:00Z', + author: { login: 'octocat' }, + isDraft: false, + headRefName: 'feature/add-feature', + headRefOid: 'head-42', + baseRefName: 'main' + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + data: { + repository: { + viewerDefaultMergeMethod: 'REBASE', + mergeCommitAllowed: false, + rebaseMergeAllowed: true, + squashMergeAllowed: true + } + } + }) + }) + + const { items } = await listWorkItems('/repo-root', 10, 'is:pr') + + expect(items).toHaveLength(1) + expect(items[0]?.mergeMethodSettings).toEqual({ + defaultMethod: 'rebase', + allowedMethods: { + squash: true, + merge: false, + rebase: true + } + }) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 2, + expect.arrayContaining(['api', 'graphql', '-f', 'owner=acme', '-f', 'repo=widgets']), + { cwd: '/repo-root' } + ) + }) + it('routes draft queries to PR search only', async () => { getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) @@ -242,7 +300,7 @@ describe('listWorkItems', () => { ]) }) const { items } = await listWorkItems('/repo-root', 10, 'is:pr is:draft') - expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) expect(ghExecFileAsyncMock).toHaveBeenCalledWith( [ 'pr', @@ -301,7 +359,7 @@ describe('listWorkItems', () => { const { items } = await listWorkItems('/repo-root', 10, 'is:merged') - expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) expect(ghExecFileAsyncMock).toHaveBeenCalledWith( [ 'pr', diff --git a/src/main/github/client.ts b/src/main/github/client.ts index f4be72e4529..dd1288475c2 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -361,9 +361,9 @@ const WORK_ITEM_PR_LIST_JSON_FIELDS = 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests' // Why: these fields are intentionally excluded from `gh pr list` because -// statusCheckRollup/review decision/merge metadata fan out into expensive -// GraphQL work across every row. Requested reviewers are kept in the list -// payload because the Tasks table renders that column on first paint. +// statusCheckRollup/review decision/PR-specific merge metadata fan out into +// expensive GraphQL work across every row. Requested reviewers are kept in the +// list payload because the Tasks table renders that column on first paint. const WORK_ITEM_PR_DETAIL_JSON_FIELDS = 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,additions,deletions,changedFiles,reviewDecision,reviewRequests,latestReviews,assignees,statusCheckRollup,mergeable,mergeStateStatus,autoMergeRequest,maintainerCanModify' @@ -694,6 +694,26 @@ function mapPullRequestWorkItem( } } +async function hydrateWorkItemMergeMethodSettings( + items: MainWorkItem[], + ownerRepo: OwnerRepo | null, + ghOptions: GhExecOptions +): Promise { + const hasPullRequest = items.some((item) => item.type === 'pr') + if (!ownerRepo || !hasPullRequest) { + return items + } + // Why: merge method settings are repository-level, so one cached metadata + // probe can keep Tasks rows accurate without per-PR GraphQL fan-out. + const mergeMetadata = await detectRepositoryMergeMetadata(ownerRepo, undefined, ghOptions) + if (!mergeMetadata.mergeMethodSettings) { + return items + } + return items.map((item) => + item.type === 'pr' ? { ...item, mergeMethodSettings: mergeMetadata.mergeMethodSettings } : item + ) +} + async function fetchIssueWorkItem( repoPath: string, ownerRepo: OwnerRepo | null, @@ -947,6 +967,7 @@ async function listRecentWorkItems( prs = (JSON.parse(prsSettled.value.stdout) as Record[]).map((item) => mapPullRequestWorkItem(item, prOwnerRepo) ) + prs = await hydrateWorkItemMergeMethodSettings(prs, prOwnerRepo, ghOptions) } else { // Why: PR-side failures must preserve the pre-diff behavior of // Promise.all by re-throwing so the rejection propagates up through @@ -1088,10 +1109,11 @@ async function listQueriedWorkItems( const mapped = (JSON.parse(stdout) as Record[]).map((item) => mapPullRequestWorkItem(item, prOwnerRepo) ) + const hydrated = await hydrateWorkItemMergeMethodSettings(mapped, prOwnerRepo, ghOptions) if (query.state === 'closed') { - return mapped.filter((item) => item.state !== 'merged') + return hydrated.filter((item) => item.state !== 'merged') } - return mapped + return hydrated } catch (err) { console.warn('listQueriedWorkItems PRs partial failure:', err) return [] diff --git a/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx b/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx index 00e1aafa2e3..96cd7905944 100644 --- a/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx +++ b/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx @@ -136,8 +136,7 @@ export default function HostedReviewActions({ }) }, [githubPR, isGitLab, review]) const mergeMethods = useMemo( - () => - resolveGitHubPRMergeMethods(isGitLab ? null : (githubPR?.mergeMethodSettings ?? null)), + () => resolveGitHubPRMergeMethods(isGitLab ? null : (githubPR?.mergeMethodSettings ?? null)), [githubPR?.mergeMethodSettings, isGitLab] ) const isUpdatingReviewState = stateUpdating !== null