feat(checks): open pr comment file:line locations in the editor (#32031)

This commit is contained in:
Kelvin Amoaba
2026-10-22 21:48:04 +00:00
committed by GitHub
parent 1ed13cf687
commit 03e7ce0833
10 changed files with 153 additions and 80 deletions
@@ -2,11 +2,7 @@ import React from 'react'
import { ExternalLink } from 'lucide-react'
import { translate } from '@/i18n/i18n'
import type { PRCheckAnnotation } from '../../../../shared/github/check-types'
import {
cancelAnnotationRevealFrame,
getOpenableAnnotationLine,
openAnnotationLocation
} from './check-annotation-open'
import { getOpenableAnnotationLine, openAnnotationLocation } from './check-annotation-open'
import { formatAnnotationsForClipboard } from './check-run-clipboard-text'
import { CheckRunCopyButton } from './CheckRunCopyButton'
@@ -17,21 +13,12 @@ export function CheckRunAnnotations({
annotations: PRCheckAnnotation[]
worktreeId: string | null
}): React.JSX.Element {
const revealRafRef = React.useRef<number | null>(null)
const revealInnerRafRef = React.useRef<number | null>(null)
React.useEffect(
() => () => {
cancelAnnotationRevealFrame(revealRafRef)
cancelAnnotationRevealFrame(revealInnerRafRef)
},
[]
)
const openAnnotation = React.useCallback(
(path: string, line: number) => {
if (!worktreeId) {
return
}
openAnnotationLocation({ worktreeId, path, line, revealRafRef, revealInnerRafRef })
openAnnotationLocation({ worktreeId, path, line })
},
[worktreeId]
)
@@ -3,7 +3,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
activateAndRevealWorktree: vi.fn(),
state: {
worktreesByRepo: { 'repo-1': [{ id: 'wt-1', repoId: 'repo-1', path: '/repo/wt' }] },
getKnownWorktreeById: vi.fn((id: string) =>
id === 'wt-1' ? { id: 'wt-1', repoId: 'repo-1', path: '/repo/wt' } : undefined
),
openFile: vi.fn(),
setPendingEditorReveal: vi.fn()
}
@@ -23,22 +25,24 @@ afterEach(() => {
})
describe('openAnnotationLocation', () => {
it('activates as a surface-providing caller before opening the editor', () => {
it('activates the given host as a surface-providing caller before opening the editor', () => {
vi.stubGlobal('requestAnimationFrame', vi.fn().mockReturnValue(1))
vi.stubGlobal('cancelAnimationFrame', vi.fn())
openAnnotationLocation({
worktreeId: 'wt-1',
executionHostId: 'ssh:devbox',
path: 'src/a.ts',
line: 3,
revealRafRef: { current: null },
revealInnerRafRef: { current: null }
line: 3
})
// Why: the annotation's editor file is the surface — the jump must not re-seed
// a shell into a workspace whose last terminal the user closed.
// a shell into a workspace whose last terminal the user closed. The host keeps a
// remote workspace from switching to its local twin with the same id.
expect(mocks.state.getKnownWorktreeById).toHaveBeenCalledWith('wt-1', 'ssh:devbox')
expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', {
providesInitialSurface: true
providesInitialSurface: true,
executionHostId: 'ssh:devbox'
})
expect(mocks.state.openFile).toHaveBeenCalledWith(
expect.objectContaining({ worktreeId: 'wt-1', relativePath: 'src/a.ts' }),
@@ -50,9 +54,7 @@ describe('openAnnotationLocation', () => {
openAnnotationLocation({
worktreeId: 'missing',
path: 'src/a.ts',
line: 3,
revealRafRef: { current: null },
revealInnerRafRef: { current: null }
line: 3
})
expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled()
@@ -1,12 +1,12 @@
import { detectLanguage } from '@/lib/language-detect'
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
import { useAppStore } from '@/store'
import { findWorktreeById } from '@/store/slices/worktree-helpers'
import { scheduleEditorLineReveal } from '@/store/slices/editor/focus/editor-focus-reveal'
import {
getOpenableAnnotationLine,
resolveAnnotationPathInsideWorktree
} from './check-annotation-path'
import { selectWorktreesByRepo } from '@/store/workspace-record-selectors'
import type { ExecutionHostId } from '../../../../shared/execution-host'
export { getOpenableAnnotationLine }
@@ -14,12 +14,11 @@ export function openAnnotationLocation(params: {
worktreeId: string
path: string
line: number
revealRafRef: React.RefObject<number | null>
revealInnerRafRef: React.RefObject<number | null>
executionHostId?: ExecutionHostId
}): void {
const { worktreeId, path, line, revealRafRef, revealInnerRafRef } = params
const { worktreeId, path, line, executionHostId } = params
const store = useAppStore.getState()
const worktree = findWorktreeById(selectWorktreesByRepo(store), worktreeId)
const worktree = store.getKnownWorktreeById(worktreeId, executionHostId)
if (!worktree) {
return
}
@@ -28,41 +27,18 @@ export function openAnnotationLocation(params: {
return
}
const { absolutePath, relativePath } = resolvedPath
const language = detectLanguage(relativePath)
// Why: reuse the shared activation path so an annotation jump lands in the
// same history stack as sidebar, palette, and terminal-link navigation.
activateAndRevealWorktree(worktreeId, { providesInitialSurface: true })
activateAndRevealWorktree(worktreeId, { providesInitialSurface: true, executionHostId })
store.openFile(
{
filePath: absolutePath,
relativePath,
worktreeId,
language: detectLanguage(relativePath),
mode: 'edit'
},
const fileId = store.openFile(
{ filePath: absolutePath, relativePath, worktreeId, language, mode: 'edit' },
{ forceContentReload: true }
)
cancelAnnotationRevealFrame(revealRafRef)
cancelAnnotationRevealFrame(revealInnerRafRef)
store.setPendingEditorReveal(null)
// Why: opening can replace the active tab and mount Monaco asynchronously.
// Matching search and terminal-link navigation, wait two frames so the
// destination editor owns layout before we ask it to reveal the line.
revealRafRef.current = requestAnimationFrame(() => {
revealInnerRafRef.current = requestAnimationFrame(() => {
store.setPendingEditorReveal({ filePath: absolutePath, line, column: 1, matchLength: 0 })
cancelAnnotationRevealFrame(revealRafRef)
cancelAnnotationRevealFrame(revealInnerRafRef)
})
})
}
export function cancelAnnotationRevealFrame(frameRef: React.RefObject<number | null>): void {
if (frameRef.current !== null) {
cancelAnimationFrame(frameRef.current)
frameRef.current = null
if (language === 'markdown') {
store.setMarkdownViewMode(fileId, 'source')
}
scheduleEditorLineReveal(useAppStore.getState, absolutePath, line, 1, fileId)
}
@@ -17,6 +17,7 @@ import { PRTriageStrip } from './triage-strip'
import { ConflictingFilesSection, MergeConflictNotice } from './conflict-summary'
import { ChecksList } from './checks-list'
import { PRCommentsList } from './comments-list'
import { openAnnotationLocation } from '@/components/editor/check-annotation-open'
import { translate } from '@/i18n/i18n'
import type { ChecksPanelActiveContentModel } from './active-content-props'
import { ChecksPanelReviewHeader } from './review-header'
@@ -259,6 +260,17 @@ export function ChecksPanelActiveContent({
onEditComment={pr ? handleEditComment : undefined}
onDeleteComment={pr ? handleDeleteComment : undefined}
onSetReaction={canTargetPRComments ? handleSetReaction : undefined}
onOpenLocation={
activeWorktree
? (path, line) =>
openAnnotationLocation({
worktreeId: activeWorktree.id,
executionHostId: activeWorktree.hostId,
path,
line
})
: undefined
}
/>
<SourceControlAgentActionDialog
open={sourceControlAiActionsVisible && agentComposerState !== null}
@@ -18,6 +18,7 @@ import {
DropdownMenuTrigger
} from '@/components/ui/dropdown-menu'
import { cn } from '@/lib/utils'
import { basename } from '@/lib/path'
import { isBotPRComment } from '../../../../../shared/pr-comment-audience'
import { setPRBotAuthorOverride } from '@/lib/pr-bot-author-overrides'
import { normalizePRCommentAuthorLogin } from '../../../../../shared/pr-bot-author-overrides'
@@ -156,6 +157,46 @@ export function formatLineRange(comment: PRComment): string | null {
return `L${comment.line}`
}
export function CommentLocationLabel({
comment,
className,
onOpenLocation
}: {
comment: PRComment
className: string
onOpenLocation?: (path: string, line: number) => void
}): React.JSX.Element | null {
const { path, line } = comment
if (!path) {
return null
}
const lineRange = formatLineRange(comment)
const label = `${basename(path)}${lineRange ? `:${lineRange}` : ''}`
if (!onOpenLocation || !line) {
return (
<span className={className} title={path}>
{label}
</span>
)
}
return (
<button
type="button"
className={cn(
className,
'cursor-pointer rounded text-left hover:text-foreground hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring/50'
)}
title={path}
onClick={(event) => {
event.stopPropagation()
onOpenLocation(path, comment.startLine ?? line)
}}
>
{label}
</button>
)
}
/** True for top-level PR conversation comments the viewer can edit or delete. */
export function isMutablePRConversationComment(comment: PRComment): boolean {
if (comment.threadId || comment.path) {
@@ -45,7 +45,8 @@ export function PRCommentGroupView({
onEditComment,
onDeleteComment,
onSetReaction,
onQueueForAgent
onQueueForAgent,
onOpenLocation
}: {
group: PRCommentGroup
botAuthorOverrides: ReadonlySet<string>
@@ -69,6 +70,7 @@ export function PRCommentGroupView({
reacted: boolean
) => Promise<boolean>
onQueueForAgent?: () => void
onOpenLocation?: (path: string, line: number) => void
}): React.JSX.Element {
// Reply targets a specific comment id so any comment in a thread — root or
// nested reply — can be replied to, not just the thread root.
@@ -116,7 +118,8 @@ export function PRCommentGroupView({
onEditComment,
onDeleteComment,
onSetReaction,
onQueueForAgent
onQueueForAgent,
onOpenLocation
}
const content =
@@ -201,7 +204,8 @@ export function ResolvedCommentGroupsSection({
onReply,
onEditComment,
onDeleteComment,
onSetReaction
onSetReaction,
onOpenLocation
}: {
groups: PRCommentGroup[]
botAuthorOverrides: ReadonlySet<string>
@@ -221,6 +225,7 @@ export function ResolvedCommentGroupsSection({
content: GitHubReactionContent,
reacted: boolean
) => Promise<boolean>
onOpenLocation?: (path: string, line: number) => void
}): React.JSX.Element | null {
if (groups.length === 0) {
return null
@@ -258,6 +263,7 @@ export function ResolvedCommentGroupsSection({
onEditComment={onEditComment}
onDeleteComment={onDeleteComment}
onSetReaction={onSetReaction}
onOpenLocation={onOpenLocation}
/>
))}
</AccordionContent>
@@ -0,0 +1,42 @@
// @vitest-environment happy-dom
import { cleanup, render, screen } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { getPRCommentPresentationClasses } from '../pr-comment-presentation'
import { CommentRow } from './comment-row'
afterEach(cleanup)
describe('CommentRow location', () => {
it('opens the full repo path at the first line of the commented range', () => {
const onOpenLocation = vi.fn()
render(
<CommentRow
comment={{
id: 1,
author: 'reviewer',
authorAvatarUrl: '',
body: 'findProjectRoot treats any folder as an app',
createdAt: '2026-10-21T00:00:00Z',
url: 'https://github.com/o/r/pull/1#discussion_r1',
path: 'src/cli/doctor.ts',
threadId: 'thread-1',
startLine: 370,
line: 380
}}
botAuthorOverrides={new Set()}
isReply={false}
showResolve={false}
actionState="open"
isQueued={false}
presentation={getPRCommentPresentationClasses()}
now={Date.parse('2026-10-22T00:00:00Z')}
onOpenLocation={onOpenLocation}
/>
)
screen.getByRole('button', { name: 'doctor.ts:L370-L380' }).click()
expect(onOpenLocation).toHaveBeenCalledWith('src/cli/doctor.ts', 370)
})
})
@@ -11,13 +11,13 @@ import { formatPrCommentRelativeTime } from '../../../../../shared/pr-comment-ti
import { translate } from '@/i18n/i18n'
import {
buildCopyText,
CommentLocationLabel,
CommentMoreMenu,
CopyButton,
isMutablePRConversationComment,
PRCommentActionBadge,
QueueForAgentButton,
ResolveButton,
formatLineRange
ResolveButton
} from './comment-controls'
/** A single comment row — used for both root and reply comments. */
@@ -39,7 +39,8 @@ export function CommentRow({
onEditComment,
onDeleteComment,
onSetReaction,
onQueueForAgent
onQueueForAgent,
onOpenLocation
}: {
comment: PRComment
botAuthorOverrides: ReadonlySet<string>
@@ -63,6 +64,7 @@ export function CommentRow({
reacted: boolean
) => Promise<boolean>
onQueueForAgent?: () => void
onOpenLocation?: (path: string, line: number) => void
}): React.JSX.Element {
const automated = isBotPRComment(comment, botAuthorOverrides)
const canMutateComment = isMutablePRConversationComment(comment)
@@ -199,12 +201,11 @@ export function CommentRow({
{translate('auto.components.right.sidebar.checks.panel.content.2ba0a32bdd', 'bot')}
</span>
) : null}
{comment.path ? (
<span className={presentation.pathBadge} title={comment.path}>
{comment.path.split('/').pop()}
{formatLineRange(comment) && `:${formatLineRange(comment)}`}
</span>
) : null}
<CommentLocationLabel
comment={comment}
className={presentation.pathBadge}
onOpenLocation={onOpenLocation}
/>
<PRCommentActionBadge
actionState={actionState}
isQueued={isQueued}
@@ -245,11 +246,12 @@ export function CommentRow({
{translate('auto.components.right.sidebar.checks.panel.content.2ba0a32bdd', 'bot')}
</span>
)}
{!isReply && comment.path && (
<span className={presentation.pathBadge}>
{comment.path.split('/').pop()}
{formatLineRange(comment) && `:${formatLineRange(comment)}`}
</span>
{!isReply && (
<CommentLocationLabel
comment={comment}
className={presentation.pathBadge}
onOpenLocation={onOpenLocation}
/>
)}
{!isReply ? (
<PRCommentActionBadge
@@ -49,7 +49,8 @@ export function PRCommentsList(props: PRCommentsListProps): React.JSX.Element {
onResolve,
onEditComment,
onDeleteComment,
onSetReaction
onSetReaction,
onOpenLocation
} = props
const {
presentation,
@@ -371,6 +372,7 @@ export function PRCommentsList(props: PRCommentsListProps): React.JSX.Element {
onEditComment={onEditComment}
onDeleteComment={onDeleteComment}
onSetReaction={onSetReaction}
onOpenLocation={onOpenLocation}
/>
</>
)}
@@ -64,6 +64,7 @@ export type PRCommentsListProps = {
content: GitHubReactionContent,
reacted: boolean
) => Promise<boolean>
onOpenLocation?: (path: string, line: number) => void
}
export function useCommentsListState({
@@ -78,7 +79,8 @@ export function useCommentsListState({
onEditComment,
onDeleteComment,
onSetReaction,
onResolveSelectedCommentsWithAI
onResolveSelectedCommentsWithAI,
onOpenLocation
}: PRCommentsListProps) {
function findVerticalScrollParent(element: HTMLElement): HTMLElement | null {
let parent = element.parentElement
@@ -250,6 +252,7 @@ export function useCommentsListState({
onDeleteComment={onDeleteComment}
onSetReaction={onSetReaction}
onQueueForAgent={canQueue ? () => addGroupToSelection(groupId) : undefined}
onOpenLocation={onOpenLocation}
/>
)
}