From a3efdcb21bcba811cbb9db6e488f2bd4a3262e4e Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:38:23 +0000 Subject: [PATCH] feat(mobile): GitHub review threads, viewed sync and line comments on the diff screen (#37073) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(mobile): GitHub threads, viewed sync and line comments on the diff screen Committed files of a GitHub pull request now show its review threads under their lines with Reply and Resolve, take their viewed state from GitHub and sync the checkbox and Viewed · next file back through setPRFileViewed. A tap or a long-press drag picks lines and opens a sheet that sends the note to the workspace's agent through the existing send path, or posts it as a review comment. Threads and GitHub comments appear only while the local branch is at the PR head, the same contents desktop shows them on; otherwise the file says to push. openDiffForFile takes an optional line for a thread's Open in diff. * test(mobile): use a type import for the line-mutations recording adapter * fix(mobile): name the staged or unstaged side in the jump sheet's file labels --- .../scripts/verify-mobile-web-app-bundle.mjs | 24 +- mobile/app/h/[hostId]/review/[worktreeId].tsx | 3 +- .../pr-line-comment-and-file-viewed.json | 219 ++++++++++++++ mobile/rpc-foundation/pilot-scenarios.json | 77 +++++ .../src/components/MobileDiffReviewBody.tsx | 126 ++------ .../components/MobileDiffReviewDrawers.tsx | 30 +- .../MobileDiffReviewFileSummary.tsx | 20 +- .../components/MobileDiffReviewFilesSheet.tsx | 3 +- .../src/components/MobileDiffReviewLine.tsx | 21 +- .../src/components/MobileDiffReviewLines.tsx | 283 ++++++++++++++++++ .../components/MobileDiffReviewRangeSheet.tsx | 152 ++++++++++ .../components/MobileDiffReviewScreenView.tsx | 37 ++- .../mobile-diff-review-control-styles.ts | 9 + .../mobile-diff-review-layout-styles.ts | 8 + ...le-diff-review-send-sheet-stacking.test.ts | 7 +- .../pr-sidebar/PRCommentsSection.tsx | 46 +-- mobile/src/i18n/en-runtime-required.json | 4 + mobile/src/i18n/locales/en.json | 18 +- .../session/MobileDiffReviewRouteScreen.tsx | 4 + .../session/github-pr-entity-reply-schema.ts | 43 ++- .../session/github-pr-mutation-operations.ts | 16 + .../session/github-pr-read-reply-schema.ts | 8 +- .../github-pr-review-line-mutations.ts | 71 +++++ mobile/src/session/mobile-diff-comments.ts | 34 +++ .../session/mobile-diff-review-github.test.ts | 130 ++++++++ .../src/session/mobile-diff-review-github.ts | 136 +++++++++ .../session/mobile-diff-review-retry.test.tsx | 10 +- .../session/mobile-diff-review-rows.test.ts | 40 ++- mobile/src/session/mobile-diff-review-rows.ts | 22 +- .../src/session/mobile-diff-review-sheets.ts | 16 +- .../session/review-terminal-reply-schema.ts | 37 ++- .../src/session/session-reply-schema.test.ts | 53 ++++ .../use-mobile-diff-review-comment-actions.ts | 76 +++-- .../use-mobile-diff-review-controller.ts | 103 ++++--- .../session/use-mobile-diff-review-github.ts | 193 ++++++++++++ .../use-mobile-diff-review-interactions.ts | 58 +++- .../use-mobile-diff-review-range-note.ts | 56 ++++ ...se-mobile-diff-review-send-actions.test.ts | 8 +- .../use-mobile-diff-review-send-actions.ts | 123 ++++++-- .../use-mobile-pr-sidebar-controller.ts | 35 ++- .../src/source-control/mobile-review-route.ts | 5 + .../use-mobile-source-control-openers.ts | 3 +- .../bridged-parity/divergence-classes.ts | 2 +- .../diff-review-action-mount-adapters.ts | 2 + .../adapters/github-pr-mount-adapters.ts | 22 ++ 45 files changed, 2105 insertions(+), 288 deletions(-) create mode 100644 mobile/rpc-foundation/goldens/pr-line-comment-and-file-viewed.json create mode 100644 mobile/src/components/MobileDiffReviewLines.tsx create mode 100644 mobile/src/components/MobileDiffReviewRangeSheet.tsx create mode 100644 mobile/src/session/github-pr-review-line-mutations.ts create mode 100644 mobile/src/session/mobile-diff-review-github.test.ts create mode 100644 mobile/src/session/mobile-diff-review-github.ts create mode 100644 mobile/src/session/use-mobile-diff-review-github.ts create mode 100644 mobile/src/session/use-mobile-diff-review-range-note.ts diff --git a/config/scripts/verify-mobile-web-app-bundle.mjs b/config/scripts/verify-mobile-web-app-bundle.mjs index 642fe9718b7..9c35afc0bc2 100644 --- a/config/scripts/verify-mobile-web-app-bundle.mjs +++ b/config/scripts/verify-mobile-web-app-bundle.mjs @@ -72,18 +72,18 @@ export const MOBILE_WEB_APP_BUNDLE_SCRIPT_SWEEP = [ ['./h/[hostId]/accounts.tsx', 12], ['./h/[hostId]/agent-history/[worktreeId].tsx', 17], ['./h/[hostId]/automations.tsx', 24], - ['./h/[hostId]/edit.tsx', 30], - ['./h/[hostId]/files/[worktreeId].tsx', 35], - ['./h/[hostId]/files/preview/[worktreeId].tsx', 42], - ['./h/[hostId]/history/[worktreeId].tsx', 45], - ['./h/[hostId]/index.tsx', 53], - ['./h/[hostId]/new-workspace.tsx', 63], - ['./h/[hostId]/pr/[worktreeId].tsx', 64], - ['./h/[hostId]/review/[worktreeId].tsx', 77], - ['./h/[hostId]/session/[worktreeId].tsx', 92], - ['./h/[hostId]/source-control/[worktreeId].tsx', 100], - ['./h/[hostId]/tasks.tsx', 114], - ['./h/_layout.tsx', 118] + ['./h/[hostId]/edit.tsx', 29], + ['./h/[hostId]/files/[worktreeId].tsx', 34], + ['./h/[hostId]/files/preview/[worktreeId].tsx', 44], + ['./h/[hostId]/history/[worktreeId].tsx', 47], + ['./h/[hostId]/index.tsx', 55], + ['./h/[hostId]/new-workspace.tsx', 66], + ['./h/[hostId]/pr/[worktreeId].tsx', 67], + ['./h/[hostId]/review/[worktreeId].tsx', 80], + ['./h/[hostId]/session/[worktreeId].tsx', 95], + ['./h/[hostId]/source-control/[worktreeId].tsx', 104], + ['./h/[hostId]/tasks.tsx', 119], + ['./h/_layout.tsx', 123] ] const sweptScripts = MOBILE_WEB_APP_BUNDLE_SCRIPT_SWEEP.map(([, scripts]) => scripts) diff --git a/mobile/app/h/[hostId]/review/[worktreeId].tsx b/mobile/app/h/[hostId]/review/[worktreeId].tsx index 5d19428a8ff..e1b035ed67b 100644 --- a/mobile/app/h/[hostId]/review/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/review/[worktreeId].tsx @@ -23,6 +23,7 @@ export default withWorkspaceRoute(function MobileDiffReviewScreen() { file?: string | string[] area?: string | string[] commit?: string | string[] + line?: string | string[] }>() const hostId = firstReviewParam(params.hostId) const worktreeId = firstReviewParam(params.worktreeId) @@ -31,7 +32,7 @@ export default withWorkspaceRoute(function MobileDiffReviewScreen() { // Carry screen query params unchanged, omitting empty values: screen normalizers distinguish absence from empty scope/file/area. const routeParams: Record = { ...Object.fromEntries( - (['name', 'scope', 'file', 'area', 'commit'] as const) + (['name', 'scope', 'file', 'area', 'commit', 'line'] as const) .map((key) => [key, firstReviewParam(params[key])] as const) .filter(([, value]) => value !== '') ), diff --git a/mobile/rpc-foundation/goldens/pr-line-comment-and-file-viewed.json b/mobile/rpc-foundation/goldens/pr-line-comment-and-file-viewed.json new file mode 100644 index 00000000000..628fa51a1d0 --- /dev/null +++ b/mobile/rpc-foundation/goldens/pr-line-comment-and-file-viewed.json @@ -0,0 +1,219 @@ +{ + "goldenFormatVersion": 6, + "operation": "session.pr-mutations", + "family": "github.pr-comment-mutation", + "namedDeltas": [], + "values": { + "00e36cc2e1a3": { + "file-viewed": { + "ok": true + }, + "line-comment": { + "ok": true + } + }, + "0632a06c33cb": { + "name": "github.addPRReviewComment#1", + "ordinal": 2, + "json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"github.addPRReviewComment\",\"params\":{\"repo\":\"id:repo-9\",\"prNumber\":12,\"commitId\":\"head-sha-1\",\"path\":\"src/app.ts\",\"line\":5,\"body\":\"recorded line comment\",\"startLine\":3}}" + }, + "1da2b9026f18": { + "name": "github.setPRFileViewed#2", + "ordinal": 5, + "args": [ + { + "name": "method", + "value": "github.setPRFileViewed" + }, + { + "name": "params", + "value": { + "path": "src/app.ts", + "pullRequestId": "PR_kwDO", + "repo": "id:repo-9", + "viewed": true + } + }, + { + "name": "options", + "value": { + "$rpc": "absent" + } + } + ], + "settlement": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "id": "frame-3", + "ok": true, + "result": false + } + } + }, + "29a4002b51d7": { + "file-viewed": { + "error": "Failed to update viewed state.", + "ok": false + }, + "line-comment": { + "ok": true + } + }, + "5af21c5f4fe0": { + "name": "github.setPRFileViewed#1", + "ordinal": 3, + "args": [ + { + "name": "method", + "value": "github.setPRFileViewed" + }, + { + "name": "params", + "value": { + "path": "src/app.ts", + "pullRequestId": "PR_kwDO", + "repo": "id:repo-9", + "viewed": true + } + }, + { + "name": "options", + "value": { + "$rpc": "absent" + } + } + ], + "settlement": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "id": "frame-2", + "ok": true, + "result": true + } + } + }, + "65a217bed712": { + "line-comment": { + "ok": true + } + }, + "94ed6c684a96": { + "name": "github.setPRFileViewed#1", + "ordinal": 4, + "json": "{\"id\":\"frame-2\",\"deviceToken\":\"recording-device\",\"method\":\"github.setPRFileViewed\",\"params\":{\"repo\":\"id:repo-9\",\"pullRequestId\":\"PR_kwDO\",\"path\":\"src/app.ts\",\"viewed\":true}}" + }, + "afe0045df4a7": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "error": "Failed to update viewed state.", + "ok": false + } + }, + "c655a2fc8c3a": { + "name": "github.addPRReviewComment#1", + "ordinal": 1, + "args": [ + { + "name": "method", + "value": "github.addPRReviewComment" + }, + { + "name": "params", + "value": { + "body": "recorded line comment", + "commitId": "head-sha-1", + "line": 5, + "path": "src/app.ts", + "prNumber": 12, + "repo": "id:repo-9", + "startLine": 3 + } + }, + { + "name": "options", + "value": { + "$rpc": "absent" + } + } + ], + "settlement": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "id": "frame-1", + "ok": true, + "result": { + "comment": { + "id": 57 + }, + "ok": true + } + } + } + }, + "de607d31ed8c": { + "name": "github.setPRFileViewed#2", + "ordinal": 6, + "json": "{\"id\":\"frame-3\",\"deviceToken\":\"recording-device\",\"method\":\"github.setPRFileViewed\",\"params\":{\"repo\":\"id:repo-9\",\"pullRequestId\":\"PR_kwDO\",\"path\":\"src/app.ts\",\"viewed\":true}}" + }, + "fbc958e4d46e": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "ok": true + } + } + }, + "recording": { + "scenario": "pr-line-comment-and-file-viewed", + "checkpoints": [ + { + "id": "line-comment", + "observation": { + "sender": ["c655a2fc8c3a"], + "payloads": ["0632a06c33cb"], + "settlements": { + "line-comment": "fbc958e4d46e" + }, + "state": "65a217bed712", + "effects": [] + } + }, + { + "id": "viewed", + "observation": { + "sender": ["c655a2fc8c3a", "5af21c5f4fe0"], + "payloads": ["0632a06c33cb", "94ed6c684a96"], + "settlements": { + "line-comment": "fbc958e4d46e", + "viewed": "fbc958e4d46e" + }, + "state": "00e36cc2e1a3", + "effects": [] + } + }, + { + "id": "refused", + "observation": { + "sender": ["c655a2fc8c3a", "5af21c5f4fe0", "1da2b9026f18"], + "payloads": ["0632a06c33cb", "94ed6c684a96", "de607d31ed8c"], + "settlements": { + "line-comment": "fbc958e4d46e", + "viewed": "fbc958e4d46e", + "refused": "afe0045df4a7" + }, + "state": "29a4002b51d7", + "effects": [] + } + } + ] + } +} diff --git a/mobile/rpc-foundation/pilot-scenarios.json b/mobile/rpc-foundation/pilot-scenarios.json index 8a47a8cd735..903c7a35b08 100644 --- a/mobile/rpc-foundation/pilot-scenarios.json +++ b/mobile/rpc-foundation/pilot-scenarios.json @@ -8928,6 +8928,83 @@ } ] }, + { + "id": "pr-line-comment-and-file-viewed", + "operation": "session.pr-mutations", + "family": "github.pr-comment-mutation", + "sites": ["mobile/src/session/github-pr-review-line-mutations.ts"], + "schedules": [], + "steps": [ + { + "action": "line-comment", + "id": "line-comment" + }, + { + "complete": "github.addPRReviewComment#1", + "params": { + "repo": "id:repo-9", + "prNumber": 12, + "commitId": "head-sha-1", + "path": "src/app.ts", + "line": 5, + "body": "recorded line comment", + "startLine": 3 + }, + "reply": { + "ok": true, + "result": { + "ok": true, + "comment": { + "id": 57 + } + } + } + }, + { + "checkpoint": "line-comment" + }, + { + "action": "file-viewed", + "id": "viewed" + }, + { + "complete": "github.setPRFileViewed#1", + "params": { + "repo": "id:repo-9", + "pullRequestId": "PR_kwDO", + "path": "src/app.ts", + "viewed": true + }, + "reply": { + "ok": true, + "result": true + } + }, + { + "checkpoint": "viewed" + }, + { + "action": "file-viewed", + "id": "refused" + }, + { + "complete": "github.setPRFileViewed#2", + "params": { + "repo": "id:repo-9", + "pullRequestId": "PR_kwDO", + "path": "src/app.ts", + "viewed": true + }, + "reply": { + "ok": true, + "result": false + } + }, + { + "checkpoint": "refused" + } + ] + }, { "id": "pr-comment-resolve-unconfirmed", "operation": "session.pr-mutations", diff --git a/mobile/src/components/MobileDiffReviewBody.tsx b/mobile/src/components/MobileDiffReviewBody.tsx index 0934058623d..3ef922e2dac 100644 --- a/mobile/src/components/MobileDiffReviewBody.tsx +++ b/mobile/src/components/MobileDiffReviewBody.tsx @@ -1,26 +1,16 @@ -import { useMemo, useState } from 'react' -import { FlatList, Pressable, Text } from 'react-native' -import { ChevronsUpDown, FileText, ListChecks } from 'lucide-react-native' +import { FileText, ListChecks } from 'lucide-react-native' import type { DiffComment } from '../../../src/shared/diff-comment-types' import { MobileLoadErrorState, MobileOfflineState, MobileStateView } from './MobileStateView' import { useMobileColorScheme } from '../theme/use-mobile-color-scheme' import { useMobileTranslation } from '../i18n/use-mobile-translation' -import { MobileDiffReviewLine } from './MobileDiffReviewLine' -import type { - ReviewDiffLine, - ReviewDiffState, - ReviewScreenState -} from '../session/mobile-diff-review-screen-model' -import { - buildMobileDiffReviewRows, - type MobileDiffReviewRow -} from '../session/mobile-diff-review-rows' -import { colors } from '../theme/mobile-theme' +import { MobileDiffReviewLines, type LineRange } from './MobileDiffReviewLines' +import type { ReviewDiffState, ReviewScreenState } from '../session/mobile-diff-review-screen-model' import type { MobileDiffReviewQueueFilter, MobileDiffReviewQueueItem } from '../session/mobile-diff-review-queue' -import { mobileDiffReviewStyles as styles } from './mobile-diff-review-screen-styles' +import type { MobileDiffReviewThread } from '../session/mobile-diff-review-github' +import type { PRCommentCardActions } from './pr-sidebar/PRCommentCard' type Props = { commentsByLine: ReadonlyMap @@ -29,7 +19,13 @@ type Props = { filteredCount: number screenState: ReviewScreenState staleCommentIds: ReadonlySet - onAddNote: (lineNumber: number) => void + threadsByLine: ReadonlyMap + threadActions: PRCommentCardActions | undefined + selectedRange: LineRange | null + onPickLines: (range: LineRange) => void + /** Scroll to this line once, then report it done. */ + scrollToLine: number | null + onScrolledToLine: () => void onEditNote: (comment: DiffComment) => void onRetry: (() => void) | undefined /** The host is unreachable, so a failed load is a wait rather than an error. */ @@ -46,7 +42,12 @@ export function MobileDiffReviewBody({ filteredCount, screenState, staleCommentIds, - onAddNote, + threadsByLine, + threadActions, + selectedRange, + onPickLines, + scrollToLine, + onScrolledToLine, onEditNote, onRetry, offline, @@ -121,96 +122,17 @@ export function MobileDiffReviewBody({ truncated={diffState.truncated} commentsByLine={commentsByLine} staleCommentIds={staleCommentIds} - {...(currentItem?.scope === 'commit' ? {} : { onAddNote })} + threadsByLine={threadsByLine} + threadActions={threadActions} + selectedRange={selectedRange} + scrollToLine={scrollToLine} + onScrolledToLine={onScrolledToLine} + {...(currentItem?.scope === 'commit' ? {} : { onPickLines })} onEditNote={onEditNote} /> ) } -const NO_OPENED_LINES: ReadonlySet = new Set() - -// Keyed by file, so folds the reader opened close again on the next file. -function MobileDiffReviewLines({ - lines, - truncated, - commentsByLine, - staleCommentIds, - onAddNote, - onEditNote -}: { - lines: ReviewDiffLine[] - truncated: boolean - commentsByLine: ReadonlyMap - staleCommentIds: ReadonlySet - onAddNote?: (lineNumber: number) => void - onEditNote: (comment: DiffComment) => void -}) { - useMobileColorScheme() - const t = useMobileTranslation() - // Opened folds are line indexes into this load; a reload with new lines starts folded again. - const [opened, setOpened] = useState<{ lines: ReviewDiffLine[]; indexes: ReadonlySet }>( - () => ({ lines, indexes: new Set() }) - ) - const openedLines = opened.lines === lines ? opened.indexes : NO_OPENED_LINES - const rows = useMemo( - () => buildMobileDiffReviewRows(lines, openedLines, new Set(commentsByLine.keys())), - [commentsByLine, lines, openedLines] - ) - return ( - > - data={rows} - keyExtractor={(row) => row.key} - renderItem={({ item: row }) => - row.kind === 'fold' ? ( - [styles.foldRow, pressed && styles.buttonPressed]} - onPress={() => { - const next = new Set(openedLines) - for (let index = row.startIndex; index < row.endIndex; index += 1) { - next.add(index) - } - setOpened({ lines, indexes: next }) - }} - accessibilityRole="button" - accessibilityLabel={t( - 'components.diffReviewBody.showUnchangedLines', - 'Show {{count}} unchanged lines', - { count: row.hiddenCount } - )} - > - - - {row.hiddenCount === 1 - ? t('components.diffReviewBody.unchangedLines_one', '{{count}} unchanged line', { - count: row.hiddenCount - }) - : t('components.diffReviewBody.unchangedLines_other', '{{count}} unchanged lines', { - count: row.hiddenCount - })} - - - ) : ( - - ) - } - contentContainerStyle={styles.diffList} - ListFooterComponent={ - truncated ? ( - - {t('components.diffReviewBody.diffTruncated', 'Diff truncated for mobile preview.')} - - ) : null - } - /> - ) -} - /** Binary, oversize and deleted diffs: nothing to retry, so no action. */ function DiffUnavailableState({ diffState }: { diffState: ReviewDiffState }) { const t = useMobileTranslation() diff --git a/mobile/src/components/MobileDiffReviewDrawers.tsx b/mobile/src/components/MobileDiffReviewDrawers.tsx index 5dd078d1a55..899bc988d46 100644 --- a/mobile/src/components/MobileDiffReviewDrawers.tsx +++ b/mobile/src/components/MobileDiffReviewDrawers.tsx @@ -19,6 +19,7 @@ import { MessageBox } from './composer-input/MessageBox' import { composerInputStyles } from './composer-input/composer-input-styles' import { hostOs } from '../platform/host-os' import { MobileDiffReviewFilesSheet } from './MobileDiffReviewFilesSheet' +import { MobileDiffReviewRangeSheet } from './MobileDiffReviewRangeSheet' import { mobileDiffReviewFilesSubtitle } from '../session/mobile-diff-review-screen-model' type Props = { @@ -51,7 +52,7 @@ function ReviewSheetContent({ controller, sheet }: Props & { sheet: ReviewSheet const t = useMobileTranslation() switch (sheet.kind) { case 'send': - return + return case 'discard': return ( case 'completion': return + case 'range': + return case 'files': return ( , - load: SendSheetState + load: SendSheetState, + only: readonly string[] | undefined ) { const t = useMobileTranslation() return useMemo(() => { - const comments = controller.unsentComments + const comments = only + ? controller.unsentComments.filter((comment) => only.includes(comment.id)) + : controller.unsentComments const terminalActions = load.kind === 'ready' || load.kind === 'error' ? load.terminals.map((terminal) => ({ @@ -155,12 +165,14 @@ function useSendActions( id: 'copyNotes', label: t('components.diffReviewDrawers.copyNotes', 'Copy Notes'), icon: Copy, - disabled: - controller.screenState.kind !== 'ready' || controller.screenState.comments.length === 0, - onPress: () => void controller.copyNotes() + // A sheet for one new note copies that note, not every note in the workspace. + disabled: only + ? comments.length === 0 + : controller.screenState.kind !== 'ready' || controller.screenState.comments.length === 0, + onPress: () => void controller.copyNotes(only ? comments : undefined) } ] - }, [controller, load, t]) + }, [controller, load, only, t]) } function NoteComposerContent({ controller, composer }: Props & { composer: ComposerState }) { diff --git a/mobile/src/components/MobileDiffReviewFileSummary.tsx b/mobile/src/components/MobileDiffReviewFileSummary.tsx index f1e7388ffc1..75111174494 100644 --- a/mobile/src/components/MobileDiffReviewFileSummary.tsx +++ b/mobile/src/components/MobileDiffReviewFileSummary.tsx @@ -15,6 +15,8 @@ type Props = { staleCommentIds: ReadonlySet onEditNote: (comment: DiffComment) => void onToggleViewed: (item: MobileDiffReviewQueueItem) => void + /** GitHub threads on this file that can't sit under their lines until the branch matches the PR. */ + hiddenThreadCount: number } /** Uncommitted files are staged rather than viewed, and a past commit's files are read-only. */ @@ -36,7 +38,8 @@ export function MobileDiffReviewFileSummary({ item, staleCommentIds, onEditNote, - onToggleViewed + onToggleViewed, + hiddenThreadCount }: Props) { useMobileColorScheme() const t = useMobileTranslation() @@ -62,6 +65,21 @@ export function MobileDiffReviewFileSummary({ /> ) : null} + {hiddenThreadCount > 0 ? ( + + {hiddenThreadCount === 1 + ? t( + 'components.diffReviewFileSummary.threadsWhenInSync_one', + 'Its GitHub comment shows here once this branch matches the pull request.', + { count: hiddenThreadCount } + ) + : t( + 'components.diffReviewFileSummary.threadsWhenInSync_other', + 'Its {{count}} GitHub comments show here once this branch matches the pull request.', + { count: hiddenThreadCount } + )} + + ) : null} {item.oldPath ? ( {t('components.diffReviewFileSummary.renamedFrom', 'Renamed from {{oldPath}}', { diff --git a/mobile/src/components/MobileDiffReviewFilesSheet.tsx b/mobile/src/components/MobileDiffReviewFilesSheet.tsx index 85af7b519f9..0342463ad21 100644 --- a/mobile/src/components/MobileDiffReviewFilesSheet.tsx +++ b/mobile/src/components/MobileDiffReviewFilesSheet.tsx @@ -178,7 +178,8 @@ function FileRow({ [styles.fileMain, pressed && pressedStyle]} > diff --git a/mobile/src/components/MobileDiffReviewLine.tsx b/mobile/src/components/MobileDiffReviewLine.tsx index 11e6d48527c..2260fad38f2 100644 --- a/mobile/src/components/MobileDiffReviewLine.tsx +++ b/mobile/src/components/MobileDiffReviewLine.tsx @@ -1,3 +1,4 @@ +import { memo } from 'react' import { Pressable, StyleSheet, Text, View } from 'react-native' import { MessageSquare } from 'lucide-react-native' import type { Translate } from '../i18n/mobile-i18n' @@ -11,10 +12,15 @@ import { themedStyleSheet } from '../theme/themed-styles' import { useMobileColorScheme } from '../theme/use-mobile-color-scheme' import { MobileSyntaxSegments } from './MobileSyntaxSegments' +// Each pickable line row carries this id, so a drag finds the row under the finger in the page itself. +export const DIFF_LINE_ID_PREFIX = 'orca-review-diff-line-' + type Props = { line: MobileHighlightedDiffLine comments: readonly DiffComment[] staleCommentIds: ReadonlySet + /** Part of the lines picked for a note. */ + selected?: boolean /** Absent where lines take no notes, e.g. a past commit's files. */ onAddNote?: (lineNumber: number) => void onEditNote: (comment: DiffComment) => void @@ -41,10 +47,11 @@ function canCommentOnLine(line: MobileDiffLine): boolean { return line.kind !== 'delete' && line.newLineNumber !== undefined } -export function MobileDiffReviewLine({ +export const MobileDiffReviewLine = memo(function MobileDiffReviewLine({ line, comments, staleCommentIds, + selected = false, onAddNote, onEditNote }: Props) { @@ -58,10 +65,14 @@ export function MobileDiffReviewLine({ style={[ styles.row, line.kind === 'add' && styles.addedRow, - line.kind === 'delete' && styles.deletedRow + line.kind === 'delete' && styles.deletedRow, + selected && styles.selectedRow ]} accessible accessibilityLabel={accessibilityLabelForLine(line, t)} + {...(canComment && line.newLineNumber !== undefined + ? { nativeID: `${DIFF_LINE_ID_PREFIX}${line.newLineNumber}` } + : {})} > {mobileDiffLinePrefix(line.kind)} {lineNumber ? String(lineNumber) : ''} @@ -113,7 +124,7 @@ export function MobileDiffReviewLine({ ) : null} ) -} +}) // Use 18px lineHeight alone without padding/minHeight to match desktop diff density (STA-1239). const styles = themedStyleSheet(() => ({ @@ -129,6 +140,10 @@ const styles = themedStyleSheet(() => ({ deletedRow: { backgroundColor: colors.diffDeletedBg }, + // The same blue tint the composer gives a picked skill. + selectedRow: { + backgroundColor: colors.skillTokenSurface + }, prefix: { width: 18, textAlign: 'center', diff --git a/mobile/src/components/MobileDiffReviewLines.tsx b/mobile/src/components/MobileDiffReviewLines.tsx new file mode 100644 index 00000000000..6bfe6a3b673 --- /dev/null +++ b/mobile/src/components/MobileDiffReviewLines.tsx @@ -0,0 +1,283 @@ +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { FlatList, Platform, Pressable, Text, View } from 'react-native' +import { Gesture, GestureDetector } from 'react-native-gesture-handler' +import { ChevronsUpDown } from 'lucide-react-native' +import type { DiffComment } from '../../../src/shared/diff-comment-types' +import { useMobileColorScheme } from '../theme/use-mobile-color-scheme' +import { useMobileTranslation } from '../i18n/use-mobile-translation' +import { DIFF_LINE_ID_PREFIX, MobileDiffReviewLine } from './MobileDiffReviewLine' +import type { ReviewDiffLine } from '../session/mobile-diff-review-screen-model' +import { + buildMobileDiffReviewRows, + type MobileDiffReviewRow +} from '../session/mobile-diff-review-rows' +import { colors } from '../theme/mobile-theme' +import { mobileDiffReviewStyles as styles } from './mobile-diff-review-screen-styles' +import type { MobileDiffReviewThread } from '../session/mobile-diff-review-github' +import type { PRCommentCardActions } from './pr-sidebar/PRCommentCard' +import { CommentGroupView } from './pr-sidebar/PRCommentsSection' +import { useNow } from '../hooks/use-now' +import { triggerSelection } from '../platform/haptics' + +const NO_OPENED_LINES: ReadonlySet = new Set() + +const RANGE_PICK_LONG_PRESS_MS = 350 +const MAX_SCROLL_ATTEMPTS = 10 +const NO_LINE_NOTES: readonly DiffComment[] = [] +const SCROLL_RETRY_MS = 100 + +export type LineRange = { startLine: number; line: number } + +// Keyed by file, so folds the reader opened close again on the next file. +export function MobileDiffReviewLines({ + lines, + truncated, + commentsByLine, + staleCommentIds, + threadsByLine, + threadActions, + selectedRange, + scrollToLine, + onScrolledToLine, + onPickLines, + onEditNote +}: { + lines: ReviewDiffLine[] + truncated: boolean + commentsByLine: ReadonlyMap + staleCommentIds: ReadonlySet + threadsByLine: ReadonlyMap + threadActions: PRCommentCardActions | undefined + /** The lines the open note sheet is about, kept highlighted under it. */ + selectedRange: LineRange | null + scrollToLine: number | null + onScrolledToLine: () => void + /** Absent where lines take no notes, e.g. a past commit's files. */ + onPickLines?: (range: LineRange) => void + onEditNote: (comment: DiffComment) => void +}) { + useMobileColorScheme() + const t = useMobileTranslation() + const now = useNow(60_000, threadsByLine.size > 0) + // Opened folds are line indexes into this load; a reload with new lines starts folded again. + const [opened, setOpened] = useState<{ lines: ReviewDiffLine[]; indexes: ReadonlySet }>( + () => ({ lines, indexes: new Set() }) + ) + const openedLines = opened.lines === lines ? opened.indexes : NO_OPENED_LINES + const rows = useMemo( + () => + buildMobileDiffReviewRows(lines, openedLines, new Set(commentsByLine.keys()), threadsByLine), + [commentsByLine, lines, openedLines, threadsByLine] + ) + const openFold = (startIndex: number, endIndex: number) => { + const next = new Set(openedLines) + for (let index = startIndex; index < endIndex; index += 1) { + next.add(index) + } + setOpened({ lines, indexes: next }) + } + + const listRef = useRef>>(null) + const scrollFailedRef = useRef(false) + const [scrollAttempt, setScrollAttempt] = useState(0) + useEffect(() => { + if (scrollToLine === null) { + return + } + const index = rows.findIndex( + (row) => + row.kind === 'line' && row.line.kind !== 'delete' && row.line.newLineNumber === scrollToLine + ) + if (index !== -1) { + // An unmeasured row can't be scrolled to yet: jump near it, let it lay out, and try again. + scrollFailedRef.current = false + listRef.current?.scrollToIndex({ index, animated: false, viewPosition: 0.3 }) + if (!scrollFailedRef.current || scrollAttempt >= MAX_SCROLL_ATTEMPTS) { + onScrolledToLine() + return + } + const retry = setTimeout(() => setScrollAttempt((attempt) => attempt + 1), SCROLL_RETRY_MS) + return () => clearTimeout(retry) + } + // A target inside folded context opens its fold first; the next rows then have it. + const lineIndex = lines.findIndex( + (line) => line.kind !== 'delete' && line.newLineNumber === scrollToLine + ) + const fold = rows.find( + (row) => row.kind === 'fold' && lineIndex >= row.startIndex && lineIndex < row.endIndex + ) + if (fold?.kind === 'fold') { + openFold(fold.startIndex, fold.endIndex) + } else { + onScrolledToLine() + } + }, [lines, onScrolledToLine, rows, scrollAttempt, scrollToLine]) + + // Long-press then drag picks lines. The screen only renders as the page (the native route mounts the web shell), + // so the row under the finger comes from the document: no cached layout can go stale as folds and threads move rows. + const [dragRange, setDragRange] = useState<{ anchor: number; line: number } | null>(null) + // The gesture's own copy, read when it ends; the state only paints the highlight. + const dragRangeRef = useRef<{ anchor: number; line: number } | null>(null) + // The release that ends a pick also reaches the starting row as a tap; that tap must not replace the range. + const pickConsumedTapRef = useRef(false) + // Browsers decide on the first touchmove whether to scroll; once a pick is live, claim it so the drag isn't cancelled. + const frameRef = useRef(null) + useEffect(() => { + const node: unknown = frameRef.current + if (Platform.OS !== 'web' || !(node instanceof EventTarget)) { + return + } + const holdTouch = (event: Event) => { + if (dragRangeRef.current) { + event.preventDefault() + } + } + node.addEventListener('touchmove', holdTouch, { passive: false }) + return () => node.removeEventListener('touchmove', holdTouch) + }, []) + const updateDragRange = (next: { anchor: number; line: number } | null) => { + dragRangeRef.current = next + setDragRange(next) + } + const lineAt = (x: number, y: number): number | null => { + if (typeof document === 'undefined') { + return null + } + const row = document.elementFromPoint(x, y)?.closest(`[id^="${DIFF_LINE_ID_PREFIX}"]`) + const line = row ? Number(row.id.slice(DIFF_LINE_ID_PREFIX.length)) : Number.NaN + return Number.isInteger(line) ? line : null + } + const pick = Gesture.Pan() + .enabled(onPickLines !== undefined) + .activateAfterLongPress(RANGE_PICK_LONG_PRESS_MS) + .shouldCancelWhenOutside(false) + .runOnJS(true) + // A new touch starts fresh, so a pick whose release sent no tap can't swallow the next real one. + .onBegin(() => { + pickConsumedTapRef.current = false + }) + .onStart((event) => { + const line = lineAt(event.absoluteX, event.absoluteY) + if (line !== null) { + triggerSelection() + updateDragRange({ anchor: line, line }) + } + }) + .onUpdate((event) => { + const line = lineAt(event.absoluteX, event.absoluteY) + const prev = dragRangeRef.current + if (line !== null && prev && prev.line !== line) { + updateDragRange({ ...prev, line }) + } + }) + .onFinalize((_event, success) => { + const picked = dragRangeRef.current + updateDragRange(null) + if (picked) { + pickConsumedTapRef.current = true + } + // A cancelled drag (the browser took the touch, say) picks nothing. + if (picked && success) { + onPickLines?.({ + startLine: Math.min(picked.anchor, picked.line), + line: Math.max(picked.anchor, picked.line) + }) + } + }) + // Stable, so a drag step re-renders only the rows whose highlight changed. + const pickOneLine = useCallback( + (line: number) => { + if (pickConsumedTapRef.current) { + pickConsumedTapRef.current = false + return + } + onPickLines?.({ startLine: line, line }) + }, + [onPickLines] + ) + const highlighted: LineRange | null = dragRange + ? { + startLine: Math.min(dragRange.anchor, dragRange.line), + line: Math.max(dragRange.anchor, dragRange.line) + } + : selectedRange + + return ( + + + > + ref={listRef} + data={rows} + // The list holds still while lines are being picked, so the drag selects instead of scrolling. + scrollEnabled={dragRange === null} + onScrollToIndexFailed={(info) => { + scrollFailedRef.current = true + listRef.current?.scrollToOffset({ + offset: Math.max(0, info.averageItemLength * info.index), + animated: false + }) + }} + keyExtractor={(row) => row.key} + renderItem={({ item: row }) => + row.kind === 'fold' ? ( + [styles.foldRow, pressed && styles.buttonPressed]} + onPress={() => openFold(row.startIndex, row.endIndex)} + accessibilityRole="button" + accessibilityLabel={t( + 'components.diffReviewBody.showUnchangedLines', + 'Show {{count}} unchanged lines', + { count: row.hiddenCount } + )} + > + + + {row.hiddenCount === 1 + ? t( + 'components.diffReviewBody.unchangedLines_one', + '{{count}} unchanged line', + { + count: row.hiddenCount + } + ) + : t( + 'components.diffReviewBody.unchangedLines_other', + '{{count}} unchanged lines', + { count: row.hiddenCount } + )} + + + ) : row.kind === 'thread' ? ( + + + + ) : ( + = highlighted.startLine && + row.line.newLineNumber <= highlighted.line + } + {...(onPickLines ? { onAddNote: pickOneLine } : {})} + onEditNote={onEditNote} + /> + ) + } + contentContainerStyle={styles.diffList} + ListFooterComponent={ + truncated ? ( + + {t('components.diffReviewBody.diffTruncated', 'Diff truncated for mobile preview.')} + + ) : null + } + /> + + + ) +} diff --git a/mobile/src/components/MobileDiffReviewRangeSheet.tsx b/mobile/src/components/MobileDiffReviewRangeSheet.tsx new file mode 100644 index 00000000000..de1f769f812 --- /dev/null +++ b/mobile/src/components/MobileDiffReviewRangeSheet.tsx @@ -0,0 +1,152 @@ +import { useEffect, useState } from 'react' +import { KeyboardAvoidingView, Pressable, Text, View } from 'react-native' +import { Sparkles } from 'lucide-react-native' +import { getRuntimePathBasename } from '../../../src/shared/cross-platform-path' +import { tuiAgentDisplayName } from '../../../src/shared/tui-agent-display-names' +import { useMobileTranslation } from '../i18n/use-mobile-translation' +import { hostOs } from '../platform/host-os' +import { canCommentOnGitHub } from '../session/mobile-diff-review-github' +import type { useMobileDiffReviewController } from '../session/use-mobile-diff-review-controller' +import { colors } from '../theme/mobile-theme' +import { useMobileColorScheme } from '../theme/use-mobile-color-scheme' +import { TextField } from './composer-input/TextField' +import { mobileDiffReviewStyles as styles } from './mobile-diff-review-screen-styles' +import { SheetHeader } from './nav-bar/SheetHeader' +import type { ReviewSheet } from '../session/mobile-diff-review-sheets' +import type { MobileReviewTerminalTab } from '../session/review-terminal-reply-schema' + +type Props = { + controller: ReturnType +} + +// Lines picked in the diff: a note for the workspace's agent, or a comment straight on GitHub. +export function MobileDiffReviewRangeSheet({ + controller, + range +}: Props & { range: Extract }) { + useMobileColorScheme() + const t = useMobileTranslation() + const item = controller.currentItem + const { agentName, terminals } = useAgentTerminals(controller) + const [busy, setBusy] = useState<'github' | 'agent' | null>(null) + const [postError, setPostError] = useState(null) + const body = controller.composerBody.trim() + const onGitHub = canCommentOnGitHub(controller.github, item, range) + const title = + range.startLine === range.line + ? t('components.diffReviewDrawers.rangeLine', 'Line {{line}}', { line: range.line }) + : t('components.diffReviewDrawers.rangeLines', 'Lines {{startLine}}–{{line}}', { + startLine: range.startLine, + line: range.line + }) + const sendLabel = agentName + ? t('components.diffReviewDrawers.sendToAgent', 'Send to {{agent}}', { agent: agentName }) + : t('components.diffReviewDrawers.sendToAnAgent', 'Send to agent') + return ( + + controller.closeSheet('range', range) + }} + /> + + {postError ? {postError} : null} + + {onGitHub && item ? ( + [ + styles.secondaryButton, + styles.sheetSecondaryButton, + (busy !== null || !body) && styles.buttonDisabled, + pressed && styles.buttonPressed + ]} + disabled={busy !== null || !body} + onPress={() => { + setBusy('github') + setPostError(null) + void controller + .commentOnGitHub({ item, startLine: range.startLine, line: range.line, body }) + .then((posted) => { + if (posted.ok) { + // Only this sheet; one opened since keeps its own draft. + controller.closeSheet('range', range) + } else { + setPostError(posted.error) + } + }) + .finally(() => setBusy(null)) + }} + accessibilityRole="button" + accessibilityLabel={t( + 'components.diffReviewDrawers.commentOnGitHub', + 'Comment on GitHub' + )} + > + + {t('components.diffReviewDrawers.commentOnGitHub', 'Comment on GitHub')} + + + ) : null} + [ + styles.primaryButton, + (busy !== null || !body) && styles.buttonDisabled, + pressed && styles.buttonPressed + ]} + disabled={busy !== null || !body} + onPress={() => { + setBusy('agent') + setPostError(null) + void controller + .sendRange(range, terminals ?? undefined) + .then(setPostError) + .finally(() => setBusy(null)) + }} + accessibilityRole="button" + accessibilityLabel={sendLabel} + > + + + {sendLabel} + + + + + ) +} + +// The workspace's terminals, listed once per sheet: Send reuses them, and names the button after the one agent. +function useAgentTerminals(controller: ReturnType) { + const [terminals, setTerminals] = useState(null) + const loadTerminals = controller.loadTerminals + useEffect(() => { + let active = true + void loadTerminals() + .then((listed) => { + if (active) { + setTerminals(listed) + } + }) + // The button still works without a name; sending lists terminals again and reports its own failure. + .catch(() => {}) + return () => { + active = false + } + }, [loadTerminals]) + const agents = terminals?.filter((terminal) => terminal.agent !== undefined) ?? [] + const [only] = agents + const agentName = + agents.length === 1 && only?.agent ? (tuiAgentDisplayName(only.agent) ?? only.title) : null + return { agentName, terminals } +} diff --git a/mobile/src/components/MobileDiffReviewScreenView.tsx b/mobile/src/components/MobileDiffReviewScreenView.tsx index c2e36dacca7..0d3ce3c65b2 100644 --- a/mobile/src/components/MobileDiffReviewScreenView.tsx +++ b/mobile/src/components/MobileDiffReviewScreenView.tsx @@ -4,6 +4,8 @@ import { Text, View, type LayoutChangeEvent } from 'react-native' import type { useMobileDiffReviewController } from '../session/use-mobile-diff-review-controller' import { useResponsiveLayout } from '../layout/responsive-layout' import { MobileDiffReviewBody } from './MobileDiffReviewBody' +import { useMobilePrCommentActions } from '../session/use-mobile-pr-comment-actions' +import { usePRCommentCardActions } from './pr-sidebar/PRCommentsSection' import { MobileDiffReviewDrawers } from './MobileDiffReviewDrawers' import { MobileDiffReviewFileSummary } from './MobileDiffReviewFileSummary' import { MobileDiffReviewFooter } from './MobileDiffReviewFooter' @@ -47,6 +49,24 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) { }) }) const item = controller.currentItem + const github = controller.github + const refetchPR = controller.refetchPRSidebar + const refetchThreads = useCallback(() => refetchPR({ includeDetails: true }), [refetchPR]) + const threadCommentActions = useMobilePrCommentActions({ + client: controller.client, + connState: controller.connState, + worktreeId: controller.worktreeId, + prNumber: github?.prNumber ?? 0, + prRepo: github?.prRepo ?? null, + refetch: refetchThreads + }) + const threadActions = usePRCommentCardActions( + github ? threadCommentActions : undefined, + github?.prRepo + ) + const sheet = controller.sheet + const selectedRange = + sheet?.kind === 'range' ? { startLine: sheet.startLine, line: sheet.line } : null // Dock GitHub sidebar only when wide; otherwise showPRSidebar toggles RightDrawer overlay. const showInlineDock = presentationMode === 'inline' && controller.prSidebarIsGithubRepo const gitStatus = controller.screenState.kind === 'ready' ? controller.screenState.status : null @@ -93,8 +113,15 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) { staleCommentIds={controller.staleCommentIds} onEditNote={controller.openEditComposer} onToggleViewed={(target) => void controller.toggleItemViewed(target)} + hiddenThreadCount={controller.hiddenThreadCount} /> ) : null} + {/* Each source keeps its own line, so a newer thread failure is never hidden behind older feedback. */} + {threadCommentActions.error ? ( + + {threadCommentActions.error} + + ) : null} {controller.actionError ? ( {controller.actionError} @@ -107,7 +134,15 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) { filteredCount={controller.filteredQueue.length} screenState={controller.screenState} staleCommentIds={controller.staleCommentIds} - onAddNote={controller.openComposer} + threadsByLine={controller.threadsByLine} + threadActions={threadActions} + selectedRange={selectedRange} + onPickLines={(range) => { + controller.setComposerBody('') + controller.openSheet({ kind: 'range', ...range }) + }} + scrollToLine={controller.scrollToLine} + onScrolledToLine={controller.clearScrollToLine} onEditNote={controller.openEditComposer} onRetry={controller.retryAction ?? undefined} offline={controller.connState !== 'connected'} diff --git a/mobile/src/components/mobile-diff-review-control-styles.ts b/mobile/src/components/mobile-diff-review-control-styles.ts index caa657fe406..0a0a156757a 100644 --- a/mobile/src/components/mobile-diff-review-control-styles.ts +++ b/mobile/src/components/mobile-diff-review-control-styles.ts @@ -92,6 +92,15 @@ export const mobileDiffReviewControlStyles = themedStyleSheet(() => ({ fontSize: typography.metaSize, marginTop: 2 }, + rangeField: { + minHeight: 96, + alignItems: 'flex-start' + }, + rangeError: { + marginTop: spacing.sm, + color: colors.statusRed, + fontSize: typography.metaSize + }, drawerButtonRow: { flexDirection: 'row', gap: spacing.sm, diff --git a/mobile/src/components/mobile-diff-review-layout-styles.ts b/mobile/src/components/mobile-diff-review-layout-styles.ts index abda512a45e..8204ed0f69a 100644 --- a/mobile/src/components/mobile-diff-review-layout-styles.ts +++ b/mobile/src/components/mobile-diff-review-layout-styles.ts @@ -68,6 +68,14 @@ export const mobileDiffReviewLayoutStyles = themedStyleSheet(() => ({ color: colors.textMuted, fontSize: typography.metaSize }, + diffListFrame: { + flex: 1 + }, + threadRow: { + paddingHorizontal: spacing.md, + paddingVertical: spacing.sm, + backgroundColor: colors.bgBase + }, diffList: { paddingBottom: 140, backgroundColor: colors.editorSurface diff --git a/mobile/src/components/mobile-diff-review-send-sheet-stacking.test.ts b/mobile/src/components/mobile-diff-review-send-sheet-stacking.test.ts index 20c52b4c90b..6fefe15ce51 100644 --- a/mobile/src/components/mobile-diff-review-send-sheet-stacking.test.ts +++ b/mobile/src/components/mobile-diff-review-send-sheet-stacking.test.ts @@ -52,7 +52,12 @@ vi.mock('../session/mobile-diff-review-loaders', () => ({ loadMobileDiffReviewDiff: vi.fn().mockResolvedValue({ kind: 'idle' }) })) vi.mock('../session/use-mobile-pr-sidebar-controller', () => ({ - useMobilePrSidebarController: () => ({}) + useMobilePrSidebarController: () => ({ + prSidebarState: { kind: 'hidden' }, + prSidebarIsGithubRepo: false, + refetchPRSidebar: async () => {}, + ensurePrSidebarDetails: async () => {} + }) })) const { MobileDiffReviewDrawers } = await import('./MobileDiffReviewDrawers') diff --git a/mobile/src/components/pr-sidebar/PRCommentsSection.tsx b/mobile/src/components/pr-sidebar/PRCommentsSection.tsx index 777a3d595d7..ef4e3f4d3e3 100644 --- a/mobile/src/components/pr-sidebar/PRCommentsSection.tsx +++ b/mobile/src/components/pr-sidebar/PRCommentsSection.tsx @@ -86,24 +86,7 @@ export function PRCommentsSection({ ) const isPr = details != null && !detailsFailed && details.item.type === 'pr' - // Memoize stable per-card callback bundles so unrelated timeline updates skip cards. - const cardActions = useMemo( - () => - actions && isPr - ? { - reply: actions.reply, - toggleResolve: actions.toggleResolve, - editComment: actions.editComment, - deleteComment: actions.deleteComment, - isReplyBusy: actions.isReplyBusy, - isResolveBusy: actions.isResolveBusy, - isEditBusy: actions.isEditBusy, - isDeleteBusy: actions.isDeleteBusy, - prRepo: prRepo ?? null - } - : undefined, - [actions, isPr, prRepo] - ) + const cardActions = usePRCommentCardActions(isPr ? actions : undefined, prRepo) const canComment = isPr && actions !== undefined && canAddRootComment(prState) const [filter, setFilter] = useState('all') @@ -266,7 +249,32 @@ export function PRCommentsSection({ ) } -function CommentGroupView({ +// Memoize stable per-card callback bundles so unrelated timeline updates skip cards. +export function usePRCommentCardActions( + actions: MobilePrCommentActions | undefined, + prRepo: GitHubPrRepoSlug | null | undefined +): PRCommentCardActions | undefined { + return useMemo( + () => + actions + ? { + reply: actions.reply, + toggleResolve: actions.toggleResolve, + editComment: actions.editComment, + deleteComment: actions.deleteComment, + isReplyBusy: actions.isReplyBusy, + isResolveBusy: actions.isResolveBusy, + isEditBusy: actions.isEditBusy, + isDeleteBusy: actions.isDeleteBusy, + prRepo: prRepo ?? null + } + : undefined, + [actions, prRepo] + ) +} + +/** One standalone comment or thread; resolved threads collapse. The diff screen shows threads under their line with it. */ +export function CommentGroupView({ group, actions, now diff --git a/mobile/src/i18n/en-runtime-required.json b/mobile/src/i18n/en-runtime-required.json index e6bced433ff..b29f3e57db1 100644 --- a/mobile/src/i18n/en-runtime-required.json +++ b/mobile/src/i18n/en-runtime-required.json @@ -140,6 +140,10 @@ "viewedFiles_one": "{{count}} file viewed", "viewedFiles_other": "{{count}} files viewed" }, + "diffReviewFileSummary": { + "threadsWhenInSync_one": "Its GitHub comment shows here once this branch matches the pull request.", + "threadsWhenInSync_other": "Its {{count}} GitHub comments show here once this branch matches the pull request." + }, "worktreeAgentList": { "hideChildAgents_one": "Hide {{count}} child agent", "hideChildAgents_other": "Hide {{count}} child agents", diff --git a/mobile/src/i18n/locales/en.json b/mobile/src/i18n/locales/en.json index f386e996a80..aea1f50c985 100644 --- a/mobile/src/i18n/locales/en.json +++ b/mobile/src/i18n/locales/en.json @@ -993,7 +993,13 @@ "unstageFile": "Unstage", "addFileNote": "Add File Note", "pullRequest": "Pull Request", - "discardChanges": "Discard Changes" + "discardChanges": "Discard Changes", + "rangeLine": "Line {{line}}", + "rangeLines": "Lines {{startLine}}–{{line}}", + "sendToAgent": "Send to {{agent}}", + "sendToAnAgent": "Send to agent", + "rangePlaceholder": "What should change here?", + "commentOnGitHub": "Comment on GitHub" }, "diffReviewFileSummary": { "changed": "Changed", @@ -1002,7 +1008,9 @@ "untracked": "Untracked", "notStaged": "Not staged", "viewed": "Viewed", - "renamedFrom": "Renamed from {{oldPath}}" + "renamedFrom": "Renamed from {{oldPath}}", + "threadsWhenInSync_one": "Its GitHub comment shows here once this branch matches the pull request.", + "threadsWhenInSync_other": "Its {{count}} GitHub comments show here once this branch matches the pull request." }, "diffReviewFooter": { "previousFile": "Previous file", @@ -1983,7 +1991,8 @@ }, "githubPrMutations": { "titleUpdateFailed": "Failed to update title.", - "reviewThreadUpdateFailed": "Failed to update review thread." + "reviewThreadUpdateFailed": "Failed to update review thread.", + "fileViewedFailed": "Failed to update viewed state." }, "githubPrRpc": { "requestFailed": "Request failed: {{method}}" @@ -2615,6 +2624,9 @@ "diffFailed": "Couldn't load diff preview", "fileFailed": "Couldn't load file preview", "outsideWorkspace": "This file is outside the workspace. Open it on desktop to view it." + }, + "useDiffReviewGitHub": { + "waitingForDesktop": "Waiting for desktop..." } }, "chat": { diff --git a/mobile/src/session/MobileDiffReviewRouteScreen.tsx b/mobile/src/session/MobileDiffReviewRouteScreen.tsx index e8dd172e892..a33674f4dad 100644 --- a/mobile/src/session/MobileDiffReviewRouteScreen.tsx +++ b/mobile/src/session/MobileDiffReviewRouteScreen.tsx @@ -18,6 +18,7 @@ export function MobileDiffReviewRouteScreen() { file?: string | string[] area?: string | string[] commit?: string | string[] + line?: string | string[] }>() const hostId = firstReviewParam(params.hostId) const worktreeId = firstReviewParam(params.worktreeId) @@ -26,6 +27,8 @@ export function MobileDiffReviewRouteScreen() { const initialArea = normalizeReviewAreaParam(firstReviewParam(params.area)) const initialFilter = initialArea ? scopeForTargetArea(initialArea) : 'all' const commitOid = firstReviewParam(params.commit) || null + const lineParam = Number.parseInt(firstReviewParam(params.line), 10) + const initialLine = Number.isInteger(lineParam) && lineParam > 0 ? lineParam : null const initialTarget = useMemo( () => (initialFile && initialArea ? { filePath: initialFile, area: initialArea } : null), [initialArea, initialFile] @@ -55,6 +58,7 @@ export function MobileDiffReviewRouteScreen() { initialFilter, initialTarget, commitOid, + initialLine, onOpenSession: openSession, onReconnect: forceReconnect }) diff --git a/mobile/src/session/github-pr-entity-reply-schema.ts b/mobile/src/session/github-pr-entity-reply-schema.ts index 32600f99e4e..9863009ac8a 100644 --- a/mobile/src/session/github-pr-entity-reply-schema.ts +++ b/mobile/src/session/github-pr-entity-reply-schema.ts @@ -1,8 +1,9 @@ import { z } from 'zod' -import { salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage' +import { openEnum, salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage' import type { GitHubReaction, PRComment } from '../../../src/shared/github/comment-types' import type { GitHubAssignableUser, + GitHubPRFile, GitHubPRMergeMethodSettings, GitHubPRReviewSummary, GitHubRepositoryIdentity, @@ -239,3 +240,43 @@ export const prChecksSchema = salvagingArray(checkDetailSchema) export function prCheckList(name: string) { return salvagedOptional(name, prChecksSchema) } + +const PR_FILE_STATUS = [ + 'added', + 'modified', + 'removed', + 'renamed', + 'copied', + 'changed', + 'unchanged' +] as const + +// The diff screen reads viewed state and commentable lines per path; a malformed file drops alone. +export const prFileSchema = z + .looseObject({ + path: z.string().min(1), + oldPath: salvagedOptional('oldPath', z.string()), + status: openEnum(PR_FILE_STATUS, 'modified').catch('modified'), + additions: z.number().catch(0), + deletions: z.number().catch(0), + isBinary: z.boolean().catch(false), + reviewCommentLineNumbers: salvagedOptional( + 'reviewCommentLineNumbers', + z.array(z.number().int().positive()) + ), + viewerViewedState: openEnum(['VIEWED', 'UNVIEWED', 'DISMISSED'] as const, undefined) + .optional() + .catch(undefined) + }) + .transform((file): GitHubPRFile => ({ + path: file.path, + ...(file.oldPath ? { oldPath: file.oldPath } : {}), + status: file.status, + additions: file.additions, + deletions: file.deletions, + isBinary: file.isBinary, + ...(file.reviewCommentLineNumbers + ? { reviewCommentLineNumbers: file.reviewCommentLineNumbers } + : {}), + ...(file.viewerViewedState ? { viewerViewedState: file.viewerViewedState } : {}) + })) diff --git a/mobile/src/session/github-pr-mutation-operations.ts b/mobile/src/session/github-pr-mutation-operations.ts index 2730a38d6cc..476474ded95 100644 --- a/mobile/src/session/github-pr-mutation-operations.ts +++ b/mobile/src/session/github-pr-mutation-operations.ts @@ -69,6 +69,11 @@ export const githubPrReviewCommentReplyAdd = mutationStatusOperation( 'github.addPRReviewCommentReply' ) +export const githubPrReviewCommentAdd = mutationStatusOperation( + 'github.add-pr-review-comment', + 'github.addPRReviewComment' +) + export const githubPrIssueCommentAdd = mutationStatusOperation( 'github.add-issue-comment', 'github.addIssueComment' @@ -104,3 +109,14 @@ export const githubPrReviewThreadResolve = bindDeferredRpcOperation( read: rpcResultVariant('pr-mutation-confirmation', githubPrMutationConfirmationSchema) }) ) + +// GitHub's viewed mark answers a bare boolean; false means it was not applied. +export const githubPrFileViewedSet = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'github.set-pr-file-viewed', + method: 'github.setPRFileViewed', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: rpcResultVariant('pr-mutation-confirmation', githubPrMutationConfirmationSchema) + }) +) diff --git a/mobile/src/session/github-pr-read-reply-schema.ts b/mobile/src/session/github-pr-read-reply-schema.ts index a7d8fa01363..40b3286a9c5 100644 --- a/mobile/src/session/github-pr-read-reply-schema.ts +++ b/mobile/src/session/github-pr-read-reply-schema.ts @@ -12,6 +12,7 @@ import { PR_STATE, REVIEW_DECISION, prCheckList, + prFileSchema, prCheckSummary, prCommentList, prCount, @@ -261,7 +262,9 @@ export const githubWorkItemDetailsSchema = z checks: prCheckList('checks'), participants: prUserList('participants'), // Absent assignees means unreported, distinct from a confirmed empty list. - assignees: salvagedOptional('assignees', salvagingArray(z.string())) + assignees: salvagedOptional('assignees', salvagingArray(z.string())), + // Older hosts send no files; the diff screen then keeps viewed state on the phone. + files: salvagedOptional('files', salvagingArray(prFileSchema)) }) .transform((details): GitHubWorkItemDetails | null => details.item === null @@ -275,7 +278,8 @@ export const githubWorkItemDetailsSchema = z pullRequestId: details.pullRequestId, checks: details.checks ?? [], participants: details.participants ?? [], - assignees: details.assignees + assignees: details.assignees, + ...(details.files ? { files: details.files } : {}) } ) .nullable() diff --git a/mobile/src/session/github-pr-review-line-mutations.ts b/mobile/src/session/github-pr-review-line-mutations.ts new file mode 100644 index 00000000000..022b3742bec --- /dev/null +++ b/mobile/src/session/github-pr-review-line-mutations.ts @@ -0,0 +1,71 @@ +import { githubPrFileViewedSet, githubPrReviewCommentAdd } from './github-pr-mutation-operations' +import { + settleGithubPrConfirmation, + settleGithubPrMutation, + type GitHubPrMutationOutcome +} from './github-pr-mutation-outcome' +import { githubPrRequestParams, type GitHubPrRepoSlug } from './github-pr-repo-slug' +import type { RpcOperationSender } from '../transport/rpc-operation-sender' +import { translate } from '../i18n/mobile-i18n' + +// The diff screen's writes on a PR file: a new line comment and its viewed mark. + +// A new line comment on the PR's head commit; GitHub only accepts lines inside the file's diff hunks. +export function fetchAddPRReviewComment( + client: RpcOperationSender, + worktreeId: string, + args: { + prNumber: number + commitId: string + path: string + line: number + startLine?: number + body: string + prRepo?: GitHubPrRepoSlug | null + } +): Promise { + const params: Record = { + prNumber: args.prNumber, + commitId: args.commitId, + path: args.path, + line: args.line, + body: args.body + } + if (args.startLine !== undefined && args.startLine !== args.line) { + params.startLine = args.startLine + } + return settleGithubPrMutation(githubPrReviewCommentAdd, () => + githubPrReviewCommentAdd.request( + client, + githubPrRequestParams(githubPrReviewCommentAdd.operation.method, worktreeId, params, { + prRepo: args.prRepo + }) + ) + ) +} + +export function fetchSetPRFileViewed( + client: RpcOperationSender, + worktreeId: string, + args: { + pullRequestId: string + path: string + viewed: boolean + prRepo?: GitHubPrRepoSlug | null + } +): Promise { + return settleGithubPrConfirmation( + githubPrFileViewedSet, + () => + githubPrFileViewedSet.request( + client, + githubPrRequestParams( + githubPrFileViewedSet.operation.method, + worktreeId, + { pullRequestId: args.pullRequestId, path: args.path, viewed: args.viewed }, + { prRepo: args.prRepo } + ) + ), + translate('session.githubPrMutations.fileViewedFailed', 'Failed to update viewed state.') + ) +} diff --git a/mobile/src/session/mobile-diff-comments.ts b/mobile/src/session/mobile-diff-comments.ts index 2ef2448407e..2dcae2a6ff7 100644 --- a/mobile/src/session/mobile-diff-comments.ts +++ b/mobile/src/session/mobile-diff-comments.ts @@ -1,9 +1,15 @@ import type { DiffComment, DiffReviewScope } from '../../../src/shared/diff-comment-types' +import { + mobileDiffReviewCommentMatchesItem, + type MobileDiffReviewQueueItem +} from './mobile-diff-review-queue' export type CreateMobileDiffCommentInput = { worktreeId: string filePath: string oldPath?: string + /** Inclusive range start, below lineNumber; absent for a single line. */ + startLine?: number lineNumber: number body: string id: string @@ -71,6 +77,9 @@ export function createMobileDiffComment(input: CreateMobileDiffCommentInput): Di filePath: input.filePath, oldPath: input.oldPath, source: 'diff', + ...(input.startLine !== undefined && input.startLine < input.lineNumber + ? { startLine: input.startLine } + : {}), lineNumber: input.lineNumber, body, createdAt: input.createdAt, @@ -133,3 +142,28 @@ export function sendableMobileDiffComments( ): DiffComment[] { return comments.filter((comment) => !comment.sentAt && !sendingIds.has(comment.id)) } + +/** The open file's notes by line, and which were written against an older version of its diff. */ +export function mobileDiffReviewNotesForItem( + comments: readonly DiffComment[], + item: MobileDiffReviewQueueItem | null +): { commentsByLine: Map; staleCommentIds: Set } { + const commentsByLine = new Map() + const staleCommentIds = new Set() + if (!item) { + return { commentsByLine, staleCommentIds } + } + for (const comment of comments) { + if (!mobileDiffReviewCommentMatchesItem(comment, item)) { + continue + } + commentsByLine.set(comment.lineNumber, [ + ...(commentsByLine.get(comment.lineNumber) ?? []), + comment + ]) + if (comment.diffIdentity !== undefined && comment.diffIdentity !== item.diffIdentity) { + staleCommentIds.add(comment.id) + } + } + return { commentsByLine, staleCommentIds } +} diff --git a/mobile/src/session/mobile-diff-review-github.test.ts b/mobile/src/session/mobile-diff-review-github.test.ts new file mode 100644 index 00000000000..c4b7d46ba12 --- /dev/null +++ b/mobile/src/session/mobile-diff-review-github.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it } from 'vitest' +import type { PRComment } from '../../../src/shared/github/comment-types' +import type { GitHubWorkItemDetails } from '../../../src/shared/github/work-item-types' +import { buildMobileDiffReviewQueue } from './mobile-diff-review-queue' +import { + applyMobileDiffReviewGitHubViewed, + buildMobileDiffReviewGitHub, + canCommentOnGitHub +} from './mobile-diff-review-github' + +function comment(overrides: Partial): PRComment { + return { + id: 1, + author: 'nwparker', + authorAvatarUrl: '', + body: 'Use radii.surface here.', + createdAt: '2026-11-01T00:00:00Z', + url: 'https://github.com/o/r/pull/1#discussion_r1', + ...overrides + } +} + +function details(overrides: Partial): GitHubWorkItemDetails { + return { + item: { + id: 'pr-1', + type: 'pr', + number: 1, + title: 'PR', + state: 'open', + url: 'https://github.com/o/r/pull/1', + labels: [], + updatedAt: '2026-11-01T00:00:00Z', + author: 'kelvin' + }, + body: '', + comments: [], + headSha: 'head', + pullRequestId: 'PR_node', + files: [ + { + path: 'src/a.ts', + status: 'modified', + additions: 2, + deletions: 1, + isBinary: false, + reviewCommentLineNumbers: [10, 11, 12], + viewerViewedState: 'DISMISSED' + } + ], + ...overrides + } +} + +const branchItem = buildMobileDiffReviewQueue({ + worktreeId: 'wt', + statusEntries: [], + branchEntries: [{ path: 'src/a.ts', status: 'modified', added: 2, removed: 1 }], + comments: [], + reviewState: { version: 1, files: {} } +}) + +describe('the diff screen’s GitHub pull request', () => { + it('puts each current thread under its end line and leaves outdated ones out', () => { + const github = buildMobileDiffReviewGitHub({ + prNumber: 1, + prRepo: null, + localHeadOid: 'head', + details: details({ + comments: [ + comment({ id: 1, path: 'src/a.ts', line: 12, startLine: 10, threadId: 'T1' }), + comment({ id: 2, path: 'src/a.ts', line: 12, threadId: 'T1' }), + comment({ id: 3, path: 'src/a.ts', line: 4, threadId: 'T2', isOutdated: true }) + ] + }) + }) + const threads = github.threadsByPath.get('src/a.ts') + expect([...(threads?.keys() ?? [])]).toEqual([12]) + expect(threads?.get(12)?.[0]?.replies.map((reply) => reply.id)).toEqual([2]) + }) + + it('takes viewed state for the PR’s files from GitHub', () => { + const github = buildMobileDiffReviewGitHub({ + prNumber: 1, + prRepo: null, + localHeadOid: 'head', + details: details({}) + }) + expect(applyMobileDiffReviewGitHubViewed(branchItem, github)[0]).toMatchObject({ + isReviewed: false, + changedSinceReview: true + }) + }) + + it('leaves viewed state on the phone when GitHub sent none for the file', () => { + const github = buildMobileDiffReviewGitHub({ + prNumber: 1, + prRepo: null, + localHeadOid: 'head', + details: details({ + pullRequestId: undefined, + files: [ + { path: 'src/a.ts', status: 'modified', additions: 2, deletions: 1, isBinary: false } + ] + }) + }) + const viewedLocally = branchItem.map((item) => ({ ...item, isReviewed: true })) + expect(applyMobileDiffReviewGitHubViewed(viewedLocally, github)[0]?.isReviewed).toBe(true) + }) + + it('offers Comment on GitHub only on commentable lines while the branch is at the PR head', () => { + const atHead = buildMobileDiffReviewGitHub({ + prNumber: 1, + prRepo: null, + localHeadOid: 'head', + details: details({}) + }) + const item = branchItem[0] ?? null + expect(canCommentOnGitHub(atHead, item, { startLine: 10, line: 12 })).toBe(true) + expect(canCommentOnGitHub(atHead, item, { startLine: 12, line: 13 })).toBe(false) + const ahead = buildMobileDiffReviewGitHub({ + prNumber: 1, + prRepo: null, + localHeadOid: 'unpushed', + details: details({}) + }) + expect(ahead.linesMatch).toBe(false) + expect(canCommentOnGitHub(ahead, item, { startLine: 10, line: 10 })).toBe(false) + }) +}) diff --git a/mobile/src/session/mobile-diff-review-github.ts b/mobile/src/session/mobile-diff-review-github.ts new file mode 100644 index 00000000000..9a257af5398 --- /dev/null +++ b/mobile/src/session/mobile-diff-review-github.ts @@ -0,0 +1,136 @@ +import type { GitHubPRFile } from '../../../src/shared/github/pull-request-types' +import type { GitHubWorkItemDetails } from '../../../src/shared/github/work-item-types' +import { getPRInlineCommentPlacement } from '../../../src/shared/github/pr-inline-comment-placement' +import { groupPRComments, type PRCommentGroup } from '../../../src/shared/pr-comment-groups' +import type { GitHubPrRepoSlug } from './github-pr-rpc' +import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue' + +export type MobileDiffReviewThread = Extract + +/** What the diff screen knows about the branch's GitHub pull request. */ +export type MobileDiffReviewGitHub = { + prNumber: number + prRepo: GitHubPrRepoSlug | null + pullRequestId: string | null + headSha: string | null + /** + * The local branch is at the PR's head, so local line numbers are GitHub's. Desktop shows threads + * only on the PR's own contents; the local diff is those contents only when the heads match. + */ + linesMatch: boolean + /** Null on hosts that don't send PR files; viewed state then stays on the phone. */ + filesByPath: ReadonlyMap | null + threadsByPath: ReadonlyMap> +} + +export function buildMobileDiffReviewGitHub(input: { + prNumber: number + prRepo: GitHubPrRepoSlug | null + details: GitHubWorkItemDetails + localHeadOid: string | null +}): MobileDiffReviewGitHub { + const { details } = input + const headSha = details.headSha ?? null + const threadsByPath = new Map>() + for (const group of groupPRComments(details.comments)) { + if (group.kind !== 'thread') { + continue + } + // Desktop's rule: outdated threads and file-level comments don't sit under a line. + const placement = getPRInlineCommentPlacement(group.root) + if (!placement) { + continue + } + const byLine = threadsByPath.get(placement.path) ?? new Map() + byLine.set(placement.line, [...(byLine.get(placement.line) ?? []), group]) + threadsByPath.set(placement.path, byLine) + } + return { + prNumber: input.prNumber, + prRepo: input.prRepo, + pullRequestId: details.pullRequestId ?? null, + headSha, + linesMatch: headSha !== null && headSha === input.localHeadOid, + filesByPath: details.files ? new Map(details.files.map((file) => [file.path, file])) : null, + threadsByPath + } +} + +/** + * The PR file whose viewed state GitHub owns: GitHub reported it, the PR can be marked, and the + * local diff is the PR's version (the threads rule); otherwise the phone keeps its own state. + */ +export function gitHubViewedFile( + github: MobileDiffReviewGitHub | null, + item: MobileDiffReviewQueueItem +): GitHubPRFile | null { + if (!github?.pullRequestId || !github.linesMatch || item.scope !== 'branch') { + return null + } + const file = github.filesByPath?.get(item.filePath) + return file?.viewerViewedState !== undefined ? file : null +} + +/** GitHub owns viewed state for the PR's files; DISMISSED means it changed after it was viewed. */ +export function applyMobileDiffReviewGitHubViewed( + queue: readonly MobileDiffReviewQueueItem[], + github: MobileDiffReviewGitHub | null +): MobileDiffReviewQueueItem[] { + return queue.map((item) => { + const file = gitHubViewedFile(github, item) + return file + ? { + ...item, + isReviewed: file.viewerViewedState === 'VIEWED', + changedSinceReview: file.viewerViewedState === 'DISMISSED' + } + : item + }) +} + +/** Whether GitHub takes a review comment on every line of the range: only lines inside the file's diff hunks. */ +export function canCommentOnGitHub( + github: MobileDiffReviewGitHub | null, + item: MobileDiffReviewQueueItem | null, + range: { startLine: number; line: number } +): boolean { + if (!github?.linesMatch || !github.headSha || !item || item.scope !== 'branch') { + return false + } + const commentable = github.filesByPath?.get(item.filePath)?.reviewCommentLineNumbers + if (!commentable) { + return false + } + const lines = new Set(commentable) + for (let line = range.startLine; line <= range.line; line += 1) { + if (!lines.has(line)) { + return false + } + } + return true +} + +const NO_THREADS: ReadonlyMap = new Map() + +/** The open file's threads by line, or how many are waiting until the branch matches the PR head. */ +export function mobileDiffReviewThreadsForItem( + github: MobileDiffReviewGitHub | null, + item: MobileDiffReviewQueueItem | null +): { + threadsByLine: ReadonlyMap + hiddenThreadCount: number +} { + const byLine = + github && item?.scope === 'branch' ? github.threadsByPath.get(item.filePath) : undefined + if (!byLine) { + return { threadsByLine: NO_THREADS, hiddenThreadCount: 0 } + } + if (github?.linesMatch) { + return { threadsByLine: byLine, hiddenThreadCount: 0 } + } + let hiddenThreadCount = 0 + for (const threads of byLine.values()) { + hiddenThreadCount += threads.length + } + return { threadsByLine: NO_THREADS, hiddenThreadCount } +} diff --git a/mobile/src/session/mobile-diff-review-retry.test.tsx b/mobile/src/session/mobile-diff-review-retry.test.tsx index 105faef663b..0b49e325f4b 100644 --- a/mobile/src/session/mobile-diff-review-retry.test.tsx +++ b/mobile/src/session/mobile-diff-review-retry.test.tsx @@ -17,6 +17,7 @@ vi.mock('react-native', () => ({ View: 'View' })) vi.mock('lucide-react-native', () => ({ + ChevronsUpDown: 'Icon', CircleAlert: 'Icon', FileText: 'Icon', ListChecks: 'Icon', @@ -24,6 +25,8 @@ vi.mock('lucide-react-native', () => ({ WifiOff: 'Icon' })) vi.mock('../components/MobileDiffReviewLine', () => ({ MobileDiffReviewLine: () => null })) +vi.mock('react-native-gesture-handler', () => ({ Gesture: {}, GestureDetector: () => null })) +vi.mock('../components/pr-sidebar/PRCommentsSection', () => ({ CommentGroupView: () => null })) vi.mock('expo-haptics', () => ({ impactAsync: vi.fn(), notificationAsync: vi.fn(), @@ -79,7 +82,12 @@ function ReviewWhileUnreachable({ filteredCount: controller.filteredQueue.length, screenState: controller.screenState, staleCommentIds: controller.staleCommentIds, - onAddNote: controller.openComposer, + threadsByLine: new Map(), + threadActions: undefined, + selectedRange: null, + onPickLines: () => {}, + scrollToLine: null, + onScrolledToLine: () => {}, onEditNote: controller.openEditComposer, onRetry: controller.retryAction ?? undefined, offline: connState !== 'connected', diff --git a/mobile/src/session/mobile-diff-review-rows.test.ts b/mobile/src/session/mobile-diff-review-rows.test.ts index d815a78afd6..88cef0323ee 100644 --- a/mobile/src/session/mobile-diff-review-rows.test.ts +++ b/mobile/src/session/mobile-diff-review-rows.test.ts @@ -12,7 +12,11 @@ function lines(pattern: string): MobileDiffLine[] { function shape(pattern: string, opened: number[] = []): string[] { return buildMobileDiffReviewRows(lines(pattern), new Set(opened)).map((row) => - row.kind === 'fold' ? `fold ${row.hiddenCount}` : row.line.kind.charAt(0) + row.kind === 'fold' + ? `fold ${row.hiddenCount}` + : row.kind === 'line' + ? row.line.kind.charAt(0) + : 'thread' ) } @@ -60,4 +64,38 @@ describe('buildMobileDiffReviewRows', () => { 'fold 7' ]) }) + + it('puts a thread under its line and keeps that line out of folds', () => { + const thread = { + kind: 'thread' as const, + threadId: 'T1', + root: { + id: 1, + author: 'nwparker', + authorAvatarUrl: '', + body: 'Use radii.surface here.', + createdAt: '2026-11-01T00:00:00Z', + url: 'https://github.com/o/r/pull/1' + }, + replies: [] + } + const rows = buildMobileDiffReviewRows( + lines('.'.repeat(20)), + new Set(), + new Set(), + new Map([[10, [thread]]]) + ) + expect(rows.map((row) => (row.kind === 'line' ? row.line.newLineNumber : row.kind))).toEqual([ + 'fold', + 7, + 8, + 9, + 10, + 'thread', + 11, + 12, + 13, + 'fold' + ]) + }) }) diff --git a/mobile/src/session/mobile-diff-review-rows.ts b/mobile/src/session/mobile-diff-review-rows.ts index 3fc411a82ed..9d140cd4881 100644 --- a/mobile/src/session/mobile-diff-review-rows.ts +++ b/mobile/src/session/mobile-diff-review-rows.ts @@ -1,4 +1,5 @@ import type { MobileDiffLine } from './mobile-diff-lines' +import type { MobileDiffReviewThread } from './mobile-diff-review-github' // Monaco's hideUnchangedRegions defaults, which desktop's diff uses: 3 lines of context each side, fold 3 or more. const CONTEXT_LINES = 3 @@ -7,6 +8,7 @@ const MINIMUM_FOLDED_LINES = 3 export type MobileDiffReviewRow = | { kind: 'line'; key: string; line: Line; lineIndex: number } | { kind: 'fold'; key: string; hiddenCount: number; startIndex: number; endIndex: number } + | { kind: 'thread'; key: string; thread: MobileDiffReviewThread } /** * Unchanged runs fold into one row each. Lines the reader opened stay open even when a later @@ -15,7 +17,9 @@ export type MobileDiffReviewRow = export function buildMobileDiffReviewRows( lines: readonly Line[], openedLineIndexes: ReadonlySet, - annotatedLineNumbers: ReadonlySet = new Set() + annotatedLineNumbers: ReadonlySet = new Set(), + /** GitHub threads by the new-side line they end on; each renders under that line. */ + threadsByLine: ReadonlyMap = new Map() ): MobileDiffReviewRow[] { // A note on an unchanged line anchors it like a change, so the note is never hidden in a fold. const foldable = (lineIndex: number) => { @@ -23,14 +27,24 @@ export function buildMobileDiffReviewRows( return ( line?.kind === 'context' && !openedLineIndexes.has(lineIndex) && - (line.newLineNumber === undefined || !annotatedLineNumbers.has(line.newLineNumber)) + (line.newLineNumber === undefined || + (!annotatedLineNumbers.has(line.newLineNumber) && !threadsByLine.has(line.newLineNumber))) ) } const rows: MobileDiffReviewRow[] = [] const pushLine = (lineIndex: number) => { const line = lines[lineIndex] - if (line) { - rows.push({ kind: 'line', key: `line:${lineIndex}`, line, lineIndex }) + if (!line) { + return + } + rows.push({ kind: 'line', key: `line:${lineIndex}`, line, lineIndex }) + // Threads sit on the new side, so a deleted line never carries one. + const threads = + line.kind !== 'delete' && line.newLineNumber !== undefined + ? threadsByLine.get(line.newLineNumber) + : undefined + for (const thread of threads ?? []) { + rows.push({ kind: 'thread', key: `thread:${thread.threadId}`, thread }) } } let index = 0 diff --git a/mobile/src/session/mobile-diff-review-sheets.ts b/mobile/src/session/mobile-diff-review-sheets.ts index 62240aa9060..ac97c8884c6 100644 --- a/mobile/src/session/mobile-diff-review-sheets.ts +++ b/mobile/src/session/mobile-diff-review-sheets.ts @@ -2,11 +2,14 @@ import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue' import type { ComposerState, SendSheetState } from './mobile-diff-review-screen-model' export type ReviewSheet = - | { kind: 'send'; load: SendSheetState } + /** `only` limits the sheet to these notes, e.g. the one just written. */ + | { kind: 'send'; load: SendSheetState; only?: readonly string[] } | { kind: 'discard'; target: MobileDiffReviewQueueItem } | { kind: 'composer'; composer: ComposerState } | { kind: 'completion' } | { kind: 'files' } + /** Lines picked in the diff, inclusive, waiting for a note to send or post. */ + | { kind: 'range'; startLine: number; line: number } export type ReviewSheetKind = ReviewSheet['kind'] @@ -19,7 +22,8 @@ export type ReviewSheetsState = { export type ReviewSheetsAction = | { type: 'open'; sheet: ReviewSheet } | { type: 'openWhenIdle'; sheet: ReviewSheet } - | { type: 'close'; kind: ReviewSheetKind } + /** With `sheet`, closes only that very presentation, so a slow completion can't close a newer one. */ + | { type: 'close'; kind: ReviewSheetKind; sheet?: ReviewSheet } | { type: 'updateSend'; load: SendSheetState } export const NO_REVIEW_SHEETS: ReviewSheetsState = { requested: null, deferred: null } @@ -46,6 +50,9 @@ export function reduceReviewSheets( } return { requested, deferred: action.sheet } case 'close': + if (action.sheet && requested !== action.sheet && deferred !== action.sheet) { + return state + } if (requested?.kind === action.kind) { return { requested: deferred, deferred: null } } @@ -58,7 +65,7 @@ export function reduceReviewSheets( if (requested?.kind !== 'send') { return state } - return { requested: { kind: 'send', load: action.load }, deferred } + return { requested: { ...requested, load: action.load }, deferred } } } @@ -71,7 +78,8 @@ export function reviewSheetIntents(dispatch: (action: ReviewSheetsAction) => voi openSheet: (sheet: ReviewSheet) => dispatch({ type: 'open', sheet }), /** For async openers: waits for the user's sheet to close instead of closing it. */ openSheetWhenIdle: (sheet: ReviewSheet) => dispatch({ type: 'openWhenIdle', sheet }), - closeSheet: (kind: ReviewSheetKind) => dispatch({ type: 'close', kind }), + closeSheet: (kind: ReviewSheetKind, sheet?: ReviewSheet) => + dispatch({ type: 'close', kind, ...(sheet ? { sheet } : {}) }), updateSendSheet: (load: SendSheetState) => dispatch({ type: 'updateSend', load }) } } diff --git a/mobile/src/session/review-terminal-reply-schema.ts b/mobile/src/session/review-terminal-reply-schema.ts index 4fd74ea7fac..10858270578 100644 --- a/mobile/src/session/review-terminal-reply-schema.ts +++ b/mobile/src/session/review-terminal-reply-schema.ts @@ -1,5 +1,11 @@ import { z } from 'zod' import { salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage' +import { AGENT_STATUS_STATES } from '../../../src/shared/agent-status-types' +import { isTuiAgent } from '../../../src/shared/tui-agent-config' +import { + isMobileTerminalTabRunningAgent, + resolveMobileTerminalTabOwnedAgentId +} from './mobile-terminal-tab-agent' // Require addressable terminal rows; salvage/drop other tab kinds rather than inventing send targets. const reviewTerminalTabSchema = z @@ -7,9 +13,36 @@ const reviewTerminalTabSchema = z type: z.literal('terminal'), id: z.string().min(1), terminal: z.string().min(1), - title: salvagedOptional('title', z.string()) + title: salvagedOptional('title', z.string()), + // Which agent runs in it, so a note can go straight to the only one; older hosts send neither. + launchAgent: salvagedOptional('launchAgent', z.string()), + agentStatus: salvagedOptional( + 'agentStatus', + z + .looseObject({ + agentType: z.string().optional().catch(undefined), + state: z.enum(AGENT_STATUS_STATES).optional().catch(undefined) + }) + .nullable() + ) + }) + .transform((tab) => { + // The session strip's rule: an agent that finished and left a shell behind no longer counts. + const identity = { + title: tab.title ?? '', + agentStatus: tab.agentStatus ?? null, + launchAgent: tab.launchAgent && isTuiAgent(tab.launchAgent) ? tab.launchAgent : null + } + const agent = isMobileTerminalTabRunningAgent(identity) + ? resolveMobileTerminalTabOwnedAgentId(identity) + : null + return { + id: tab.id, + terminal: tab.terminal, + title: tab.title ?? 'Terminal', + ...(agent ? { agent } : {}) + } }) - .transform((tab) => ({ id: tab.id, terminal: tab.terminal, title: tab.title ?? 'Terminal' })) // Require an inventory array to distinguish malformed replies from no sessions; salvage valid rows independently. export const reviewTerminalTabsSchema = z diff --git a/mobile/src/session/session-reply-schema.test.ts b/mobile/src/session/session-reply-schema.test.ts index f0ebbc04ba6..402db2365a1 100644 --- a/mobile/src/session/session-reply-schema.test.ts +++ b/mobile/src/session/session-reply-schema.test.ts @@ -121,6 +121,25 @@ describe('required members', () => { expect(refuses(reviewTerminalTabsSchema, { tabs: 'none' })).toBe(true) }) + it('keeps which agent a terminal runs, so a note can go straight to the only one', () => { + const tabs = reads(reviewTerminalTabsSchema, { + tabs: [ + { id: 'tab-1', type: 'terminal', terminal: 't-1', launchAgent: 'claude' }, + { id: 'tab-2', type: 'terminal', terminal: 't-2', agentStatus: { agentType: 'codex' } }, + { id: 'tab-3', type: 'terminal', terminal: 't-3', agentStatus: null }, + { + id: 'tab-4', + type: 'terminal', + terminal: 't-4', + title: 'zsh', + launchAgent: 'claude', + agentStatus: { agentType: 'claude', state: 'done' } + } + ] + }) + expect(tabs?.map((tab) => tab.agent)).toEqual(['claude', 'codex', undefined, undefined]) + }) + it('requires the three compare members every summary line reads', () => { const summary = { baseRef: 'origin/main', compareRef: 'feature', changedFiles: 2 } expect(reads(branchCompareProjectionSchema, { summary, entries: [] }).summary.baseRef).toBe( @@ -363,6 +382,40 @@ describe('closed arm sets drop rather than default', () => { }) expect(details?.comments[0]?.reactions).toEqual([{ content: '+1', count: 2 }]) }) + + it('keeps the PR files the diff screen reads viewed state and commentable lines from', () => { + const details = reads(githubWorkItemDetailsSchema, { + item: { id: 'i-1', number: 7, type: 'pr', state: 'open' }, + files: [ + { + path: 'src/a.ts', + status: 'modified', + additions: 2, + deletions: 1, + isBinary: false, + reviewCommentLineNumbers: [3, 4], + viewerViewedState: 'DISMISSED' + }, + { path: 'src/b.ts', status: 'moved', viewerViewedState: 'SOMETIMES' }, + { status: 'added' } + ] + }) + expect(details?.files).toEqual([ + { + path: 'src/a.ts', + status: 'modified', + additions: 2, + deletions: 1, + isBinary: false, + reviewCommentLineNumbers: [3, 4], + viewerViewedState: 'DISMISSED' + }, + { path: 'src/b.ts', status: 'modified', additions: 0, deletions: 0, isBinary: false } + ]) + expect( + reads(githubWorkItemDetailsSchema, { item: { id: 'i-1', number: 7, type: 'pr' } })?.files + ).toBeUndefined() + }) }) // Preserve null as explicit no; undefined means the host omitted the flag. diff --git a/mobile/src/session/use-mobile-diff-review-comment-actions.ts b/mobile/src/session/use-mobile-diff-review-comment-actions.ts index 0a4c69a27e2..b2e90249552 100644 --- a/mobile/src/session/use-mobile-diff-review-comment-actions.ts +++ b/mobile/src/session/use-mobile-diff-review-comment-actions.ts @@ -40,6 +40,8 @@ type CommentActionsInput = { selectIndex: (index: number) => void /** The reader's navigation count; a save that outlasts a newer pick leaves that pick alone. */ navigationRef: { readonly current: number } + /** GitHub's viewed mark for a PR file, or null when the file's viewed state lives on the phone. */ + remoteViewed: (item: MobileDiffReviewQueueItem, viewed: boolean) => Promise | null setComposerBody: Dispatch> setActionError: Dispatch> sheets: Pick @@ -60,6 +62,7 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) { setScreenState, selectIndex, navigationRef, + remoteViewed, setComposerBody, setActionError, sheets @@ -219,24 +222,47 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) { closeComposer() }, [closeComposer, composer, saveCommentsAndReviewState, screenState]) + // Resolves whether the file is now viewed: on GitHub for a PR file, otherwise in the phone's review state. + const setItemViewed = useCallback( + async (item: MobileDiffReviewQueueItem, viewed: boolean): Promise => { + const remote = remoteViewed(item, viewed) + if (remote) { + return remote + } + const descriptor = reviewDescriptorFromItem(item) + if (!descriptor || screenState.kind !== 'ready') { + return false + } + const now = Date.now() + let nextReviewState = viewed + ? markMobileDiffReviewFileReviewed(screenState.reviewState, descriptor, now) + : { + ...clearMobileDiffReviewFileReviewed(screenState.reviewState, item.key, now), + completedAt: undefined + } + if ( + viewed && + queue.every((candidate) => candidate.key === item.key || candidate.isReviewed) + ) { + nextReviewState = completeMobileDiffReviewState(nextReviewState, now) + } + try { + await saveCommentsAndReviewState(screenState.comments, nextReviewState) + return true + } catch { + // The failed save already rolled back and showed its error. + return false + } + }, + [queue, remoteViewed, saveCommentsAndReviewState, screenState] + ) + const markReviewed = useCallback(async () => { - const descriptor = currentItem ? reviewDescriptorFromItem(currentItem) : null - if (!currentItem || !descriptor || screenState.kind !== 'ready') { + if (!currentItem) { return } - const now = Date.now() - let nextReviewState = markMobileDiffReviewFileReviewed(screenState.reviewState, descriptor, now) - if (queue.every((item) => item.key === currentItem.key || item.isReviewed)) { - nextReviewState = completeMobileDiffReviewState(nextReviewState, now) - } const navigation = navigationRef.current - try { - await saveCommentsAndReviewState(screenState.comments, nextReviewState) - } catch { - // The failed save already rolled back and showed its error. - return - } - if (navigation !== navigationRef.current) { + if (!(await setItemViewed(currentItem, true)) || navigation !== navigationRef.current) { return } const nextIndex = nextReviewIndexAfterMarkReviewed({ @@ -256,29 +282,15 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) { filteredQueue, navigationRef, openSheetWhenIdle, - queue, - saveCommentsAndReviewState, - screenState, - selectIndex + selectIndex, + setItemViewed ]) const toggleItemViewed = useCallback( async (item: MobileDiffReviewQueueItem) => { - const descriptor = reviewDescriptorFromItem(item) - if (!descriptor || screenState.kind !== 'ready') { - return - } - const now = Date.now() - const nextReviewState = item.isReviewed - ? { - ...clearMobileDiffReviewFileReviewed(screenState.reviewState, item.key, now), - completedAt: undefined - } - : markMobileDiffReviewFileReviewed(screenState.reviewState, descriptor, now) - // A failed save already rolled back and showed its error. - await saveCommentsAndReviewState(screenState.comments, nextReviewState).catch(() => {}) + await setItemViewed(item, !item.isReviewed) }, - [saveCommentsAndReviewState, screenState] + [setItemViewed] ) return { diff --git a/mobile/src/session/use-mobile-diff-review-controller.ts b/mobile/src/session/use-mobile-diff-review-controller.ts index 849cf2ff920..9469f69078c 100644 --- a/mobile/src/session/use-mobile-diff-review-controller.ts +++ b/mobile/src/session/use-mobile-diff-review-controller.ts @@ -10,7 +10,6 @@ import { getUnsentMobileDiffComments } from './mobile-diff-comment-edit' import { buildMobileDiffReviewQueue, filterMobileDiffReviewQueue, - mobileDiffReviewCommentMatchesItem, type MobileDiffReviewQueueFilter } from './mobile-diff-review-queue' import { useMobileDiffReviewSelection } from './use-mobile-diff-review-selection' @@ -29,6 +28,11 @@ import { import { useMobileDiffReviewInteractions } from './use-mobile-diff-review-interactions' import { resolveMobileAgentLaunchAvailability } from './mobile-agent-launch-availability' import { useMobilePrSidebarController } from './use-mobile-pr-sidebar-controller' +import { useMobileDiffReviewGitHub } from './use-mobile-diff-review-github' +import { mobileDiffReviewThreadsForItem } from './mobile-diff-review-github' +import { mobileDiffReviewNotesForItem } from './mobile-diff-comments' + +const NO_COMMENTS: readonly DiffComment[] = [] type ControllerInput = { client: RpcClient | null @@ -43,6 +47,8 @@ type ControllerInput = { initialTarget: MobileDiffReviewInitialTarget | null /** Shows this commit's files instead of the worktree's changes. */ commitOid: string | null + /** A line of the tapped file to scroll to once, e.g. from a review thread's "Open in diff". */ + initialLine?: number | null onOpenSession: () => void /** Null on the page, where the shell owns the connection. */ onReconnect: ((hostId: string) => void | Promise) | null @@ -62,6 +68,7 @@ export function useMobileDiffReviewController(input: ControllerInput) { initialFilter, initialTarget, commitOid, + initialLine = null, onOpenSession, onReconnect } = input @@ -70,6 +77,8 @@ export function useMobileDiffReviewController(input: ControllerInput) { // Bumped by every snapshot load (open, retry, after a git action), so the open file's diff reloads with it. const [snapshotVersion, setSnapshotVersion] = useState(0) const filter = initialFilter + const [pendingLine, setPendingLine] = useState(initialLine) + const clearScrollToLine = useCallback(() => setPendingLine(null), []) const [sheets, dispatchSheets] = useReducer(reduceReviewSheets, NO_REVIEW_SHEETS) const sheetIntents = useMemo(() => reviewSheetIntents(dispatchSheets), []) const composer = reviewComposer(sheets) @@ -147,6 +156,30 @@ export function useMobileDiffReviewController(input: ControllerInput) { void loadReviewData() }, [loadReviewData]) + // Use git.status for PR head identity; branchCompare’s base ref is not the head. + const prSidebarBranch = screenState.kind === 'ready' ? (screenState.status.branch ?? null) : null + const prSidebarHeadSha = + screenState.kind === 'ready' + ? (screenState.status.head ?? screenState.branchCompare?.summary.headOid ?? null) + : null + const prSidebar = useMobilePrSidebarController({ + client, + connState, + worktreeId, + branch: prSidebarBranch, + headSha: prSidebarHeadSha + }) + + const { github, applyViewed, setRemoteViewed, commentOnGitHub } = useMobileDiffReviewGitHub({ + client, + connState, + worktreeId, + filter, + screenState, + prSidebar, + setActionError + }) + const queue = useMemo(() => { if (screenState.kind !== 'ready') { return [] @@ -155,7 +188,7 @@ export function useMobileDiffReviewController(input: ControllerInput) { screenState.branchCompare && canOpenMobileBranchCompareDiff(screenState.branchCompare.summary) ? (screenState.branchCompare.entries ?? []) : [] - return buildMobileDiffReviewQueue({ + const built = buildMobileDiffReviewQueue({ worktreeId, statusEntries: screenState.status.entries, branchEntries, @@ -165,7 +198,8 @@ export function useMobileDiffReviewController(input: ControllerInput) { comments: screenState.comments, reviewState: screenState.reviewState }) - }, [screenState, worktreeId]) + return applyViewed(built) + }, [applyViewed, screenState, worktreeId]) const filteredQueue = useMemo(() => filterMobileDiffReviewQueue(queue, filter), [filter, queue]) const { currentIndex, currentItem, selectIndex, selectKey, moveSelection } = @@ -186,54 +220,12 @@ export function useMobileDiffReviewController(input: ControllerInput) { snapshotVersion }) - const commentsForCurrentItem = useMemo(() => { - if (!currentItem || screenState.kind !== 'ready') { - return [] - } - return screenState.comments.filter((comment) => - mobileDiffReviewCommentMatchesItem(comment, currentItem) - ) - }, [currentItem, screenState]) - - const staleCommentIds = useMemo( - () => - new Set( - commentsForCurrentItem - .filter( - (comment) => - currentItem && - comment.diffIdentity !== undefined && - comment.diffIdentity !== currentItem.diffIdentity - ) - .map((comment) => comment.id) - ), - [commentsForCurrentItem, currentItem] + const comments = screenState.kind === 'ready' ? screenState.comments : NO_COMMENTS + const { commentsByLine, staleCommentIds } = useMemo( + () => mobileDiffReviewNotesForItem(comments, currentItem), + [comments, currentItem] ) - const commentsByLine = useMemo(() => { - const map = new Map() - for (const comment of commentsForCurrentItem) { - const list = map.get(comment.lineNumber) ?? [] - list.push(comment) - map.set(comment.lineNumber, list) - } - return map - }, [commentsForCurrentItem]) - - // Use git.status for PR head identity; branchCompare’s base ref is not the head. - const prSidebarBranch = screenState.kind === 'ready' ? (screenState.status.branch ?? null) : null - const prSidebarHeadSha = - screenState.kind === 'ready' - ? (screenState.status.head ?? screenState.branchCompare?.summary.headOid ?? null) - : null - const prSidebar = useMobilePrSidebarController({ - client, - connState, - worktreeId, - branch: prSidebarBranch, - headSha: prSidebarHeadSha - }) - const interactions = useMobileDiffReviewInteractions({ client, connState, @@ -251,6 +243,7 @@ export function useMobileDiffReviewController(input: ControllerInput) { selectIndex, selectKey, moveSelection, + remoteViewed: setRemoteViewed, setComposerBody, setActionError, setBusyAction, @@ -276,6 +269,18 @@ export function useMobileDiffReviewController(input: ControllerInput) { worktreeId, prSidebarBranch, prSidebarHeadSha, + github, + // Threads only line up when the local branch is at the PR head; otherwise the file says so. + ...mobileDiffReviewThreadsForItem(github, currentItem), + commentOnGitHub, + // Only the tapped file scrolls to the line, and only until it has. + scrollToLine: + pendingLine !== null && + initialTarget !== null && + currentItem?.filePath === initialTarget.filePath + ? pendingLine + : null, + clearScrollToLine, actionError, busyAction, commentsByLine, diff --git a/mobile/src/session/use-mobile-diff-review-github.ts b/mobile/src/session/use-mobile-diff-review-github.ts new file mode 100644 index 00000000000..7d28a28fd7f --- /dev/null +++ b/mobile/src/session/use-mobile-diff-review-github.ts @@ -0,0 +1,193 @@ +import { useCallback, useEffect, useMemo, useState } from 'react' +import { translate } from '../i18n/mobile-i18n' +import type { ConnectionState } from '../transport/types' +import type { RpcClient } from '../transport/rpc-client' +import { triggerError, triggerSuccess } from '../platform/haptics' +import { fetchAddPRReviewComment, fetchSetPRFileViewed } from './github-pr-review-line-mutations' +import { + applyMobileDiffReviewGitHubViewed, + buildMobileDiffReviewGitHub, + canCommentOnGitHub, + gitHubViewedFile, + type MobileDiffReviewGitHub +} from './mobile-diff-review-github' +import type { + MobileDiffReviewQueueFilter, + MobileDiffReviewQueueItem +} from './mobile-diff-review-queue' +import type { ReviewScreenState } from './mobile-diff-review-screen-model' +import { isPrSidebarDetailsPlaceholder, prSidebarDetailsNeedFetch } from './mobile-pr-sidebar-state' +import type { MobilePrSidebarController } from './use-mobile-pr-sidebar-controller' + +type Input = { + client: RpcClient | null + connState: ConnectionState + worktreeId: string + filter: MobileDiffReviewQueueFilter + screenState: ReviewScreenState + prSidebar: MobilePrSidebarController + setActionError: (message: string | null) => void +} + +/** The branch's GitHub pull request as the diff screen uses it: viewed state, threads and line comments. */ +export function useMobileDiffReviewGitHub(input: Input) { + const { client, connState, worktreeId, filter, screenState, prSidebar, setActionError } = input + const { + prSidebarState, + prSidebarIsGithubRepo, + refetchPRSidebar, + ensurePrSidebarDetails, + patchPrFileViewed + } = prSidebar + const wantsPullRequest = filter === 'branch' && prSidebarIsGithubRepo + + // Committed files are the PR's files, so load its details (threads, viewed state) with the screen. + const sidebarKind = prSidebarState.kind + const needsDetails = + prSidebarState.kind === 'ready' && prSidebarDetailsNeedFetch(prSidebarState.data.details) + useEffect(() => { + if (!wantsPullRequest) { + return + } + if (sidebarKind === 'hidden') { + void refetchPRSidebar({ includeDetails: true }) + } else if (needsDetails) { + void ensurePrSidebarDetails() + } + }, [ensurePrSidebarDetails, needsDetails, refetchPRSidebar, sidebarKind, wantsPullRequest]) + + const localHeadOid = + screenState.kind === 'ready' + ? (screenState.branchCompare?.summary.headOid ?? screenState.status.head ?? null) + : null + const data = prSidebarState.kind === 'ready' ? prSidebarState.data : null + const details = + data?.details && !isPrSidebarDetailsPlaceholder(data.details) ? data.details : null + const github = useMemo( + () => + wantsPullRequest && data && details + ? buildMobileDiffReviewGitHub({ + prNumber: data.pr.number, + prRepo: data.pr.prRepo ?? null, + details, + localHeadOid + }) + : null, + [data, details, localHeadOid, wantsPullRequest] + ) + + // Like desktop, a file whose viewed write is still in flight takes no second tick until it settles. + const [pendingViewed, setPendingViewed] = useState>(() => new Set()) + const applyViewed = useCallback( + (queue: readonly MobileDiffReviewQueueItem[]) => + applyMobileDiffReviewGitHubViewed(queue, github), + [github] + ) + + /** + * Marks a PR file viewed on GitHub. Null when this file's viewed state lives on the phone; false + * while the PR's details are still loading, so a tick is never saved where GitHub will replace it. + */ + const setRemoteViewed = useCallback( + (item: MobileDiffReviewQueueItem, nextViewed: boolean): Promise | null => { + // A failed details load leaves a placeholder; only a load still in flight waits. + const detailsLoading = + prSidebarState.kind === 'loading' || + (prSidebarState.kind === 'ready' && prSidebarState.data.details === null) + if (wantsPullRequest && item.scope === 'branch' && detailsLoading) { + return Promise.resolve(false) + } + const pullRequestId = github?.pullRequestId + const file = gitHubViewedFile(github, item) + if (!github || !pullRequestId || !file) { + return null + } + if (pendingViewed.has(item.filePath)) { + return Promise.resolve(false) + } + if (!client || connState !== 'connected') { + setActionError( + translate('session.useDiffReviewGitHub.waitingForDesktop', 'Waiting for desktop...') + ) + return Promise.resolve(false) + } + const path = item.filePath + const previous = file.viewerViewedState + const setPending = (pending: boolean) => + setPendingViewed((prev) => { + const next = new Set(prev) + if (pending) { + next.add(path) + } else { + next.delete(path) + } + return next + }) + setPending(true) + patchPrFileViewed(pullRequestId, path, nextViewed ? 'VIEWED' : 'UNVIEWED') + return fetchSetPRFileViewed(client, worktreeId, { + pullRequestId, + path, + viewed: nextViewed, + prRepo: github.prRepo + }) + .then((outcome) => { + if (outcome.ok) { + return true + } + if (previous) { + patchPrFileViewed(pullRequestId, path, previous) + } + triggerError() + setActionError(outcome.error) + return false + }) + .finally(() => setPending(false)) + }, + [ + client, + connState, + github, + patchPrFileViewed, + pendingViewed, + prSidebarState, + setActionError, + wantsPullRequest, + worktreeId + ] + ) + + const commentOnGitHub = useCallback( + async (args: { + item: MobileDiffReviewQueueItem + startLine: number + line: number + body: string + }): Promise<{ ok: true } | { ok: false; error: string | null }> => { + if (!client || !github?.headSha || !canCommentOnGitHub(github, args.item, args)) { + return { ok: false, error: null } + } + const outcome = await fetchAddPRReviewComment(client, worktreeId, { + prNumber: github.prNumber, + commitId: github.headSha, + path: args.item.filePath, + line: args.line, + startLine: args.startLine, + body: args.body, + prRepo: github.prRepo + }) + if (!outcome.ok) { + // The sheet is still open over the screen's banner, so the sheet shows this. + triggerError() + return { ok: false, error: outcome.error } + } + triggerSuccess() + // Show the new thread under its line. + void refetchPRSidebar({ includeDetails: true }) + return { ok: true } + }, + [client, github, refetchPRSidebar, worktreeId] + ) + + return { github, applyViewed, setRemoteViewed, commentOnGitHub } +} diff --git a/mobile/src/session/use-mobile-diff-review-interactions.ts b/mobile/src/session/use-mobile-diff-review-interactions.ts index a290ea5aac5..36cd5769f48 100644 --- a/mobile/src/session/use-mobile-diff-review-interactions.ts +++ b/mobile/src/session/use-mobile-diff-review-interactions.ts @@ -4,12 +4,14 @@ import type { ConnectionState } from '../transport/types' import type { RpcClient } from '../transport/rpc-client' import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue' import type { ComposerState, ReviewScreenState } from './mobile-diff-review-screen-model' -import type { ReviewSheetIntents } from './mobile-diff-review-sheets' +import type { ReviewSheet, ReviewSheetIntents } from './mobile-diff-review-sheets' +import type { MobileReviewTerminalTab } from './review-terminal-reply-schema' import { sourceFileDiffOpenRun } from '../source-control/mobile-source-file-open-operations' import { refusedRpcMessageOrFallback } from '../transport/rpc-refusal-message' import { useMobileDiffReviewCommentActions } from './use-mobile-diff-review-comment-actions' import { useMobileDiffReviewGitActions } from './use-mobile-diff-review-git-actions' import { useMobileDiffReviewSendActions } from './use-mobile-diff-review-send-actions' +import { useMobileDiffReviewRangeNote } from './use-mobile-diff-review-range-note' import { connectionRetryAction } from '../transport/connection-retry-action' type InteractionInput = { @@ -30,6 +32,7 @@ type InteractionInput = { /** Keeps a file open by key, through the reload that follows a mutation. */ selectKey: (key: string) => void moveSelection: (direction: 'next' | 'previous') => void + remoteViewed: (item: MobileDiffReviewQueueItem, viewed: boolean) => Promise | null setComposerBody: Dispatch> setActionError: Dispatch> setBusyAction: Dispatch> @@ -57,6 +60,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) { selectIndex, selectKey, moveSelection, + remoteViewed, setComposerBody, setActionError, setBusyAction, @@ -92,11 +96,20 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) { setScreenState, selectIndex, navigationRef, + remoteViewed, setComposerBody, setActionError, sheets }) + const saveRangeNote = useMobileDiffReviewRangeNote({ + worktreeId, + screenState, + currentItem, + composerBody, + saveCommentsAndReviewState + }) + const { runGitMutation } = useMobileDiffReviewGitActions({ client, connState, @@ -106,17 +119,24 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) { loadReviewData }) - const { clearSentNotes, copyNotes, createTerminalAndSend, openSendSheet, sendPromptToTerminal } = - useMobileDiffReviewSendActions({ - client, - connState, - hostCapabilities, - worktreeId, - screenState, - setActionError, - sheets, - saveCommentsAndReviewState - }) + const { + clearSentNotes, + copyNotes, + createTerminalAndSend, + loadTerminals, + openSendSheet, + sendNoteNow, + sendPromptToTerminal + } = useMobileDiffReviewSendActions({ + client, + connState, + hostCapabilities, + worktreeId, + screenState, + setActionError, + sheets, + saveCommentsAndReviewState + }) return { clearSentNotes, @@ -172,6 +192,20 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) { runGitMutation, saveComposer, sendPromptToTerminal, + loadTerminals, + // The sheet's own lines and the terminals it already listed; the draft clears when the next sheet opens. + sendRange: async ( + sheet: Extract, + terminals?: readonly MobileReviewTerminalTab[] + ) => { + const saved = await saveRangeNote(sheet) + if (!('comment' in saved)) { + return saved.error + } + sheets.closeSheet('range', sheet) + await sendNoteNow(saved.comment, terminals) + return null + }, // Staging drops the file from the uncommitted queue; land on the file after it, wrapping from the last. stageAndOpenNext: async () => { if (!currentItem?.canStage) { diff --git a/mobile/src/session/use-mobile-diff-review-range-note.ts b/mobile/src/session/use-mobile-diff-review-range-note.ts new file mode 100644 index 00000000000..cc6b594e4a0 --- /dev/null +++ b/mobile/src/session/use-mobile-diff-review-range-note.ts @@ -0,0 +1,56 @@ +import { useCallback } from 'react' +import type { DiffComment, MobileDiffReviewState } from '../../../src/shared/diff-comment-types' +import { addMobileDiffComment } from './mobile-diff-comments' +import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue' +import type { ReviewScreenState } from './mobile-diff-review-screen-model' + +type Input = { + worktreeId: string + screenState: ReviewScreenState + currentItem: MobileDiffReviewQueueItem | null + composerBody: string + saveCommentsAndReviewState: ( + comments: DiffComment[], + reviewState: MobileDiffReviewState + ) => Promise +} + +/** Saves the range sheet's text as a note on those lines, for the agent; null when there is nothing to save. */ +export function useMobileDiffReviewRangeNote(input: Input) { + const { worktreeId, screenState, currentItem, composerBody, saveCommentsAndReviewState } = input + return useCallback( + async (range: { + startLine: number + line: number + }): Promise<{ comment: DiffComment } | { error: string | null }> => { + const body = composerBody.trim() + if (!body || !currentItem || screenState.kind !== 'ready' || currentItem.scope === 'commit') { + return { error: null } + } + const now = Date.now() + const result = addMobileDiffComment(screenState.comments, { + id: `mobile-${now}-${Math.random().toString(36).slice(2)}`, + worktreeId, + filePath: currentItem.filePath, + oldPath: currentItem.oldPath, + ...(range.startLine < range.line ? { startLine: range.startLine } : {}), + lineNumber: range.line, + body, + createdAt: now, + scope: currentItem.scope, + diffIdentity: currentItem.diffIdentity + }) + if (!result.comment) { + return { error: null } + } + try { + await saveCommentsAndReviewState(result.comments, screenState.reviewState) + } catch (err) { + // The screen's banner sits under the open sheet, so the sheet shows why nothing was sent. + return { error: err instanceof Error ? err.message : null } + } + return { comment: result.comment } + }, + [composerBody, currentItem, saveCommentsAndReviewState, screenState, worktreeId] + ) +} diff --git a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts index 65a11693815..e3d9f091681 100644 --- a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts +++ b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts @@ -117,6 +117,7 @@ describe('useMobileDiffReviewSendActions', () => { let setActionError: Mock let sheets: { openSheet: Mock + openSheetWhenIdle: Mock closeSheet: Mock updateSendSheet: Mock } @@ -128,7 +129,12 @@ describe('useMobileDiffReviewSendActions', () => { clipboardMock.setStringAsync.mockReset().mockResolvedValue(true) resetMobileNativeChatStaleInputForTests() setActionError = vi.fn() - sheets = { openSheet: vi.fn(), closeSheet: vi.fn(), updateSendSheet: vi.fn() } + sheets = { + openSheet: vi.fn(), + openSheetWhenIdle: vi.fn(), + closeSheet: vi.fn(), + updateSendSheet: vi.fn() + } saveCommentsAndReviewState = vi.fn().mockResolvedValue(undefined) }) diff --git a/mobile/src/session/use-mobile-diff-review-send-actions.ts b/mobile/src/session/use-mobile-diff-review-send-actions.ts index 31e28793837..e167d7094b6 100644 --- a/mobile/src/session/use-mobile-diff-review-send-actions.ts +++ b/mobile/src/session/use-mobile-diff-review-send-actions.ts @@ -8,12 +8,15 @@ import { triggerSuccess } from '../platform/haptics' import { formatDiffComments } from '../../../src/shared/diff-comments-format' import { clearSentMobileDiffComments, markMobileDiffCommentsSent } from './mobile-diff-comment-edit' import { reviewTerminalListRead, reviewTerminalSendRun } from './mobile-review-terminal-operations' +import type { MobileReviewTerminalTab } from './review-terminal-reply-schema' import { launchAgentWithPrompt } from './pr-ai-triage-launch' import { interpretOrThrowRefusalMessage } from '../transport/rpc-refusal-message' import { healMobileNativeChatStaleInput } from './native-chat/mobile-native-chat-stale-input' import type { ReviewScreenState } from './mobile-diff-review-screen-model' import type { ReviewSheetIntents } from './mobile-diff-review-sheets' +const NO_NOTES: readonly DiffComment[] = [] + type SendActionsInput = { client: RpcClient | null connState: ConnectionState @@ -21,7 +24,10 @@ type SendActionsInput = { worktreeId: string screenState: ReviewScreenState setActionError: Dispatch> - sheets: Pick + sheets: Pick< + ReviewSheetIntents, + 'openSheet' | 'openSheetWhenIdle' | 'closeSheet' | 'updateSendSheet' + > saveCommentsAndReviewState: ( comments: DiffComment[], reviewState: MobileDiffReviewState @@ -41,29 +47,36 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) { sheets, saveCommentsAndReviewState } = input - const { openSheet, closeSheet, updateSendSheet } = sheets + const { openSheet, openSheetWhenIdle, closeSheet, updateSendSheet } = sheets - const copyNotes = useCallback(async () => { - if (screenState.kind !== 'ready' || screenState.comments.length === 0) { - return - } - // Contain clipboard refusal because copyNotes is fire-and-forget; never leave copied as the last feedback. - try { - await clipboard.writeText(formatDiffComments(screenState.comments)) - } catch (err) { + // Copies these notes, or every note when none are named. + const copyNotes = useCallback( + async (only?: readonly DiffComment[]) => { + const comments = only ?? (screenState.kind === 'ready' ? screenState.comments : NO_NOTES) + if (comments.length === 0) { + return + } + // Contain clipboard refusal because copyNotes is fire-and-forget; never leave copied as the last feedback. + try { + await clipboard.writeText(formatDiffComments(comments)) + } catch (err) { + setActionError( + err instanceof Error + ? err.message + : translate( + 'session.useDiffReviewSendActions.copyFailed', + 'Unable to copy the review notes' + ) + ) + return + } + triggerSuccess() setActionError( - err instanceof Error - ? err.message - : translate( - 'session.useDiffReviewSendActions.copyFailed', - 'Unable to copy the review notes' - ) + translate('session.useDiffReviewSendActions.notesCopied', 'Review notes copied') ) - return - } - triggerSuccess() - setActionError(translate('session.useDiffReviewSendActions.notesCopied', 'Review notes copied')) - }, [clipboard, screenState, setActionError]) + }, + [clipboard, screenState, setActionError] + ) const clearSentNotes = useCallback(async () => { if (screenState.kind !== 'ready') { @@ -94,8 +107,9 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) { await save(next, current.reviewState) }, []) + // From the picker, a send closes it; a direct send has no picker and must not close one opened meanwhile. const sendPromptToTerminal = useCallback( - async (terminal: string, comments: readonly DiffComment[]) => { + async (terminal: string, comments: readonly DiffComment[], fromPicker = true) => { if (!client || connState !== 'connected') { throw new Error( translate('session.useDiffReviewSendActions.waitingForDesktop', 'Waiting for desktop...') @@ -125,7 +139,9 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) { await markNotesSent(comments) triggerSuccess() setActionError(translate('session.useDiffReviewSendActions.notesSent', 'Review notes sent')) - closeSheet('send') + if (fromPicker) { + closeSheet('send') + } }, [client, connState, closeSheet, markNotesSent, setActionError] ) @@ -231,11 +247,72 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) { } }, [client, connState, openSheet, setActionError, updateSendSheet, worktreeId]) + const loadTerminals = useCallback(async () => { + if (!client || connState !== 'connected') { + throw new Error( + translate('session.useDiffReviewSendActions.waitingForDesktop', 'Waiting for desktop...') + ) + } + const response = await reviewTerminalListRead.request(client, { + worktree: `id:${worktreeId}` + }) + return interpretOrThrowRefusalMessage( + () => reviewTerminalListRead.interpret(response), + translate( + 'session.useDiffReviewSendActions.loadSessionsFailed', + 'Unable to load agent sessions' + ) + ) + }, [client, connState, worktreeId]) + + // A note written in the diff goes straight to the workspace's one agent; with none or several, the picker asks. + const sendNoteNow = useCallback( + async (comment: DiffComment, listed?: readonly MobileReviewTerminalTab[]) => { + let terminals + try { + terminals = listed ?? (await loadTerminals()) + } catch (err) { + setActionError( + err instanceof Error + ? err.message + : translate( + 'session.useDiffReviewSendActions.loadSessionsFailed', + 'Unable to load agent sessions' + ) + ) + return + } + const agents = terminals.filter((terminal) => terminal.agent !== undefined) + const [onlyAgent] = agents + if (agents.length === 1 && onlyAgent) { + try { + await sendPromptToTerminal(onlyAgent.terminal, [comment], false) + } catch (err) { + setActionError( + err instanceof Error + ? err.message + : translate('session.useDiffReviewSendActions.sendFailed', 'Failed to send notes') + ) + } + return + } + // The terminal list can outlast a sheet the user opened meanwhile; wait for it rather than replace it. + openSheetWhenIdle({ + kind: 'send', + load: { kind: 'ready', terminals: [...terminals] }, + only: [comment.id] + }) + }, + [loadTerminals, openSheetWhenIdle, sendPromptToTerminal, setActionError] + ) + return { clearSentNotes, copyNotes, createTerminalAndSend, + loadTerminals, openSendSheet, + sendNoteNow, sendPromptToTerminal } } diff --git a/mobile/src/session/use-mobile-pr-sidebar-controller.ts b/mobile/src/session/use-mobile-pr-sidebar-controller.ts index 450c4ca2f36..4483ab2d74d 100644 --- a/mobile/src/session/use-mobile-pr-sidebar-controller.ts +++ b/mobile/src/session/use-mobile-pr-sidebar-controller.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react' import type { ConnectionState } from '../transport/types' +import type { GitHubPRFileViewedState } from '../../../src/shared/github/pull-request-types' import type { RpcClient } from '../transport/rpc-client' import { fetchGithubRepoSlug, @@ -279,6 +280,37 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { } }, [identity, state, load, ensurePrSidebarDetails]) + // Desktop's patchCachedPRFileViewedState: a viewed tick shows at once, and a failed write puts the old state back. + const patchPrFileViewed = useCallback( + (pullRequestId: string, path: string, viewed: GitHubPRFileViewedState) => { + setState((prev) => { + const files = prev.kind === 'ready' ? prev.data.details?.files : undefined + // A write for one PR must not land on another the workspace was relinked to meanwhile. + if ( + prev.kind !== 'ready' || + !prev.data.details || + !files || + prev.data.details.pullRequestId !== pullRequestId + ) { + return prev + } + return { + kind: 'ready', + data: { + ...prev.data, + details: { + ...prev.data.details, + files: files.map((file) => + file.path === path ? { ...file, viewerViewedState: viewed } : file + ) + } + } + } + }) + }, + [] + ) + const retry = useCallback(() => { void load({ includeDetails: true }) }, [load]) @@ -292,7 +324,8 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { openPRSidebar, retryPRSidebar: retry, refetchPRSidebar: load, - ensurePrSidebarDetails + ensurePrSidebarDetails, + patchPrFileViewed } } diff --git a/mobile/src/source-control/mobile-review-route.ts b/mobile/src/source-control/mobile-review-route.ts index c6315a2c60b..e037c0c9c73 100644 --- a/mobile/src/source-control/mobile-review-route.ts +++ b/mobile/src/source-control/mobile-review-route.ts @@ -7,6 +7,8 @@ export type MobileReviewRouteTarget = { worktreeId: string worktreeName: string filePath: string + /** Scrolls to this new-side line, e.g. a review thread's "Open in diff". */ + line?: number } & ({ area: MobileReviewRouteArea } | { area: 'commit'; commitOid: string }) export function buildMobileReviewFileRoute(target: MobileReviewRouteTarget): string { @@ -16,6 +18,9 @@ export function buildMobileReviewFileRoute(target: MobileReviewRouteTarget): str if (target.area === 'commit') { params.set('commit', target.commitOid) } + if (target.line !== undefined) { + params.set('line', String(target.line)) + } if (target.worktreeName) { params.set('name', target.worktreeName) } diff --git a/mobile/src/source-control/use-mobile-source-control-openers.ts b/mobile/src/source-control/use-mobile-source-control-openers.ts index 7b5951f3e7f..fda05f27f00 100644 --- a/mobile/src/source-control/use-mobile-source-control-openers.ts +++ b/mobile/src/source-control/use-mobile-source-control-openers.ts @@ -4,7 +4,7 @@ import { buildMobileReviewFileRoute, type MobileReviewRouteArea } from './mobile // A changed file in the worktree or on the branch, or a file in one commit (diffed against its parent). export type MobileDiffFileTarget = - | { kind: 'changes'; filePath: string; area: MobileReviewRouteArea } + | { kind: 'changes'; filePath: string; area: MobileReviewRouteArea; line?: number } | { kind: 'commit'; filePath: string; commitOid: string } type Params = { @@ -26,6 +26,7 @@ export function useMobileSourceControlOpeners({ hostId, worktreeId, name }: Para worktreeId, worktreeName: name, filePath: target.filePath, + ...(target.kind === 'changes' && target.line !== undefined ? { line: target.line } : {}), ...(target.kind === 'commit' ? { area: 'commit', commitOid: target.commitOid } : { area: target.area }) diff --git a/mobile/src/test-support/bridged-parity/divergence-classes.ts b/mobile/src/test-support/bridged-parity/divergence-classes.ts index 9ef2bea22ab..a046852a834 100644 --- a/mobile/src/test-support/bridged-parity/divergence-classes.ts +++ b/mobile/src/test-support/bridged-parity/divergence-classes.ts @@ -116,7 +116,7 @@ export const BRIDGED_PARITY_EXCLUSIONS: Readonly> = { - identical: 386, + identical: 387, 'reply-meta-required': 0, 'result-absent-settlement': 335, 'result-absent-observation': 3, diff --git a/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts b/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts index d105f19d9cf..a0584ef998c 100644 --- a/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts +++ b/mobile/src/test-support/rpc-recording/adapters/diff-review-action-mount-adapters.ts @@ -102,6 +102,8 @@ export function diffReviewActionMountAdapters( selectIndex: () => {}, selectKey: () => {}, moveSelection: () => {}, + // Viewed state stays on the phone here; GitHub-viewed files have their own recordings. + remoteViewed: () => null, setComposerBody: () => {}, setActionError: (update) => { actionError = typeof update === 'function' ? update(actionError) : update diff --git a/mobile/src/test-support/rpc-recording/adapters/github-pr-mount-adapters.ts b/mobile/src/test-support/rpc-recording/adapters/github-pr-mount-adapters.ts index 70496f7fe12..930a7212951 100644 --- a/mobile/src/test-support/rpc-recording/adapters/github-pr-mount-adapters.ts +++ b/mobile/src/test-support/rpc-recording/adapters/github-pr-mount-adapters.ts @@ -1,5 +1,6 @@ import type { MountAdapter } from '../recording-scenario' import type { operationModuleLoader } from '../operation-module-loader' +import type * as LineMutations from '../../../session/github-pr-review-line-mutations' const WORKTREE = 'repo-9::/w' const PR_NUMBER = 12 @@ -65,6 +66,8 @@ export function githubPrMountAdapters( const mutations = modules.load( 'mobile/src/session/github-pr-mutations.ts' ) + const lineMutations = () => + modules.load('mobile/src/session/github-pr-review-line-mutations.ts') const results: Record = {} function send(name: string, args: Record): Promise { const prRepo = args.fork === true ? FORK_REPO : null @@ -130,6 +133,25 @@ export function githubPrMountAdapters( prRepo }) } + if (name === 'line-comment') { + return lineMutations().fetchAddPRReviewComment(client, WORKTREE, { + prNumber: PR_NUMBER, + commitId: 'head-sha-1', + path: 'src/app.ts', + startLine: 3, + line: 5, + body: 'recorded line comment', + prRepo + }) + } + if (name === 'file-viewed') { + return lineMutations().fetchSetPRFileViewed(client, WORKTREE, { + pullRequestId: 'PR_kwDO', + path: 'src/app.ts', + viewed: true, + prRepo + }) + } if (name === 'resolve-thread') { return mutations.fetchResolveReviewThread(client, WORKTREE, { threadId: 'thread-1',