diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index a13d62506fc..b4ba830b622 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -114,7 +114,16 @@ describe('listWorkItems', () => { author: { login: 'octocat' }, isDraft: false, headRefName: 'feature/add-feature', - baseRefName: 'main' + baseRefName: 'main', + reviewRequests: [ + { + requestedReviewer: { + login: 'AmethystLiang', + name: 'Amethyst Liang', + avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' + } + } + ] } ]) }) @@ -147,7 +156,7 @@ describe('listWorkItems', () => { '--limit', '10', '--json', - 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner', + 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests', '--repo', 'acme/widgets', '--assignee', @@ -157,7 +166,7 @@ describe('listWorkItems', () => { ) const prListFields = ghExecFileAsyncMock.mock.calls[1][0].join(',') expect(prListFields).not.toContain('statusCheckRollup') - expect(prListFields).not.toContain('reviewRequests') + expect(prListFields).toContain('reviewRequests') expect(prListFields).not.toContain('mergeStateStatus') expect(items).toEqual([ { @@ -182,7 +191,14 @@ describe('listWorkItems', () => { updatedAt: '2026-03-28T00:00:00Z', author: 'octocat', branchName: 'feature/add-feature', - baseRefName: 'main' + baseRefName: 'main', + reviewRequests: [ + { + login: 'AmethystLiang', + name: 'Amethyst Liang', + avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' + } + ] } ]) }) @@ -215,7 +231,7 @@ describe('listWorkItems', () => { '--limit', '10', '--json', - 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner', + 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests', '--repo', 'acme/widgets', '--state', @@ -315,7 +331,7 @@ describe('listWorkItems', () => { '--limit', '10', '--json', - 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner', + 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests', '--repo', 'acme/widgets', '--state', @@ -354,26 +370,24 @@ describe('listWorkItems', () => { it('marks fork PRs as cross-repository when REST payload only includes head.label', async () => { getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) - ghExecFileAsyncMock - .mockResolvedValueOnce({ stdout: '[]' }) - .mockResolvedValueOnce({ - stdout: JSON.stringify([ - { - number: 1849, - title: 'Fork PR with missing head repo', - state: 'open', - html_url: 'https://github.com/stablyai/orca/pull/1849', - updated_at: '2026-04-01T00:00:00Z', - user: { login: 'contributor' }, - head: { - ref: 'feat/onboarding-model-choice-782', - repo: null, - label: 'contributor:feat/onboarding-model-choice-782' - }, - base: { ref: 'main' } - } - ]) - }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 1849, + title: 'Fork PR with missing head repo', + state: 'open', + html_url: 'https://github.com/stablyai/orca/pull/1849', + updated_at: '2026-04-01T00:00:00Z', + user: { login: 'contributor' }, + head: { + ref: 'feat/onboarding-model-choice-782', + repo: null, + label: 'contributor:feat/onboarding-model-choice-782' + }, + base: { ref: 'main' } + } + ]) + }) const { items } = await listWorkItems('/repo-root', 10) expect(items).toEqual([ diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 864ba558cba..f278624ae28 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -84,6 +84,7 @@ vi.mock('./rate-limit', () => ({ import { getPRComments, getPRForBranch, + getWorkItem, getPullRequestPushTarget, mergePR, resolveReviewThread, @@ -824,17 +825,46 @@ describe('getPRForBranch', () => { remoteName: 'origin', branchName: 'feature/test' }) - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - ['api', 'repos/fork/orca/pulls/1849'], - { cwd: '/repo-root' } - ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(1, ['api', 'repos/fork/orca/pulls/1849'], { + cwd: '/repo-root' + }) expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( 2, ['api', 'repos/stablyai/orca/pulls/1849'], { cwd: '/repo-root' } ) }) + + it('normalizes reviewer avatars from REST pull request payloads', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 42, + title: 'Review me', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/42', + labels: [], + updated_at: '2026-03-28T00:00:00Z', + user: { login: 'author' }, + draft: false, + requested_reviewers: [ + { + login: 'AmethystLiang', + avatar_url: 'https://avatars.githubusercontent.com/u/1?v=4' + } + ] + }) + }) + + await expect(getWorkItem('/repo-root', 42, 'pr')).resolves.toMatchObject({ + reviewRequests: [ + { + login: 'AmethystLiang', + avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' + } + ] + }) + }) }) describe('GitHub GraphQL rate-limit guard', () => { diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 82ed230bc9a..588be53125d 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -249,11 +249,12 @@ export async function getAuthenticatedViewer(): Promise { type MainWorkItem = Omit const WORK_ITEM_PR_LIST_JSON_FIELDS = - 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner' + 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests' // Why: these fields are intentionally excluded from `gh pr list` because -// statusCheckRollup/review/merge metadata fan out into expensive GraphQL work -// across every row. Fetch them only for single-PR detail surfaces. +// 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. const WORK_ITEM_PR_DETAIL_JSON_FIELDS = 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,additions,deletions,changedFiles,reviewDecision,reviewRequests,latestReviews,assignees,statusCheckRollup,mergeable,mergeStateStatus,maintainerCanModify' @@ -342,7 +343,12 @@ function userFromUnknown( return { login, name: typeof raw.name === 'string' ? raw.name : null, - avatarUrl: typeof raw.avatarUrl === 'string' ? raw.avatarUrl : '' + avatarUrl: + typeof raw.avatarUrl === 'string' + ? raw.avatarUrl + : typeof raw.avatar_url === 'string' + ? raw.avatar_url + : '' } } @@ -2504,7 +2510,7 @@ export async function requestPRReviewers( ): Promise<{ ok: true } | { ok: false; error: string }> { const logins = reviewers.map((reviewer) => reviewer.trim()).filter(Boolean) if (logins.length === 0) { - return { ok: false, error: 'Enter at least one reviewer login' } + return { ok: false, error: 'Enter at least one reviewer' } } const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId)) const ownerRepo = await getOwnerRepo(repoPath, connectionId) diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 92f035a7cc8..a0635b7fa6c 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -34,11 +34,13 @@ import { RefreshCw, Send, UndoDot, + Users, X } from 'lucide-react' import { toast } from 'sonner' import { Button } from '@/components/ui/button' import { ButtonGroup } from '@/components/ui/button-group' +import { Input } from '@/components/ui/input' import { Sheet, SheetContent, SheetDescription, SheetTitle } from '@/components/ui/sheet' import { VisuallyHidden } from 'radix-ui' import { @@ -84,6 +86,11 @@ import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-cl import { useRepoLabels, useRepoAssignees, useImmediateMutation } from '@/hooks/useIssueMetadata' import { useRepoLabelsBySlug, useRepoAssigneesBySlug } from '@/hooks/useGitHubSlugMetadata' import IssueSourceIndicator, { sameGitHubOwnerRepo } from '@/components/github/IssueSourceIndicator' +import { + appendGitHubPRRequestedReviewers, + getGitHubPRReviewerRows, + normalizeGitHubReviewerLogins +} from '@/components/github-pr-reviewer-display' import type { GitHubOwnerRepo, GitHubPRFile, @@ -177,6 +184,10 @@ type GitHubItemDialogProps = { repoId?: string | null /** Called when the user clicks the primary CTA to start work from this item. */ onUse: (item: GitHubWorkItem) => void + onReviewRequestsChange?: ( + itemKey: { id: string; repoId: string }, + reviewRequests: GitHubAssignableUser[] + ) => void onClose: () => void /** Optional Project-origin context. When set, edits in the dialog are * routed via slug-addressed mutation IPCs against the row's actual repo @@ -359,6 +370,193 @@ function WorkItemStateBadge({ ) } +function ReviewerAvatar({ + login, + avatarUrl +}: { + login: string + avatarUrl: string +}): React.JSX.Element { + if (avatarUrl) { + return ( + + ) + } + return ( + + {login.slice(0, 1).toUpperCase()} + + ) +} + +function PRReviewersPanel({ + item, + loading, + repoPath, + onReviewersRequested +}: { + item: GitHubWorkItem + loading: boolean + repoPath: string | null + onReviewersRequested: (reviewRequests: GitHubAssignableUser[]) => void +}): React.JSX.Element { + const [reviewerInput, setReviewerInput] = useState('') + const [submitting, setSubmitting] = useState(false) + const [localReviewRequests, setLocalReviewRequests] = useState( + () => item.reviewRequests ?? [] + ) + const patchWorkItem = useAppStore((s) => s.patchWorkItem) + + useEffect(() => { + setLocalReviewRequests(item.reviewRequests ?? []) + }, [item.id, item.reviewRequests]) + + const displayItem = { ...item, reviewRequests: localReviewRequests } + const reviewers = getGitHubPRReviewerRows(displayItem) + const selectedReviewerLogins = useMemo( + () => + new Set( + localReviewRequests.map((reviewer) => reviewer.login.trim().toLowerCase()).filter(Boolean) + ), + [localReviewRequests] + ) + const hasReviewerMetadata = + item.reviewDecision !== undefined || + localReviewRequests.length > 0 || + item.reviewRequests !== undefined || + item.latestReviews !== undefined + const canRequestReview = + !!repoPath || getActiveRuntimeTarget(useAppStore.getState().settings).kind === 'environment' + + const handleRequestReview = async (event: React.FormEvent): Promise => { + event.preventDefault() + if (submitting) { + return + } + const logins = normalizeGitHubReviewerLogins( + reviewerInput.split(/[\s,]+/), + selectedReviewerLogins + ) + if (logins.length === 0) { + toast.error('Enter a reviewer') + return + } + if (localReviewRequests.length + logins.length > 15) { + toast.error('You can request up to 15 reviewers') + return + } + const target = getActiveRuntimeTarget(useAppStore.getState().settings) + if (target.kind !== 'environment' && !repoPath) { + toast.error('No repo context available for this pull request.') + return + } + setSubmitting(true) + try { + const result = + target.kind === 'environment' + ? await callRuntimeRpc<{ ok: boolean; error?: string }>( + target, + 'github.requestPRReviewers', + { repo: item.repoId, prNumber: item.number, reviewers: logins }, + { timeoutMs: 30_000 } + ) + : await window.api.gh.requestPRReviewers({ + repoPath: repoPath ?? '', + repoId: item.repoId, + prNumber: item.number, + reviewers: logins + }) + if (!result.ok) { + toast.error(result.error ?? 'Failed to request reviewer') + return + } + const nextReviewRequests = appendGitHubPRRequestedReviewers(localReviewRequests, logins) + setLocalReviewRequests(nextReviewRequests) + patchWorkItem(item.id, { reviewRequests: nextReviewRequests }, item.repoId) + onReviewersRequested(nextReviewRequests) + setReviewerInput('') + toast.success(logins.length === 1 ? 'Reviewer requested' : 'Reviewers requested') + } catch { + toast.error('Failed to request reviewer') + } finally { + setSubmitting(false) + } + } + + return ( + + ) +} + function fileStatusTone(status: GitHubPRFile['status']): string { switch (status) { case 'added': @@ -743,6 +941,25 @@ function patchCachedPRChecks(cacheKey: string, checks: PRCheckDetail[]): void { }) } +function patchCachedPRReviewRequests( + cacheKey: string, + reviewRequests: GitHubAssignableUser[] +): void { + const prev = workItemDetailsCache.get(cacheKey) + if (!prev?.details) { + return + } + touchWorkItemDetailsCache(cacheKey, { + ...prev, + details: { + ...prev.details, + item: { ...prev.details.item, reviewRequests } + }, + fetchedAt: Date.now(), + error: undefined + }) +} + // Why: install once at module load — every dialog instance shares the cache, // so a single subscription is enough. The preload bridge re-emits the // main-process broadcast for every window, so each renderer invalidates its @@ -1484,7 +1701,8 @@ function ConversationTab({ onUse, onMutated, onChecksUpdated, - onCommentAdded + onCommentAdded, + onReviewersRequested }: { item: GitHubWorkItem repoPath: string | null @@ -1504,6 +1722,7 @@ function ConversationTab({ onMutated: () => void onChecksUpdated: (checks: PRCheckDetail[]) => void onCommentAdded: (comment: PRComment) => void + onReviewersRequested: (reviewRequests: GitHubAssignableUser[]) => void }): React.JSX.Element { const authorLabel = item.author ?? 'unknown' const [replyingTo, setReplyingTo] = useState(null) @@ -1573,7 +1792,7 @@ function ConversationTab({ const startWorkspaceButton = ( ) - const inserted = `@${suggestion.login}` - const nextValue = `${reviewerInput.slice(0, token.start)}${inserted}${reviewerInput.slice(token.end)}` - const nextCaret = token.start + inserted.length - setReviewerInput(nextValue) - setReviewerInputCaret(nextCaret) - setReviewerSuggestionsOpen(false) - requestAnimationFrame(() => { - reviewerInputRef.current?.focus() - reviewerInputRef.current?.setSelectionRange(nextCaret, nextCaret) - }) } return ( - + event.stopPropagation()} > -
-
-
Reviewers
-
- {reviewers.length > 0 ? ( - reviewers.slice(0, 6).map((reviewer) => ( -
- @{reviewer.login} - - {reviewer.state} - +
+
+ Request up to 15 reviewers +
+
+
+ setReviewerInput(event.target.value)} + placeholder="Type or choose a user" + disabled={!repo || submitting} + className="h-8 rounded-md bg-background px-2 text-[13px]" + aria-label="Type or choose a user" + aria-autocomplete="list" + onKeyDown={(event) => { + if (event.key === 'ArrowDown' && actionableReviewerRows.length > 0) { + event.preventDefault() + setActiveReviewerIndex((current) => (current + 1) % actionableReviewerRows.length) + return + } + if (event.key === 'ArrowUp' && actionableReviewerRows.length > 0) { + event.preventDefault() + setActiveReviewerIndex( + (current) => + (current - 1 + actionableReviewerRows.length) % actionableReviewerRows.length + ) + return + } + if (event.key === 'Enter') { + event.preventDefault() + const activeReviewer = actionableReviewerRows[activeReviewerIndex] + if (activeReviewer) { + void requestReviewer(activeReviewer) + return + } + void handleRequestReview() + return + } + if (event.key === 'Escape') { + event.preventDefault() + handleReviewerPickerOpenChange(false) + } + }} + /> +
+
+ {reviewerMetadata.loading ? ( +
Loading…
+ ) : filteredReviewerCandidates.length > 0 ? ( + <> + {suggestedReviewerRows.length > 0 ? ( + <> +
+ Suggestions
- )) + {suggestedReviewerRows.map((reviewer, index) => + renderReviewerPickerRow(reviewer, { suggested: true, activeIndex: index }) + )} + + ) : null} +
+ Everyone else +
+ {everyoneElseReviewerRows.length > 0 ? ( + everyoneElseReviewerRows.map((reviewer, index) => + renderReviewerPickerRow(reviewer, { + suggested: false, + activeIndex: suggestedReviewerRows.length + index + }) + ) ) : ( -
- {hasReviewerMetadata - ? 'No reviewers requested yet.' - : 'Open the PR details to view current reviewers.'} +
+ No matching reviewers.
)} + + ) : ( +
+ {reviewerMetadata.error ?? + (hasReviewerMetadata + ? 'No matching reviewers.' + : 'Open the PR details to view current reviewers.')}
-
-
-
- { - setReviewerInput(event.target.value) - setReviewerInputCaret( - event.currentTarget.selectionStart ?? event.target.value.length - ) - setReviewerSuggestionsOpen(true) - }} - onClick={(event) => { - setReviewerInputCaret(event.currentTarget.selectionStart ?? reviewerInput.length) - setReviewerSuggestionsOpen(true) - }} - onFocus={(event) => { - setReviewerInputCaret(event.currentTarget.selectionStart ?? reviewerInput.length) - setReviewerSuggestionsOpen(true) - }} - onBlur={() => setReviewerSuggestionsOpen(false)} - onKeyUp={(event) => { - if (!['ArrowDown', 'ArrowUp', 'Enter', 'Tab', 'Escape'].includes(event.key)) { - setReviewerInputCaret( - event.currentTarget.selectionStart ?? reviewerInput.length - ) - } - }} - placeholder="login or @login" - disabled={!repo || submitting} - className="h-8 text-xs" - aria-expanded={showReviewerSuggestions} - aria-autocomplete="list" - onKeyDown={(event) => { - if (showReviewerSuggestions && reviewerSuggestions.length > 0) { - if (event.key === 'ArrowDown') { - event.preventDefault() - setActiveReviewerSuggestionIndex( - (current) => (current + 1) % reviewerSuggestions.length - ) - return - } - if (event.key === 'ArrowUp') { - event.preventDefault() - setActiveReviewerSuggestionIndex( - (current) => - (current - 1 + reviewerSuggestions.length) % reviewerSuggestions.length - ) - return - } - if (event.key === 'Enter' || event.key === 'Tab') { - event.preventDefault() - insertReviewerSuggestion( - reviewerSuggestions[activeReviewerSuggestionIndex] ?? reviewerSuggestions[0] - ) - return - } - } - if (event.key === 'Escape' && reviewerSuggestionsOpen) { - event.preventDefault() - setReviewerSuggestionsOpen(false) - return - } - if (event.key === 'Enter') { - event.preventDefault() - void handleRequestReview() - } - }} - /> - -
- {showReviewerSuggestions && ( -
- {reviewerMetadata.loading ? ( -
Loading…
- ) : reviewerSuggestions.length > 0 ? ( - reviewerSuggestions.map((suggestion, index) => ( - - )) - ) : ( -
- {reviewerMetadata.error ?? 'No matching reviewers.'} -
- )} -
- )} -
+ )}
@@ -1825,6 +1846,7 @@ export default function TaskPage(): React.JSX.Element { defaultTaskViewPreset ) const [tasksLoading, setTasksLoading] = useState(false) + const [tasksRefreshing, setTasksRefreshing] = useState(false) const [tasksError, setTasksError] = useState(null) // Why: per-repo failure count surfaced through the "N of M" banner. IPC-level // rejections populate tasksError instead — the two are mutually exclusive so @@ -1908,6 +1930,34 @@ export default function TaskPage(): React.JSX.Element { setDialogWorkItemFallback(item) }, []) + const patchTaskPageWorkItemRows = useCallback( + (itemKey: { id: string; repoId: string }, patch: Partial): void => { + setPages((current) => { + let changed = false + const nextPages = current.map((page) => { + let pageChanged = false + const nextPage = page.map((item) => { + if (item.id !== itemKey.id || item.repoId !== itemKey.repoId) { + return item + } + pageChanged = true + changed = true + return { ...item, ...patch } + }) + return pageChanged ? nextPage : page + }) + return changed ? nextPages : current + }) + }, + [] + ) + const handleDialogReviewRequestsChange = useCallback( + (itemKey: { id: string; repoId: string }, reviewRequests: GitHubAssignableUser[]): void => { + patchTaskPageWorkItemRows(itemKey, { reviewRequests }) + }, + [patchTaskPageWorkItemRows] + ) + // Why: feature 1 — render the "Issues from {owner}/{repo}" indicator per // selected repo whose issue-source and PR-source slugs differ, and surface // a per-repo retryable banner when the issue-side fetch failed. Both derive @@ -1922,6 +1972,17 @@ export default function TaskPage(): React.JSX.Element { [selectedRepos, selectedWorkItemsCacheEntries] ) + useEffect(() => { + if (taskSource !== 'github' || githubMode !== 'items') { + return + } + // Why: inline/dialog edits patch `workItemsCache`; the paged table renders + // from a local snapshot so it needs the patched row objects copied across. + setPages((current) => + reconcileTaskPagePagesWithWorkItemsCache(current, selectedWorkItemsCacheEntries) + ) + }, [githubMode, selectedWorkItemsCacheEntries, taskSource]) + // Why: surface a one-time toast per session per repo when the user's // preferred `'upstream'` is no longer configured and we fell back to // origin. Gated on a ref-backed set so repeated list refreshes don't @@ -1981,6 +2042,10 @@ export default function TaskPage(): React.JSX.Element { }, [selectedRepos] ) + const handleRefreshGithubTasks = useCallback((): void => { + setTasksRefreshing(true) + setTaskRefreshNonce((current) => current + 1) + }, []) const [newIssueOpen, setNewIssueOpen] = useState(false) const [newIssueTitle, setNewIssueTitle] = useState('') const [newIssueBody, setNewIssueBody] = useState('') @@ -2692,10 +2757,12 @@ export default function TaskPage(): React.JSX.Element { // button would stay stuck in its disabled/Retrying state indefinitely. if (taskSource !== 'github' || githubMode !== 'items') { setRetryingRepoPaths(new Set()) + setTasksRefreshing(false) return } if (selectedRepos.length === 0) { setRetryingRepoPaths(new Set()) + setTasksRefreshing(false) return } // unreachable — multi-combobox forbids empty @@ -2745,6 +2812,11 @@ export default function TaskPage(): React.JSX.Element { const preferenceInvalidated = workItemsInvalidationNonce !== lastFetchedInvalidationNonceRef.current lastFetchedInvalidationNonceRef.current = workItemsInvalidationNonce + const forcedFetch = (forceRefresh && taskRefreshNonce > 0) || preferenceInvalidated + // Why: manual refresh keeps cached rows visible, so the normal + // `tasksLoading` flag may stay false. Track the forced fetch separately + // so the toolbar still shows a refresh-in-progress affordance. + setTasksRefreshing(forcedFetch) const repoArgs = selectedRepos.map((r) => ({ repoId: r.id, path: r.path })) // Why: snapshot the retrying paths at effect-dispatch so overlapping @@ -2754,7 +2826,7 @@ export default function TaskPage(): React.JSX.Element { // when this effect dispatched preserves later additions. const dispatchedRetryPaths = retryingRepoPaths void fetchWorkItemsAcrossRepos(repoArgs, PER_REPO_FETCH_LIMIT, CROSS_REPO_DISPLAY_LIMIT, q, { - force: (forceRefresh && taskRefreshNonce > 0) || preferenceInvalidated + force: forcedFetch }) .then(({ items, failedCount: failed }) => { // Why: clear only the repos this effect was responsible for @@ -2780,6 +2852,7 @@ export default function TaskPage(): React.JSX.Element { setCurrentPage(0) setFailedCount(failed) setTasksLoading(false) + setTasksRefreshing(false) }) .catch((err) => { // Why: fetchWorkItemsAcrossRepos swallows per-repo failures, so a @@ -2806,6 +2879,7 @@ export default function TaskPage(): React.JSX.Element { setTasksError(err instanceof Error ? err.message : 'Failed to load GitHub work.') setFailedCount(0) // the per-repo banner would be misleading next to tasksError setTasksLoading(false) + setTasksRefreshing(false) }) // Why: fire-and-forget count query in parallel with the items fetch. @@ -3123,6 +3197,8 @@ export default function TaskPage(): React.JSX.Element { setSelectedLinearIssue ]) + const githubTasksBusy = tasksLoading || tasksRefreshing + useEffect(() => { // Why: when a modal is open, let it own Esc dismissal. if ( @@ -3382,19 +3458,19 @@ export default function TaskPage(): React.JSX.Element { return (
-
+
{/* Why: pt-1.5 vertically centers this row's 32px icon cluster (X + source toggles) with the sidebar's "Tasks" nav row. Sidebar Tasks center sits 22px below the titlebar (pt-2 + py-1.5 + half size-4 icon). Matching that here needs 6px top padding above the 32px cluster (6 + 16 = 22). The previous pt-3 placed the cluster 6px too low, breaking the visual band across the top chrome. */} -
+
-
+
{/* Why: Close is anchored left in the same row as the source icons so the top chrome is one compact band. Left-aligned keeps it clear of the app sidebar on the @@ -3478,7 +3554,7 @@ export default function TaskPage(): React.JSX.Element { ) : null} -
+
{taskSource === 'github' ? ( -
+
{projectModeVisible ? (
{GITHUB_MODE_BUTTONS.map((mode) => { @@ -3545,7 +3621,7 @@ export default function TaskPage(): React.JSX.Element { inert — hide it to avoid suggesting it does something. */} {githubMode !== 'project' && ( -
+
-
-
+
+
+
{getGitHubTaskKindPresets(activeGithubTaskKind).map((option) => { const active = activeTaskPreset === option.id @@ -3637,12 +3713,15 @@ export default function TaskPage(): React.JSX.Element { - Refresh GitHub work + {githubTasksBusy ? 'Refreshing GitHub work…' : 'Refresh GitHub work'}
-
+
) : taskSource === 'linear' && linearStatus.connected ? ( -
+
{LINEAR_PRESETS.map((preset) => { @@ -3832,8 +3911,8 @@ export default function TaskPage(): React.JSX.Element {
-
-
+
+
) : taskSource === 'gitlab' ? ( -
+
{/* Why: view toggle — Project = the selected repo's MRs and issues; My Todos = the user's cross-project gitlab.com/dashboard/todos stream. They have @@ -3972,11 +4051,11 @@ export default function TaskPage(): React.JSX.Element {
{taskSource === 'github' && githubMode === 'project' ? ( -
+
) : taskSource === 'github' ? ( -
+
- ID - Title / Context + ID + Title / Context Branch Status {showPRManagementColumns ? ( @@ -4071,10 +4150,10 @@ export default function TaskPage(): React.JSX.Element {
{Array.from({ length: 3 }).map((_, i) => (
-
+
-
+
@@ -4154,11 +4233,11 @@ export default function TaskPage(): React.JSX.Element { } }} className={cn( - 'grid cursor-pointer gap-2 px-3 py-2 text-left transition hover:bg-muted/40 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring/50', + 'group/github-task-row grid cursor-pointer gap-2 px-3 py-2 text-left transition hover:bg-muted/40 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring/50', githubTaskGridClass )} > -
+
{item.type === 'pr' ? ( @@ -4171,7 +4250,7 @@ export default function TaskPage(): React.JSX.Element {
-
+

{item.title} @@ -5332,6 +5411,7 @@ export default function TaskPage(): React.JSX.Element { setDialogWorkItem(null) handleUseWorkItem(item) }} + onReviewRequestsChange={handleDialogReviewRequestsChange} onClose={() => setDialogWorkItem(null)} /> diff --git a/src/renderer/src/components/github-pr-reviewer-display.test.ts b/src/renderer/src/components/github-pr-reviewer-display.test.ts new file mode 100644 index 00000000000..62a762ad70c --- /dev/null +++ b/src/renderer/src/components/github-pr-reviewer-display.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from 'vitest' + +import type { GitHubWorkItem } from '../../../shared/types' +import { + appendGitHubPRRequestedReviewers, + getGitHubPRPrimaryReviewer, + getGitHubPRReviewerRows, + getGitHubPRReviewLabel, + normalizeGitHubReviewerLogins +} from './github-pr-reviewer-display' + +function item(patch: Partial): GitHubWorkItem { + return patch as GitHubWorkItem +} + +describe('GitHub PR reviewer display', () => { + it('shows the requested reviewer instead of a request count', () => { + expect( + getGitHubPRReviewLabel( + item({ + reviewRequests: [{ login: 'ExampleReviewer', name: null, avatarUrl: '' }] + }) + ) + ).toBe('ExampleReviewer') + }) + + it('keeps multiple reviewers compact while still naming the first reviewer', () => { + expect( + getGitHubPRReviewLabel( + item({ + reviewRequests: [ + { login: 'ExampleReviewer', name: null, avatarUrl: '' }, + { login: 'agent-slack', name: null, avatarUrl: '' }, + { login: 'stably', name: null, avatarUrl: '' } + ] + }) + ) + ).toBe('ExampleReviewer +2') + }) + + it('preserves stronger review decision labels', () => { + expect( + getGitHubPRReviewLabel( + item({ + reviewDecision: 'APPROVED', + reviewRequests: [{ login: 'ExampleReviewer', name: null, avatarUrl: '' }] + }) + ) + ).toBe('Approved') + }) + + it('falls back to reviewed users and empty metadata labels', () => { + expect(getGitHubPRReviewLabel(item({ latestReviews: [{ login: 'reviewer' }] }))).toBe( + 'reviewer' + ) + expect(getGitHubPRReviewLabel(item({ reviewRequests: [] }))).toBe('No reviewers') + expect(getGitHubPRReviewLabel(item({}))).toBe('Reviewers') + }) + + it('returns the primary reviewer avatar without requiring another lookup', () => { + expect( + getGitHubPRPrimaryReviewer( + item({ + reviewRequests: [ + { + login: 'ExampleReviewer', + name: null, + avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' + } + ] + }) + ) + ).toEqual({ + login: 'ExampleReviewer', + name: null, + avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' + }) + }) + + it('builds reviewer rows for requested and reviewed users', () => { + expect( + getGitHubPRReviewerRows( + item({ + reviewRequests: [{ login: 'ExampleReviewer', name: null, avatarUrl: 'avatar-1' }], + latestReviews: [ + { login: 'reviewer', state: 'APPROVED', avatarUrl: 'avatar-2' }, + { login: 'ExampleReviewer', state: 'COMMENTED', avatarUrl: 'avatar-1b' } + ] + }) + ) + ).toEqual([ + { + login: 'ExampleReviewer', + name: null, + avatarUrl: 'avatar-1', + stateLabel: 'Requested' + }, + { + login: 'reviewer', + name: null, + avatarUrl: 'avatar-2', + stateLabel: 'Approved' + } + ]) + }) + + it('appends requested reviewers without duplicating existing logins', () => { + expect( + appendGitHubPRRequestedReviewers( + [{ login: 'ExampleReviewer', name: null, avatarUrl: 'avatar-1' }], + ['examplereviewer', '@new-reviewer'] + ) + ).toEqual([ + { login: 'ExampleReviewer', name: null, avatarUrl: 'avatar-1' }, + { login: 'new-reviewer', name: null, avatarUrl: '' } + ]) + }) + + it('normalizes reviewer input before sending it to GitHub', () => { + expect( + normalizeGitHubReviewerLogins( + [' @ExampleReviewer ', 'examplereviewer', '@new-reviewer'], + new Set(['existing']) + ) + ).toEqual(['ExampleReviewer', 'new-reviewer']) + }) +}) diff --git a/src/renderer/src/components/github-pr-reviewer-display.ts b/src/renderer/src/components/github-pr-reviewer-display.ts new file mode 100644 index 00000000000..1988b0c42a4 --- /dev/null +++ b/src/renderer/src/components/github-pr-reviewer-display.ts @@ -0,0 +1,164 @@ +import type { GitHubAssignableUser, GitHubWorkItem } from '../../../shared/types' + +type ReviewDisplayItem = Pick +export type GitHubPRPrimaryReviewer = Pick & { + name?: string | null +} +export type GitHubPRReviewerRow = GitHubPRPrimaryReviewer & { + stateLabel: string +} + +function uniqueLogins(logins: readonly (string | null | undefined)[]): string[] { + const seen = new Set() + const result: string[] = [] + for (const login of logins) { + const trimmed = login?.trim() + if (!trimmed) { + continue + } + const key = trimmed.toLowerCase() + if (seen.has(key)) { + continue + } + seen.add(key) + result.push(trimmed) + } + return result +} + +export function normalizeGitHubReviewerLogins( + logins: readonly string[], + excludedLogins: ReadonlySet = new Set() +): string[] { + return uniqueLogins(logins.map((login) => login.trim().replace(/^@/, ''))).filter( + (login) => !excludedLogins.has(login.toLowerCase()) + ) +} + +function formatReviewerLogins(logins: readonly string[]): string | null { + if (logins.length === 0) { + return null + } + if (logins.length === 1) { + return logins[0] + } + return `${logins[0]} +${logins.length - 1}` +} + +function formatReviewState(state: string | null | undefined): string { + switch (state) { + case 'APPROVED': + return 'Approved' + case 'CHANGES_REQUESTED': + return 'Changes requested' + case 'COMMENTED': + return 'Commented' + case 'DISMISSED': + return 'Dismissed' + case 'PENDING': + return 'Pending' + default: + return 'Reviewed' + } +} + +export function getGitHubPRReviewLabel(item: ReviewDisplayItem): string { + if ( + item.reviewDecision === undefined && + item.reviewRequests === undefined && + item.latestReviews === undefined + ) { + return 'Reviewers' + } + if (item.reviewDecision === 'APPROVED') { + return 'Approved' + } + if (item.reviewDecision === 'CHANGES_REQUESTED') { + return 'Changes requested' + } + const requestedLabel = formatReviewerLogins( + uniqueLogins((item.reviewRequests ?? []).map((user) => user.login)) + ) + if (requestedLabel) { + return requestedLabel + } + const reviewedLabel = formatReviewerLogins( + uniqueLogins((item.latestReviews ?? []).map((review) => review.login)) + ) + if (reviewedLabel) { + return reviewedLabel + } + return 'No reviewers' +} + +export function getGitHubPRPrimaryReviewer( + item: ReviewDisplayItem +): GitHubPRPrimaryReviewer | null { + const requested = (item.reviewRequests ?? []).find((user) => user.login.trim()) + if (requested) { + return requested + } + const reviewed = (item.latestReviews ?? []).find((review) => review.login.trim()) + if (reviewed) { + return { + login: reviewed.login, + avatarUrl: reviewed.avatarUrl ?? '', + name: null + } + } + return null +} + +export function getGitHubPRReviewerRows(item: ReviewDisplayItem): GitHubPRReviewerRow[] { + const byLogin = new Map() + for (const user of item.reviewRequests ?? []) { + const login = user.login.trim() + if (!login) { + continue + } + byLogin.set(login.toLowerCase(), { + login, + name: user.name, + avatarUrl: user.avatarUrl, + stateLabel: 'Requested' + }) + } + for (const review of item.latestReviews ?? []) { + const login = review.login.trim() + const key = login.toLowerCase() + if (!login || byLogin.has(key)) { + continue + } + byLogin.set(key, { + login, + name: null, + avatarUrl: review.avatarUrl ?? '', + stateLabel: formatReviewState(review.state) + }) + } + return Array.from(byLogin.values()) +} + +export function appendGitHubPRRequestedReviewers( + current: readonly GitHubAssignableUser[], + logins: readonly string[] +): GitHubAssignableUser[] { + const byLogin = new Map() + for (const user of current) { + const login = user.login.trim() + if (login) { + byLogin.set(login.toLowerCase(), user) + } + } + for (const rawLogin of logins) { + const login = rawLogin.trim().replace(/^@/, '') + if (!login) { + continue + } + const key = login.toLowerCase() + if (!byLogin.has(key)) { + byLogin.set(key, { login, name: null, avatarUrl: '' }) + } + } + return Array.from(byLogin.values()) +} diff --git a/src/renderer/src/components/github-project/ProjectRow.tsx b/src/renderer/src/components/github-project/ProjectRow.tsx index 627b66c1d82..e76fa769362 100644 --- a/src/renderer/src/components/github-project/ProjectRow.tsx +++ b/src/renderer/src/components/github-project/ProjectRow.tsx @@ -13,6 +13,11 @@ import type { GitHubProjectRow as GitHubProjectRowType } from '../../../../shared/github-project-types' +const PROJECT_FROZEN_COLUMN_SURFACE_CLASS = + '[background:color-mix(in_srgb,var(--muted)_50%,var(--background))]' +const PROJECT_FROZEN_COLUMN_HOVER_SURFACE_CLASS = + 'group-hover/project-row:[background:color-mix(in_srgb,var(--accent)_60%,var(--background))]' + type Props = { row: GitHubProjectRowType fields: GitHubProjectField[] @@ -55,15 +60,32 @@ export default function ProjectRow({ const rowInner = (
{fields.map((f, idx) => { const next = fields[idx + 1] + const frozen = idx < 2 return ( -
+
> +): string { + // Why: the first two columns are frozen during horizontal scroll, so their + // actual widths must be deterministic for the second sticky offset. + const cols = fields.map((field, index) => + index < 2 + ? `${resolveWidth(field, widths)}px` + : `minmax(${MIN_COLUMN_WIDTH}px, ${resolveWidth(field, widths)}fr)` + ) + cols.push(`${ACTION_COLUMN_WIDTH}px`) + return cols.join(' ') +} + type Props = { table: GitHubProjectTable onOpenDialog?: (row: GitHubProjectRow) => void @@ -94,7 +112,16 @@ export default function ProjectViewList({ [scopeKey] ) - const gridTemplate = useMemo(() => buildGridTemplate(fields, widths), [fields, widths]) + const gridTemplate = useMemo(() => buildProjectGridTemplate(fields, widths), [fields, widths]) + + const handleListScroll = useCallback((event: React.UIEvent): void => { + // Why: frozen columns need the horizontal offset, but piping every scroll + // tick through React state rerenders the entire project row set. + event.currentTarget.style.setProperty( + '--project-scroll-left', + `${event.currentTarget.scrollLeft}px` + ) + }, []) const toggleColumn = (fieldId: string): void => { setHidden((prev) => { @@ -167,7 +194,11 @@ export default function ProjectViewList({ : null return ( -
+
+
@@ -899,7 +909,7 @@ function ProjectSearchInput({ }, []) return ( -
+
{ + it('contains long PR body markdown inside its available width', () => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('min-w-0') + expect(markup).toContain('max-w-full') + expect(markup).toContain('[overflow-wrap:anywhere]') + expect(markup).toContain('overflow-x-auto') + }) +}) diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.tsx index 2ac38022b4f..62dc509bc97 100644 --- a/src/renderer/src/components/sidebar/CommentMarkdown.tsx +++ b/src/renderer/src/components/sidebar/CommentMarkdown.tsx @@ -38,11 +38,13 @@ const compactComponents: Components = { // more reliable than checking `className` — which is only set when // the fenced block specifies a language (```js), not for bare ```. code: ({ children }) => ( - {children} + + {children} + ), // Compact pre blocks — no syntax highlighting needed for short comments pre: ({ children }) => ( -
+    
       {children}
     
), @@ -90,7 +92,7 @@ const compactComponents: Components = { // overflow container keeps the card layout stable while still letting the // user scroll to see the full table. table: ({ children }) => ( -
+
{children}
@@ -112,10 +114,12 @@ const documentComponents: Components = { ), code: ({ children }) => ( - {children} + + {children} + ), pre: ({ children }) => ( -
+    
       {children}
     
), @@ -207,6 +211,7 @@ const CommentMarkdown = React.memo( // The descendant selector (pre code) has higher specificity than the // direct utility classes on , so these overrides win reliably. '[&_pre_code]:bg-transparent [&_pre_code]:p-0 [&_pre_code]:rounded-none', + 'min-w-0 max-w-full [overflow-wrap:anywhere]', className )} {...rest} diff --git a/src/renderer/src/components/task-page-cache-selectors.test.ts b/src/renderer/src/components/task-page-cache-selectors.test.ts index 2a9c735502d..5c73332c7af 100644 --- a/src/renderer/src/components/task-page-cache-selectors.test.ts +++ b/src/renderer/src/components/task-page-cache-selectors.test.ts @@ -7,6 +7,7 @@ import { buildTaskPageRepoSourceState, findTaskPageDialogWorkItem, findTaskPageLinearDrawerIssue, + reconcileTaskPagePagesWithWorkItemsCache, selectTaskPageWorkItemsCacheEntries } from './task-page-cache-selectors' @@ -73,6 +74,26 @@ describe('task page cache selectors', () => { expect(findTaskPageDialogWorkItem(cache, { id: 'issue-1', repoId: 'repo-2' })).toBeNull() }) + it('reconciles paged table rows with patched work-item cache entries', () => { + const stale = { + ...workItem('pr-1', 'repo-1'), + reviewRequests: [] + } + const patched = { + ...stale, + reviewRequests: [{ login: 'AmethystLiang', name: null, avatarUrl: '' }] + } + const otherRepoSameId = workItem('pr-1', 'repo-2') + const pages = [[stale, otherRepoSameId]] + + const nextPages = reconcileTaskPagePagesWithWorkItemsCache(pages, [ + entry([patched]) + ]) + + expect(nextPages[0][0]).toBe(patched) + expect(nextPages[0][1]).toBe(otherRepoSameId) + }) + it('returns null while the Linear drawer is closed and finds open issues by stable reference', () => { const issue = linearIssue('LIN-1') const searchIssue = linearIssue('LIN-2') diff --git a/src/renderer/src/components/task-page-cache-selectors.ts b/src/renderer/src/components/task-page-cache-selectors.ts index 816be6cc66e..99bda1ffb0c 100644 --- a/src/renderer/src/components/task-page-cache-selectors.ts +++ b/src/renderer/src/components/task-page-cache-selectors.ts @@ -51,6 +51,39 @@ export function buildTaskPageRepoSourceState( }) } +function taskPageWorkItemCacheKey(item: GitHubWorkItem): string { + return `${item.repoId}\u0000${item.id}` +} + +export function reconcileTaskPagePagesWithWorkItemsCache( + pages: readonly GitHubWorkItem[][], + entries: readonly (CacheEntry | undefined)[] +): GitHubWorkItem[][] { + const cachedItems = new Map() + for (const entry of entries) { + for (const item of entry?.data ?? []) { + cachedItems.set(taskPageWorkItemCacheKey(item), item) + } + } + + let changed = false + const nextPages = pages.map((page) => { + let pageChanged = false + const nextPage = page.map((item) => { + const cached = cachedItems.get(taskPageWorkItemCacheKey(item)) + if (!cached || cached === item) { + return item + } + pageChanged = true + changed = true + return cached + }) + return pageChanged ? nextPage : page + }) + + return changed ? nextPages : (pages as GitHubWorkItem[][]) +} + export function findTaskPageDialogWorkItem( workItemsCache: WorkItemsCache, dialogWorkItemKey: TaskPageDialogWorkItemKey diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index 8b0dc76d28e..44285903251 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -5,7 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { create } from 'zustand' import { createGitHubSlice, workItemsCacheKey } from './github' import type { AppState } from '../types' -import type { PRInfo } from '../../../../shared/types' +import type { GitHubWorkItem, PRInfo } from '../../../../shared/types' import { createCompatibleRuntimeStatusResponseIfNeeded, type RuntimeEnvironmentCallRequest @@ -163,6 +163,49 @@ describe('createGitHubSlice.evictGitHubRepoCaches', () => { }) }) +describe('createGitHubSlice.patchWorkItem', () => { + beforeEach(() => { + vi.clearAllMocks() + resetRemoteRuntimeMocks() + }) + + it('can scope patches to one repo when different repos have the same work-item id', () => { + const store = createTestStore() + const repoOneItem = { + id: 'pr:42', + repoId: 'repo-1', + type: 'pr', + number: 42, + title: 'Repo one PR' + } as GitHubWorkItem + const repoTwoItem = { + id: 'pr:42', + repoId: 'repo-2', + type: 'pr', + number: 42, + title: 'Repo two PR' + } as GitHubWorkItem + + store.setState({ + workItemsCache: { + [workItemsCacheKey('repo-1', 20, '')]: { data: [repoOneItem], fetchedAt: 1 }, + [workItemsCacheKey('repo-2', 20, '')]: { data: [repoTwoItem], fetchedAt: 1 } + } + }) + + store.getState().patchWorkItem('pr:42', { reviewRequests: [] }, 'repo-1') + + const state = store.getState() + const repoOnePatched = state.workItemsCache[workItemsCacheKey('repo-1', 20, '')]?.data?.[0] + const repoTwoPatched = state.workItemsCache[workItemsCacheKey('repo-2', 20, '')]?.data?.[0] + expect(repoOnePatched).toMatchObject({ + repoId: 'repo-1', + reviewRequests: [] + }) + expect(repoTwoPatched).toBe(repoTwoItem) + }) +}) + describe('createGitHubSlice.fetchPRChecks', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index c9423825ad7..e4227b89ead 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -585,7 +585,7 @@ export type GitHubSlice = { * "new workspace" buttons) to warm the cache before the page mounts. */ prefetchWorkItems: (repoId: string, repoPath: string, limit?: number, query?: string) => void - patchWorkItem: (itemId: string, patch: Partial) => void + patchWorkItem: (itemId: string, patch: Partial, repoId?: string | null) => void /** * Monotonic counter bumped whenever a repo's issue-source preference is * flipped. Subscribers (TaskPage's fetch effect) include this in their @@ -1663,7 +1663,7 @@ export const createGitHubSlice: StateCreator = (s } }, - patchWorkItem: (itemId, patch) => { + patchWorkItem: (itemId, patch, repoId) => { set((s) => { const nextCache = { ...s.workItemsCache } let changed = false @@ -1672,7 +1672,11 @@ export const createGitHubSlice: StateCreator = (s if (!entry?.data) { continue } - const idx = entry.data.findIndex((item) => item.id === itemId) + // Why: GitHub issue/PR ids are only unique within a repo. Cross-repo + // task views can contain the same `pr:42` id from multiple repos. + const idx = entry.data.findIndex( + (item) => item.id === itemId && (!repoId || item.repoId === repoId) + ) if (idx === -1) { continue }