diff --git a/src/main/github/gh-utils.test.ts b/src/main/github/gh-utils.test.ts index 91cc8588b..41ce4f465 100644 --- a/src/main/github/gh-utils.test.ts +++ b/src/main/github/gh-utils.test.ts @@ -20,6 +20,7 @@ import { classifyListIssuesError, getIssueOwnerRepo, getOwnerRepo, + parseGitHubRemoteIdentity, parseGitHubOwnerRepo, resolveIssueSource } from './gh-utils' @@ -47,6 +48,20 @@ describe('github owner/repo resolution', () => { expect(parseGitHubOwnerRepo('git@example.com:stablyai/orca.git')).toBeNull() }) + it('parses GitHub Enterprise host identity', () => { + expect(parseGitHubRemoteIdentity('https://ghe.acme.internal/acme/orca.git')).toEqual({ + host: 'ghe.acme.internal', + owner: 'acme', + repo: 'orca' + }) + expect(parseGitHubRemoteIdentity('git@ghe.acme.internal:acme/orca.git')).toEqual({ + host: 'ghe.acme.internal', + owner: 'acme', + repo: 'orca' + }) + expect(parseGitHubOwnerRepo('https://ghe.acme.internal/acme/orca.git')).toBeNull() + }) + it('keeps getOwnerRepo origin-based', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'git@github.com:fork/orca.git\n' diff --git a/src/main/github/gh-utils.ts b/src/main/github/gh-utils.ts index 3d8b34788..bcff859ad 100644 --- a/src/main/github/gh-utils.ts +++ b/src/main/github/gh-utils.ts @@ -113,6 +113,8 @@ export function classifyListIssuesError(stderr: string): ClassifiedError { // short local name `OwnerRepo`. export type OwnerRepo = GitHubOwnerRepo +export type GitHubRemoteIdentity = GitHubOwnerRepo & { host: string } + export type GitHubRepoContext = { repoPath: string connectionId?: string | null @@ -143,11 +145,24 @@ export function _resetOwnerRepoCache(): void { } export function parseGitHubOwnerRepo(remoteUrl: string): OwnerRepo | null { - const match = remoteUrl.trim().match(/github\.com[:/]([^/]+)\/([^/]+?)(?:\.git)?$/) - if (!match) { + const identity = parseGitHubRemoteIdentity(remoteUrl) + if (!identity || identity.host.toLowerCase() !== 'github.com') { return null } - return { owner: match[1], repo: match[2] } + return { owner: identity.owner, repo: identity.repo } +} + +export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdentity | null { + const trimmed = remoteUrl.trim() + const httpsMatch = trimmed.match(/^https?:\/\/([^/]+)\/([^/]+)\/([^/]+?)(?:\.git)?\/?$/i) + if (httpsMatch) { + return { host: httpsMatch[1], owner: httpsMatch[2], repo: httpsMatch[3] } + } + const sshMatch = trimmed.match(/^git@([^:]+):([^/]+)\/([^/]+?)(?:\.git)?$/i) + if (sshMatch) { + return { host: sshMatch[1], owner: sshMatch[2], repo: sshMatch[3] } + } + return null } export async function getRemoteUrlForRepo( diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 7099e13c4..582a552a7 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -22,6 +22,11 @@ import { getConnectionId } from '@/lib/connection-context' import { CreatePullRequestDialog } from './CreatePullRequestDialog' import type { HostedReviewCreationEligibility } from '../../../../shared/hosted-review' import { toast } from 'sonner' +import { + classifyHostedReview, + type HostedReviewClassificationOptions +} from '../../../../shared/hosted-review-queue' +import { hostedReviewSummaryFromGitHubPRInfo } from '../../../../shared/hosted-review-github' export default function ChecksPanel(): React.JSX.Element { const activeWorktree = useActiveWorktree() @@ -579,6 +584,68 @@ export default function ChecksPanel(): React.JSX.Element { ] ) + const activeReviewClassification = React.useMemo(() => { + if (!pr || !repo) { + return null + } + let host = 'github.com' + let owner = 'unknown' + let repoName = 'unknown' + try { + const parsed = new URL(pr.url) + host = parsed.host || host + const segments = parsed.pathname.split('/').filter(Boolean) + if (segments.length >= 2) { + owner = segments[0] + repoName = segments[1] + } + } catch { + // Why: malformed URLs should not block queue-state classification. + } + + // Why: unresolved thread data is paginated and fetched separately. Until + // comments have loaded for this PR, do not let queue badges imply a clean review. + const commentsForClassification = + commentsFetchedAt !== undefined && !commentsLoading ? comments : undefined + const summary = hostedReviewSummaryFromGitHubPRInfo({ + pr, + owner, + repo: repoName, + host, + comments: commentsForClassification, + checks + }) + const options: HostedReviewClassificationOptions = { + agentAuthorLogins: [], + viewer: null + } + return classifyHostedReview(summary, options) + }, [pr, repo, comments, commentsFetchedAt, commentsLoading, checks]) + + const queueBadges = React.useMemo(() => { + if (!activeReviewClassification) { + return [] as string[] + } + const badges: string[] = [] + if (activeReviewClassification.needsResponse) { + badges.push('Needs response') + } + if (activeReviewClassification.readyToMerge) { + badges.push('Ready to merge') + } + if (activeReviewClassification.requested) { + badges.push('Review requested') + } + if (activeReviewClassification.state === 'mine') { + badges.push('My PR') + } else if (activeReviewClassification.state === 'agent') { + badges.push('AI PR') + } else { + badges.push('Teammate PR') + } + return badges + }, [activeReviewClassification]) + // ── Empty state ── if (!activeWorktree) { return ( @@ -763,6 +830,19 @@ export default function ChecksPanel(): React.JSX.Element { )} + {queueBadges.length > 0 && ( +
+ {queueBadges.map((badge) => ( + + {badge} + + ))} +
+ )} + {/* Merge / Delete Worktree actions */} {activeWorktree && repo && ( diff --git a/src/shared/hosted-review-github.test.ts b/src/shared/hosted-review-github.test.ts new file mode 100644 index 000000000..6ab4dac01 --- /dev/null +++ b/src/shared/hosted-review-github.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest' +import { hostedReviewSummaryFromGitHubPRInfo } from './hosted-review-github' +import type { PRInfo } from './types' + +const pr: PRInfo = { + number: 12, + title: 'Add queue badges', + state: 'open', + url: 'https://github.com/acme/orca/pull/12', + checksStatus: 'pending', + updatedAt: '2026-05-12T00:00:00.000Z', + mergeable: 'MERGEABLE', + headSha: 'abc123' +} + +describe('hostedReviewSummaryFromGitHubPRInfo', () => { + it('maps PRInfo into provider-neutral summary with host identity', () => { + const summary = hostedReviewSummaryFromGitHubPRInfo({ + pr, + owner: 'acme', + repo: 'orca', + host: 'github.acme.internal' + }) + + expect(summary.identity).toEqual({ + provider: 'github', + host: 'github.acme.internal', + owner: 'acme', + repo: 'orca', + number: 12 + }) + expect(summary.checksStatus).toBe('pending') + expect(summary.threadSummary).toBeUndefined() + }) + + it('derives unresolved thread count and failing status from enrichers', () => { + const summary = hostedReviewSummaryFromGitHubPRInfo({ + pr: { ...pr, checksStatus: 'success' }, + owner: 'acme', + repo: 'orca', + comments: [ + { + id: 1, + author: 'a', + authorAvatarUrl: '', + body: '', + createdAt: '', + url: '', + threadId: 't1', + isResolved: false + }, + { + id: 2, + author: 'b', + authorAvatarUrl: '', + body: '', + createdAt: '', + url: '', + threadId: 't1', + isResolved: false + }, + { + id: 3, + author: 'c', + authorAvatarUrl: '', + body: '', + createdAt: '', + url: '', + threadId: 't2', + isResolved: true + } + ], + checks: [{ name: 'ci', status: 'completed', conclusion: 'failure', url: null }] + }) + + expect(summary.threadSummary).toEqual({ unresolvedCount: 1, dataCompleteness: 'partial' }) + expect(summary.checksStatus).toBe('failure') + }) + + it('distinguishes loaded empty comments from unknown comments', () => { + expect( + hostedReviewSummaryFromGitHubPRInfo({ + pr, + owner: 'acme', + repo: 'orca' + }).threadSummary + ).toBeUndefined() + + expect( + hostedReviewSummaryFromGitHubPRInfo({ + pr, + owner: 'acme', + repo: 'orca', + comments: [] + }).threadSummary + ).toEqual({ unresolvedCount: 0, dataCompleteness: 'partial' }) + }) +}) diff --git a/src/shared/hosted-review-github.ts b/src/shared/hosted-review-github.ts new file mode 100644 index 000000000..ea733a59d --- /dev/null +++ b/src/shared/hosted-review-github.ts @@ -0,0 +1,88 @@ +import type { PRCheckDetail, PRComment, PRInfo } from './types' +import type { HostedReviewQueueSummary } from './hosted-review' + +export type HostedReviewFromGitHubPRInfoArgs = { + pr: PRInfo + owner: string + repo: string + host?: string + authorLogin?: string | null + authorIsBot?: boolean + requestedReviewerLogins?: string[] | null + comments?: PRComment[] + checks?: PRCheckDetail[] + lastViewedAt?: number +} + +function unresolvedThreadCount(comments?: PRComment[]): number | null { + if (comments === undefined) { + return null + } + const unresolved = new Set() + for (const comment of comments) { + if (!comment.threadId || comment.isResolved !== false) { + continue + } + unresolved.add(comment.threadId) + } + return unresolved.size +} + +function deriveChecksStatus( + prChecksStatus: PRInfo['checksStatus'], + checks?: PRCheckDetail[] +): PRInfo['checksStatus'] { + if (!checks || checks.length === 0) { + return prChecksStatus + } + const hasFailure = checks.some( + (check) => check.conclusion === 'failure' || check.conclusion === 'timed_out' + ) + if (hasFailure) { + return 'failure' + } + const hasPending = checks.some( + (check) => + check.status !== 'completed' || check.conclusion === null || check.conclusion === 'pending' + ) + if (hasPending) { + return 'pending' + } + const hasSuccess = checks.some((check) => check.conclusion === 'success') + if (hasSuccess) { + return 'success' + } + return 'neutral' +} + +export function hostedReviewSummaryFromGitHubPRInfo( + args: HostedReviewFromGitHubPRInfoArgs +): HostedReviewQueueSummary { + const unresolvedCount = unresolvedThreadCount(args.comments) + return { + identity: { + provider: 'github', + host: args.host ?? 'github.com', + owner: args.owner, + repo: args.repo, + number: args.pr.number + }, + title: args.pr.title, + url: args.pr.url, + state: args.pr.state, + author: args.authorLogin ? { login: args.authorLogin, isBot: args.authorIsBot } : null, + updatedAt: args.pr.updatedAt, + mergeable: args.pr.mergeable, + checksStatus: deriveChecksStatus(args.pr.checksStatus, args.checks), + threadSummary: + unresolvedCount === null + ? undefined + : { + unresolvedCount, + dataCompleteness: 'partial' + }, + requestedReviewerLogins: args.requestedReviewerLogins, + lastViewedAt: args.lastViewedAt, + draft: args.pr.state === 'draft' + } +} diff --git a/src/shared/hosted-review-queue.test.ts b/src/shared/hosted-review-queue.test.ts new file mode 100644 index 000000000..4e75f314a --- /dev/null +++ b/src/shared/hosted-review-queue.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from 'vitest' +import type { HostedReviewQueueSummary } from './hosted-review' +import { + classifyHostedReview, + hostedReviewIdentityKey, + reviewNeedsResponse, + reviewReadyToMerge +} from './hosted-review-queue' + +function baseSummary(overrides: Partial = {}): HostedReviewQueueSummary { + return { + identity: { provider: 'github', host: 'github.com', owner: 'acme', repo: 'orca', number: 42 }, + title: 'Improve checks panel', + url: 'https://github.com/acme/orca/pull/42', + state: 'open', + author: { login: 'teammate' }, + updatedAt: '2026-05-10T00:00:00.000Z', + mergeable: 'MERGEABLE', + checksStatus: 'success', + threadSummary: { unresolvedCount: 0 }, + ...overrides + } +} + +describe('hostedReviewIdentityKey', () => { + it('includes provider and host for enterprise-safe keys', () => { + const dotcom = hostedReviewIdentityKey({ + provider: 'github', + host: 'github.com', + owner: 'acme', + repo: 'orca', + number: 7 + }) + const ghe = hostedReviewIdentityKey({ + provider: 'github', + host: 'github.acme.internal', + owner: 'acme', + repo: 'orca', + number: 7 + }) + expect(dotcom).not.toBe(ghe) + }) +}) + +describe('classifyHostedReview', () => { + it('classifies mine/requested/agent/teammate', () => { + expect( + classifyHostedReview(baseSummary({ author: { login: 'me' } }), { + viewer: { login: 'me' } + }).state + ).toBe('mine') + + expect( + classifyHostedReview(baseSummary({ requestedReviewerLogins: ['me'] }), { + viewer: { login: 'me' } + }).state + ).toBe('requested') + + expect( + classifyHostedReview(baseSummary({ author: { login: 'orca-ci' } }), { + agentAuthorLogins: ['orca-ci'] + }).state + ).toBe('agent') + + expect(classifyHostedReview(baseSummary()).state).toBe('teammate') + }) +}) + +describe('reviewNeedsResponse', () => { + it('returns true for unresolved threads, failed checks, conflicts, and newer remote updates', () => { + expect(reviewNeedsResponse(baseSummary({ threadSummary: { unresolvedCount: 1 } }))).toBe(true) + expect(reviewNeedsResponse(baseSummary({ checksStatus: 'failure' }))).toBe(true) + expect(reviewNeedsResponse(baseSummary({ mergeable: 'CONFLICTING' }))).toBe(true) + expect( + reviewNeedsResponse( + baseSummary({ + updatedAt: '2026-05-11T00:00:00.000Z', + lastViewedAt: Date.parse('2026-05-10T00:00:00.000Z') + }) + ) + ).toBe(true) + }) + + it('does not mark needs-response from updatedAt alone when lastViewedAt is missing', () => { + expect(reviewNeedsResponse(baseSummary({ updatedAt: '2026-05-11T00:00:00.000Z' }))).toBe(false) + }) +}) + +describe('reviewReadyToMerge', () => { + it('rejects drafts, conflicts, failed/pending checks, unresolved threads, and unknown mergeability', () => { + expect(reviewReadyToMerge(baseSummary({ state: 'draft', draft: true }))).toBe(false) + expect(reviewReadyToMerge(baseSummary({ mergeable: 'CONFLICTING' }))).toBe(false) + expect(reviewReadyToMerge(baseSummary({ checksStatus: 'failure' }))).toBe(false) + expect(reviewReadyToMerge(baseSummary({ checksStatus: 'pending' }))).toBe(false) + expect(reviewReadyToMerge(baseSummary({ threadSummary: { unresolvedCount: 2 } }))).toBe(false) + expect(reviewReadyToMerge(baseSummary({ threadSummary: undefined }))).toBe(false) + expect(reviewReadyToMerge(baseSummary({ mergeable: 'UNKNOWN' }))).toBe(false) + }) + + it('accepts neutral checks when all other gates pass', () => { + expect(reviewReadyToMerge(baseSummary({ checksStatus: 'neutral' }))).toBe(true) + }) +}) diff --git a/src/shared/hosted-review-queue.ts b/src/shared/hosted-review-queue.ts new file mode 100644 index 000000000..ddd6a4212 --- /dev/null +++ b/src/shared/hosted-review-queue.ts @@ -0,0 +1,122 @@ +import type { + HostedReviewIdentity, + HostedReviewQueueClassification, + HostedReviewQueueState, + HostedReviewQueueSummary, + HostedReviewUser +} from './hosted-review' + +export type HostedReviewClassificationOptions = { + viewer?: HostedReviewUser | null + agentAuthorLogins?: string[] +} + +export function hostedReviewIdentityKey(identity: HostedReviewIdentity): string { + return `${identity.provider}::${identity.host.toLowerCase()}::${identity.owner.toLowerCase()}::${identity.repo.toLowerCase()}::${identity.number}` +} + +function hasRequestedReviewerSignal( + summary: HostedReviewQueueSummary, + viewer?: HostedReviewUser | null +): boolean { + if (!viewer?.login) { + return false + } + const requested = summary.requestedReviewerLogins + if (!requested || requested.length === 0) { + return false + } + const viewerLogin = viewer.login.toLowerCase() + return requested.some((login) => login.toLowerCase() === viewerLogin) +} + +function isAgentAuthored( + summary: HostedReviewQueueSummary, + options?: HostedReviewClassificationOptions +): boolean { + if (summary.author?.isBot) { + return true + } + const author = summary.author?.login?.toLowerCase() + if (!author) { + return false + } + if (options?.agentAuthorLogins?.some((login) => login.toLowerCase() === author)) { + return true + } + return author.endsWith('[bot]') || author.includes('bot') +} + +function getQueueState( + summary: HostedReviewQueueSummary, + options?: HostedReviewClassificationOptions +): HostedReviewQueueState { + const viewerLogin = options?.viewer?.login?.toLowerCase() ?? null + const authorLogin = summary.author?.login?.toLowerCase() ?? null + if (viewerLogin && authorLogin && viewerLogin === authorLogin) { + return 'mine' + } + if (hasRequestedReviewerSignal(summary, options?.viewer)) { + return 'requested' + } + if (isAgentAuthored(summary, options)) { + return 'agent' + } + return 'teammate' +} + +export function reviewNeedsResponse( + summary: HostedReviewQueueSummary, + viewer?: HostedReviewUser | null +): boolean { + void viewer + if (summary.state !== 'open' && summary.state !== 'draft') { + return false + } + if ((summary.threadSummary?.unresolvedCount ?? 0) > 0) { + return true + } + if (summary.checksStatus === 'failure') { + return true + } + if (summary.mergeable === 'CONFLICTING') { + return true + } + if (summary.lastViewedAt === undefined) { + return false + } + const updatedAt = Date.parse(summary.updatedAt) + return Number.isFinite(updatedAt) && updatedAt > summary.lastViewedAt +} + +export function reviewReadyToMerge(summary: HostedReviewQueueSummary): boolean { + if (summary.state !== 'open') { + return false + } + if (summary.draft) { + return false + } + if (summary.mergeable !== 'MERGEABLE') { + return false + } + if (summary.checksStatus !== 'success' && summary.checksStatus !== 'neutral') { + return false + } + if (summary.threadSummary?.unresolvedCount !== 0) { + return false + } + return true +} + +export function classifyHostedReview( + summary: HostedReviewQueueSummary, + options?: HostedReviewClassificationOptions +): HostedReviewQueueClassification { + const state = getQueueState(summary, options) + return { + state, + requested: hasRequestedReviewerSignal(summary, options?.viewer), + needsResponse: reviewNeedsResponse(summary, options?.viewer), + readyToMerge: reviewReadyToMerge(summary) + } +} diff --git a/src/shared/hosted-review.ts b/src/shared/hosted-review.ts index c26d6a0b1..4ebd122c4 100644 --- a/src/shared/hosted-review.ts +++ b/src/shared/hosted-review.ts @@ -113,3 +113,56 @@ export type HostedReviewCreationEligibilityArgs = { linkedBitbucketPR?: number | null linkedGiteaPR?: number | null } + +export type HostedReviewIdentity = { + provider: HostedReviewProvider + host: string + owner: string + repo: string + number: number +} + +export type HostedReviewUser = { + login: string | null + isBot?: boolean +} + +export type HostedReviewDecision = 'approved' | 'changes_requested' | 'review_required' | null + +export type HostedReviewThreadSummary = { + unresolvedCount: number | null + dataCompleteness?: 'full' | 'partial' +} + +export type HostedReviewQueueSummary = { + identity: HostedReviewIdentity + title: string + url: string + state: HostedReviewState + author: HostedReviewUser | null + updatedAt: string + lastViewedAt?: number + mergeable: PRMergeableState + checksStatus: CheckStatus + reviewDecision?: HostedReviewDecision + threadSummary?: HostedReviewThreadSummary + requestedReviewerLogins?: string[] | null + draft?: boolean +} + +export type HostedReviewQueueKey = + | 'mine' + | 'requested' + | 'agent' + | 'teammate' + | 'needs-response' + | 'ready-to-merge' + +export type HostedReviewQueueState = 'mine' | 'requested' | 'agent' | 'teammate' + +export type HostedReviewQueueClassification = { + state: HostedReviewQueueState + needsResponse: boolean + readyToMerge: boolean + requested: boolean +}