From 3a3e33f14b79cd40ebfbcc333b2b86035198f3ee Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 9 Jul 2026 22:01:22 -0700 Subject: [PATCH] The `--sandbox` flag (or terminal sandboxing) in Google Antigravity is a (#8017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Here is a summary of how the sandbox behaves on your macOS system: ### βš™οΈ How it Works When `--sandbox` is enabled (either via the launch flag or the `enableTerminalSandbox` setting in your `settings.json`), terminal commands run inside a lightweight containment boundary: - **macOS Native Isolation**: It utilizes macOS's native `sandbox-exec` utility to restrict system calls, network sockets, and directory access. - **Secure File Boundaries**: File system writes are locked down to designated safe zones (such as your designated workspace or scratch directory). Access to critical system paths, private user data, and external network resources is restricted. --- ### πŸ›‘οΈ Active Permissions for this Session In this current session, the permission model is configured as follows: | Action / Resource | Permission Status | Details / Paths | | :--- | :--- | :--- | | **Command Execution** | βœ… **Allowed** | Terminal command execution is enabled. | | **File Reads (Allowed)** | βœ… **Allowed** | `/scratch`, `/browser_recordings`, `/html_artifacts`, `/knowledge`, `/worktrees`, `/skills`, `/builtin` | | **File Writes (Allowed)**| βœ… **Allowed** | `/scratch`, `/browser_recordings`, `/html_artifacts`, `/knowledge`, `/worktrees` | | **Sensitive Files** | ⚠️ **Ask** | `.env`, `.npmrc`, `.vscode`, `.git-credentials`, etc. | | **Root/App Settings** | 🚫 **Denied** | Direct modifications to `/config` and main `.gemini` configurations | --- ### πŸ”§ Configuration and Management * **Persistent Settings**: To enable sandboxing by default for all future sessions, configure the `enableTerminalSandbox` setting in your `~/.gemini/antigravity-cli/settings.json`: ```json { "enableTerminalSandbox": true } ``` * **Dynamic Adjustments**: Within an active CLI (`agy`) session, you can run the `/permissions` slash command to view or modify your autonomy and sandboxing levels on the fly. > [!NOTE] > Running in sandbox mode provides an excellent balance of autonomy and security, allowing me to execute build commands, run test scripts, and manage project files safely without risk to your primary host environment. Please let me know if you would like me to set up a new project workspace or run any specific tasks within this session! --- mobile/mock-source-control-hub.html | 2 +- mobile/src/components/MobileMarkdown.tsx | 2 +- mobile/src/components/MobilePRSidebar.tsx | 58 +++++--- .../src/components/MobilePrComposeSheet.tsx | 5 +- .../pr-sidebar/MobilePrViewPanel.tsx | 2 + .../pr-sidebar/PRActionsSection.tsx | 124 +++++++++++------- .../pr-sidebar/PRCommentsSection.tsx | 16 ++- .../pr-sidebar/PRReviewersSection.tsx | 46 ++++--- .../src/components/pr-sidebar/PRSection.tsx | 20 ++- .../components/pr-sidebar/PRSidebarHeader.tsx | 110 +++++++++------- .../pr-sidebar/ReviewerPickerDrawer.tsx | 48 +++---- .../pr-sidebar/mobile-pr-sidebar-styles.ts | 41 +++++- .../pr-sidebar/pr-actions-styles.ts | 13 ++ .../pr-sidebar/pr-comment-composer-styles.ts | 3 +- mobile/src/session/github-pr-rpc.test.ts | 17 +++ mobile/src/session/github-pr-value-readers.ts | 13 +- mobile/src/session/mobile-pr-sidebar-state.ts | 15 +++ mobile/src/session/pr-actions-engine.test.ts | 45 +++++++ mobile/src/session/pr-actions-engine.ts | 21 ++- .../use-mobile-pr-sidebar-controller.test.ts | 25 ++++ .../use-mobile-pr-sidebar-controller.ts | 30 +++-- .../MobileSourceControlPanel.tsx | 14 +- .../mobile-source-control-actions.test.ts | 4 +- .../mobile-source-control-actions.ts | 2 +- .../mobile-source-control-hub-styles.ts | 28 ++-- .../mobile-source-control-hub-tab.ts | 2 +- src/main/github/client.ts | 71 ++++++++-- 27 files changed, 558 insertions(+), 219 deletions(-) diff --git a/mobile/mock-source-control-hub.html b/mobile/mock-source-control-hub.html index 0a64d75ec..4e8035ade 100644 --- a/mobile/mock-source-control-hub.html +++ b/mobile/mock-source-control-hub.html @@ -1165,7 +1165,7 @@ Changes - + diff --git a/mobile/src/components/MobileMarkdown.tsx b/mobile/src/components/MobileMarkdown.tsx index 5ee7ca6b4..3200dcc5d 100644 --- a/mobile/src/components/MobileMarkdown.tsx +++ b/mobile/src/components/MobileMarkdown.tsx @@ -15,7 +15,7 @@ const MAX_TABLE_COLUMNS = 8 function openMarkdownUrl(url: string): void { const trimmed = url.trim() if (/^(https?:|mailto:)/i.test(trimmed)) { - void Linking.openURL(trimmed) + void Linking.openURL(trimmed).catch(() => {}) } } diff --git a/mobile/src/components/MobilePRSidebar.tsx b/mobile/src/components/MobilePRSidebar.tsx index 932f72e26..42829df03 100644 --- a/mobile/src/components/MobilePRSidebar.tsx +++ b/mobile/src/components/MobilePRSidebar.tsx @@ -39,6 +39,8 @@ type Props = { gitStatus: MobileGitStatusResult | null headSha: string | null bottomInset?: number + // Hub chrome already shows open-on-web; hide the in-body icon there. + showOpenOnWeb?: boolean } // Mutation hooks run unconditionally here and gate internally until a PR is ready. @@ -52,17 +54,15 @@ export function MobilePRSidebar({ gitBranch, gitStatus, headSha, - bottomInset = 0 + bottomInset = 0, + showOpenOnWeb = true }: Props) { const branch = prSidebarRenderBranch(state) // prNumber is 0 until ready; the hook gates on `ready` so it never fires early. const prNumber = state.kind === 'ready' ? state.data.pr.number : 0 - const prRepo = - state.kind === 'ready' - ? state.data.pr.prRepo - ? { owner: state.data.pr.prRepo.owner, repo: state.data.pr.prRepo.repo } - : null - : null + // Prefer the stable PRInfo.prRepo reference β€” cloning owner/repo each render + // reallocates and thrash-updates the mutation/comment/title hooks. + const prRepo = state.kind === 'ready' ? (state.data.pr.prRepo ?? null) : null const actions = useMobilePrActions({ client, connState, @@ -95,6 +95,9 @@ export function MobilePRSidebar({ style={{ flex: 1 }} contentContainerStyle={[styles.scrollContent, { paddingBottom: bottomInset }]} keyboardShouldPersistTaps="handled" + // Why: root-comment / reply composers sit at the bottom of this scroll + // area; without keyboard insets the focused field stays under the keyboard. + automaticallyAdjustKeyboardInsets showsVerticalScrollIndicator={false} > ) @@ -129,7 +133,8 @@ function PrSidebarContent({ actions, commentActions, titleAction, - triage + triage, + showOpenOnWeb }: { branch: ReturnType state: PrSidebarState @@ -144,6 +149,7 @@ function PrSidebarContent({ commentActions: MobilePrCommentActions titleAction: MobilePrTitleAction triage: MobilePrAiTriage + showOpenOnWeb: boolean }) { if (branch === 'loading') { return ( @@ -209,6 +215,7 @@ function PrSidebarContent({ titleAction={titleAction} triage={triage} refetch={refetch} + showOpenOnWeb={showOpenOnWeb} /> ) } @@ -223,7 +230,8 @@ function PrSidebarSections({ commentActions, titleAction, triage, - refetch + refetch, + showOpenOnWeb }: { data: Extract['data'] client: RpcClient | null @@ -233,6 +241,7 @@ function PrSidebarSections({ titleAction: MobilePrTitleAction triage: MobilePrAiTriage refetch: () => void + showOpenOnWeb: boolean }) { const pr = data.pr // Bind the triage launchers to this PR's data; the prompt builders are pure so @@ -262,19 +271,30 @@ function PrSidebarSections({ isBusy: triage.isBusy('resolve-conflicts'), error: triage.error } + // One card for identity + actions so the ready PR isn't a stack of thin + // duplicate blocks (badge row, title, branches, then another action band). return ( <> - - {/* Conflicting-files section mirrors desktop order: directly below the header, - before actions/checks. Renders only when the PR has merge conflicts. */} + + + + + + + {/* Own titled section when present; null otherwise (no empty chrome). */} - {}) } diff --git a/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx b/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx index 5eb17e243..f587074e1 100644 --- a/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx +++ b/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx @@ -62,6 +62,8 @@ export function MobilePrViewPanelBody({ gitStatus={gitStatus} headSha={headSha} bottomInset={insets.bottom} + // Hub header already hosts open-on-web while this segment is active. + showOpenOnWeb={false} /> ) diff --git a/mobile/src/components/pr-sidebar/PRActionsSection.tsx b/mobile/src/components/pr-sidebar/PRActionsSection.tsx index 86b910a08..5fae68038 100644 --- a/mobile/src/components/pr-sidebar/PRActionsSection.tsx +++ b/mobile/src/components/pr-sidebar/PRActionsSection.tsx @@ -7,7 +7,6 @@ import type { RpcClient } from '../../transport/rpc-client' import type { MobilePrActions } from '../../session/use-mobile-pr-actions' import { unlinkMobilePr } from '../../source-control/mobile-pr-link' import { ConfirmModal } from '../ConfirmModal' -import { PRSection } from './PRSection' import { canShowMobilePRAutoMergeControl } from './pr-auto-merge-availability' import { resolveMobilePrMergeMethod, resolvePrActionAvailability } from './pr-actions-state' import { prActionsStyles as styles } from './pr-actions-styles' @@ -25,12 +24,13 @@ type Confirm = | { kind: 'merge'; method: GitHubPRMergeMethod } | { kind: 'state'; state: 'open' | 'closed' } -// Merge, auto-merge toggle, and close/reopen. Destructive actions route through -// ConfirmModal first (R5). The firing row shows a spinner in place of its icon -// and disables; other rows stay interactive (uniform visual). +// Merge primary; Close/Reopen + Unlink share one secondary row. No section title β€” +// button labels are self-explanatory and a header wasted a full row on mobile. export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked }: Props) { const [confirm, setConfirm] = useState(null) const [unlinking, setUnlinking] = useState(false) + // Local unlink errors β€” unlink is not routed through the actions engine. + const [unlinkError, setUnlinkError] = useState(null) // Mobile keeps merge one-tap: use the repo default instead of surfacing a // desktop-style method picker in the narrow PR action stack. @@ -41,23 +41,31 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked } const mergeBusy = actions.isBusy({ kind: 'merge' }) const autoMergeBusy = actions.isBusy({ kind: 'autoMerge' }) const stateBusy = actions.isBusy({ kind: 'state' }) + const unlinkBusy = unlinking || mergeBusy || autoMergeBusy || stateBusy const showAutoMerge = avail.canAutoMerge && canShowMobilePRAutoMergeControl({ ...pr, autoMergeEnabled: autoMerge || pr.autoMergeEnabled === true }) + const showSecondary = avail.canClose || avail.canReopen || avail.canUnlink + const actionError = unlinkError ?? actions.error const unlink = useCallback(async (): Promise => { if (!client || unlinking) { return } setUnlinking(true) + setUnlinkError(null) try { const outcome = await unlinkMobilePr(client, worktreeId) if (outcome.ok) { onUnlinked() + } else { + setUnlinkError(outcome.error) } + } catch (err) { + setUnlinkError(err instanceof Error ? err.message : 'Failed to unlink pull request.') } finally { setUnlinking(false) } @@ -89,6 +97,8 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked } if (!confirm) { return } + // Engine errors take over the shared error line after this; drop unlink text. + setUnlinkError(null) if (confirm.kind === 'merge') { actions.merge(confirm.method) } else { @@ -99,8 +109,7 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked } const copy = confirmCopy() return ( - - {/* Merge controls only while the PR can still be merged (open/draft). */} + {avail.canMerge ? ( setConfirm({ kind: 'merge', method: effectiveMethod })} + onPress={() => { + setUnlinkError(null) + setConfirm({ kind: 'merge', method: effectiveMethod }) + }} disabled={mergeBusy} accessibilityRole="button" accessibilityLabel="Merge pull request" @@ -124,13 +136,15 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked } ) : null} - {/* Auto-merge toggle β€” optimistic, reverts on transient failure. */} {showAutoMerge ? ( Auto-merge when ready actions.setAutoMerge(!autoMerge, effectiveMethod)} + onPress={() => { + setUnlinkError(null) + actions.setAutoMerge(!autoMerge, effectiveMethod) + }} disabled={autoMergeBusy} accessibilityRole="switch" accessibilityState={{ checked: autoMerge }} @@ -147,49 +161,59 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked } ) : null} - {/* Close (open PRs) / Reopen (closed PRs) β€” confirmed before firing (R5). */} - {avail.canClose || avail.canReopen ? ( - setConfirm({ kind: 'state', state: avail.canClose ? 'closed' : 'open' })} - disabled={stateBusy} - accessibilityRole="button" - accessibilityLabel={avail.canClose ? 'Close pull request' : 'Reopen pull request'} - > - {stateBusy ? : null} - - {avail.canClose ? 'Close' : 'Reopen'} - - + {showSecondary ? ( + + {avail.canClose || avail.canReopen ? ( + { + setUnlinkError(null) + setConfirm({ kind: 'state', state: avail.canClose ? 'closed' : 'open' }) + }} + disabled={stateBusy} + accessibilityRole="button" + accessibilityLabel={avail.canClose ? 'Close pull request' : 'Reopen pull request'} + > + {stateBusy ? : null} + + {avail.canClose ? 'Close' : 'Reopen'} + + + ) : null} + {avail.canUnlink ? ( + void unlink()} + disabled={unlinkBusy} + accessibilityRole="button" + accessibilityLabel="Unlink pull request" + > + {unlinking ? ( + + ) : ( + + )} + Unlink + + ) : null} + ) : null} - {/* Unlink the PR from this worktree. Disabled while another PR mutation is in - flight so clearing the link can't race a merge/close refetch. */} - {avail.canUnlink ? ( - void unlink()} - disabled={unlinking || mergeBusy || autoMergeBusy || stateBusy} - accessibilityRole="button" - accessibilityLabel="Unlink pull request" - > - {unlinking ? ( - - ) : ( - - )} - Unlink - - ) : null} + {actionError ? {actionError} : null} - {actions.error ? {actions.error} : null} - - {/* A Modal is taken out of the flex flow, so it adds no body gap here. */} setConfirm(null)} /> - + ) } diff --git a/mobile/src/components/pr-sidebar/PRCommentsSection.tsx b/mobile/src/components/pr-sidebar/PRCommentsSection.tsx index 051f1392e..b6c0a1009 100644 --- a/mobile/src/components/pr-sidebar/PRCommentsSection.tsx +++ b/mobile/src/components/pr-sidebar/PRCommentsSection.tsx @@ -5,6 +5,7 @@ import type { GitHubWorkItemDetails, PRState } from '../../../../src/shared/type import type { GitHubPrRepoSlug } from '../../session/github-pr-rpc' import { colors } from '../../theme/mobile-theme' import { canAddRootComment } from '../../session/pr-comment-actions' +import { isPrSidebarDetailsPlaceholder } from '../../session/mobile-pr-sidebar-state' import type { MobilePrCommentActions } from '../../session/use-mobile-pr-comment-actions' import { PRSection } from './PRSection' import { CommentMarkdown } from './CommentMarkdown' @@ -50,10 +51,15 @@ const COMMENT_PAGE = 12 // review comments, reactions, and collapsible resolved threads. export function PRCommentsSection({ details, prState, prRepo, actions }: Props) { // details is null while phase 2 (the heavy comments/body payload) is still loading. + // A synthetic placeholder means phase 2 failed β€” do not paint that as empty success. const loadingDetails = details === null + const detailsFailed = details != null && isPrSidebarDetailsPlaceholder(details) const body = details?.body ?? '' - const comments = useMemo(() => details?.comments ?? [], [details]) - const isPr = details?.item.type === 'pr' + const comments = useMemo( + () => (details && !isPrSidebarDetailsPlaceholder(details) ? details.comments : []), + [details] + ) + const isPr = details != null && !detailsFailed && details.item.type === 'pr' // Per-card action bundle (stable callbacks from the hook) β€” built once so the // memo'd cards don't re-render on unrelated timeline changes. @@ -94,6 +100,10 @@ export function PRCommentsSection({ details, prState, prRepo, actions }: Props) {loadingDetails ? ( + ) : detailsFailed ? ( + + Could not load description. Tap refresh to try again. + ) : body.trim() ? ( ) : ( @@ -113,6 +123,8 @@ export function PRCommentsSection({ details, prState, prRepo, actions }: Props) > {loadingDetails ? ( + ) : detailsFailed ? ( + Could not load comments. Tap refresh to try again. ) : ( {comments.length === 0 ? ( diff --git a/mobile/src/components/pr-sidebar/PRReviewersSection.tsx b/mobile/src/components/pr-sidebar/PRReviewersSection.tsx index ad16241c2..bc09ecd35 100644 --- a/mobile/src/components/pr-sidebar/PRReviewersSection.tsx +++ b/mobile/src/components/pr-sidebar/PRReviewersSection.tsx @@ -5,6 +5,7 @@ import { colors } from '../../theme/mobile-theme' import type { GitHubWorkItemDetails } from '../../../../src/shared/types' import type { RpcClient } from '../../transport/rpc-client' import type { MobilePrActions } from '../../session/use-mobile-pr-actions' +import { isPrSidebarDetailsPlaceholder } from '../../session/mobile-pr-sidebar-state' import { getPRReviewerRows } from './pr-checks-presentation' import { ReviewerPickerDrawer } from './ReviewerPickerDrawer' import { PRSection } from './PRSection' @@ -20,8 +21,17 @@ type Props = { // Requested reviewers + their latest review status, with a picker to request / // remove (optimistic add/remove via the actions hook). export function PRReviewersSection({ details, actions, client, worktreeId }: Props) { + // details === null means phase 2 (work-item payload) is still in flight β€” same + // signal Comments uses. Do not treat that as "no reviewers" or the section goes + // blank while checks (phase 1) already paint. A synthetic placeholder means + // phase 2 failed (no body/comments/reviews landed). + const loadingDetails = details === null + const detailsFailed = details != null && isPrSidebarDetailsPlaceholder(details) const authoritativeRows = useMemo( - () => (details?.item ? getPRReviewerRows(details.item) : []), + () => + details?.item && !isPrSidebarDetailsPlaceholder(details) + ? getPRReviewerRows(details.item) + : [], [details] ) const [pickerOpen, setPickerOpen] = useState(false) @@ -46,21 +56,27 @@ export function PRReviewersSection({ details, actions, client, worktreeId }: Pro return author ? [author, ...logins] : logins }, [authoritativeRows, details]) - return ( - setPickerOpen(true)} - accessibilityRole="button" - accessibilityLabel="Add or remove reviewers" - > - - - } + const addButton = ( + setPickerOpen(true)} + accessibilityRole="button" + accessibilityLabel="Add or remove reviewers" > - {rows.length === 0 ? ( + + + ) + + return ( + + {loadingDetails ? ( + + + Loading reviewers… + + ) : detailsFailed ? ( + Could not load reviewers. Tap refresh to try again. + ) : rows.length === 0 ? ( No reviewers requested ) : ( rows.map((row) => { diff --git a/mobile/src/components/pr-sidebar/PRSection.tsx b/mobile/src/components/pr-sidebar/PRSection.tsx index a573258d4..0ed5ce904 100644 --- a/mobile/src/components/pr-sidebar/PRSection.tsx +++ b/mobile/src/components/pr-sidebar/PRSection.tsx @@ -3,22 +3,28 @@ import { Text, View } from 'react-native' import { mobilePrSidebarStyles as styles } from './mobile-pr-sidebar-styles' type Props = { - title: string + // Optional: omit for self-explanatory sections (e.g. action buttons) so the + // header row doesn't waste vertical space on mobile. + title?: string // Optional trailing control(s) in the header row (e.g. add-reviewer, checks // summary + rerun). Rendered right-aligned opposite the title. trailing?: ReactNode children: ReactNode } -// Shared card shell for the titled PR sections (Actions/Reviewers/Checks). Mirrors -// the desktop PR page's card-with-header-divider so the sections read consistently. +// Shared card shell for PR sections (Actions/Reviewers/Checks). Mirrors the +// desktop PR page's card-with-header-divider so the sections read consistently. +// Header is omitted when neither title nor trailing is provided. export function PRSection({ title, trailing, children }: Props) { + const showHeader = Boolean(title) || trailing != null return ( - - {title} - {trailing ? {trailing} : null} - + {showHeader ? ( + + {title ? {title} : null} + {trailing ? {trailing} : null} + + ) : null} {children} ) diff --git a/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx b/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx index bf0833031..8a399babe 100644 --- a/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx +++ b/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx @@ -16,11 +16,21 @@ type Props = { details: GitHubWorkItemDetails | null // Inline title-edit action; the pencil affordance only shows when the PR is editable. titleAction: MobilePrTitleAction + // Hub chrome already surfaces open-on-web; hide the duplicate icon in that case. + showOpenOnWeb?: boolean + // When true, render without section chrome so identity can share a card with actions. + bare?: boolean } -// Header: state badge (incl. draft β€” display-only), title, author, head->base. -// The title is inline-editable on an open/draft PR (desktop parity). -export function PRSidebarHeader({ pr, details, titleAction }: Props) { +// Compact identity: state + # + author on one meta row, title, headβ†’base. +// # lives only in the meta row (not also after the title) to avoid repetition. +export function PRSidebarHeader({ + pr, + details, + titleAction, + showOpenOnWeb = true, + bare = false +}: Props) { const item = details?.item const badge = prStateBadge(pr.state) const badgeColor = statusColor(badge.token) @@ -29,14 +39,12 @@ export function PRSidebarHeader({ pr, details, titleAction }: Props) { const baseRef = item?.baseRefName ?? null const headRef = item?.branchName ?? null const editable = canEditPRTitle(pr.state) - // Badge, #number, and the flush-right external-link control all open pr.url - // (canonical host web URL) in the phone browser. const openPr = pr.url ? () => openMobilePrUrl(pr.url) : undefined - return ( - - - + const body = ( + <> + + {badge.label} - {openPr ? ( - [styles.iconButton, pressed && { opacity: 0.6 }]} - > - - - ) : null} + + #{pr.number} + + {author ? Β· {author} : null} - - {author ? by {author} : null} - {baseRef && headRef ? ( - // head -> base reads in merge direction (desktop ChecksPanel parity). - - {headRef} - - {baseRef} - + {showOpenOnWeb && openPr ? ( + [styles.iconButton, pressed && { opacity: 0.6 }]} + > + + ) : null} + + {baseRef && headRef ? ( + + + {headRef} + + + + {baseRef} + + + ) : null} + + ) + + if (bare) { + return {body} + } + return ( + + {body} ) } function PRTitle({ title, - number, editable, - openPr, titleAction }: { title: string - number: number editable: boolean - openPr: (() => void) | undefined titleAction: MobilePrTitleAction }) { const [editing, setEditing] = useState(false) @@ -119,7 +137,7 @@ function PRTitle({ if (editing) { return ( - + - - {title}{' '} - - #{number} - - + {title} {editable ? ( diff --git a/mobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsx b/mobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsx index 97d89a34b..8a3a161f3 100644 --- a/mobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsx +++ b/mobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsx @@ -1,5 +1,5 @@ import { useEffect, useMemo, useState } from 'react' -import { ActivityIndicator, FlatList, Pressable, Text, TextInput, View } from 'react-native' +import { ActivityIndicator, Pressable, Text, TextInput, View } from 'react-native' import { Check } from 'lucide-react-native' import { colors } from '../../theme/mobile-theme' import type { GitHubAssignableUser } from '../../../../src/shared/types' @@ -26,9 +26,10 @@ type LoadState = | { status: 'error'; message: string } | { status: 'loaded'; users: GitHubAssignableUser[] } -// A search + FlatList of github.listAssignableUsers in a BottomDrawer (not a new -// primitive). Seeded with already-requested/latest reviewers + author at the top. -// Optimistic add/remove via onToggle. +// Searchable assignable-user list in a BottomDrawer. Mapped rows (not FlatList): +// this drawer is opened from the PR ScrollView, and a VirtualizedList nested in +// that ScrollView throws and can leave the Reviewers section blank. +// Seeded reviewers + author sort first for quick un-request. export function ReviewerPickerDrawer({ visible, onClose, @@ -47,16 +48,22 @@ export function ReviewerPickerDrawer({ } let cancelled = false setLoad({ status: 'loading' }) - void fetchAssignableUsers(client, worktreeId).then((outcome) => { - if (cancelled) { - return - } - setLoad( - outcome.ok - ? { status: 'loaded', users: outcome.result } - : { status: 'error', message: outcome.error } - ) - }) + void fetchAssignableUsers(client, worktreeId) + .then((outcome) => { + if (cancelled) { + return + } + setLoad( + outcome.ok + ? { status: 'loaded', users: outcome.result } + : { status: 'error', message: outcome.error } + ) + }) + .catch(() => { + if (!cancelled) { + setLoad({ status: 'error', message: 'Failed to load people' }) + } + }) return () => { cancelled = true } @@ -107,15 +114,12 @@ export function ReviewerPickerDrawer({ No matching people ) : ( - u.login} - keyboardShouldPersistTaps="handled" - renderItem={({ item }) => { + + {ordered.map((item) => { const requested = isRequested(item.login) return ( onToggle(item.login)} accessibilityRole="button" @@ -134,8 +138,8 @@ export function ReviewerPickerDrawer({ ) - }} - /> + })} + )} ) diff --git a/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts b/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts index 1c66ed7a4..edf308350 100644 --- a/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts +++ b/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts @@ -25,8 +25,7 @@ export const mobilePrSidebarStyles = StyleSheet.create({ section: { backgroundColor: colors.bgPanel, borderBottomWidth: StyleSheet.hairlineWidth, - borderBottomColor: colors.borderSubtle, - overflow: 'hidden' + borderBottomColor: colors.borderSubtle }, // Section header row: title + optional trailing control, divided from the body // by a hairline border (desktop `h-10 border-b px-3`). @@ -55,14 +54,25 @@ export const mobilePrSidebarStyles = StyleSheet.create({ fontSize: 13, fontWeight: '600' }, - // Header section: state badge, title, author, base<-head branches. - // Badge + trailing open-on-web icon (hub chromeless has no panel chrome for it). - badgeRow: { + // Identity + actions share one section card (hub redesign): no per-block chrome. + identityBlock: { + gap: spacing.sm + }, + // State badge + #number + author on one row; open-on-web flush right when shown. + metaRow: { flexDirection: 'row', alignItems: 'center', justifyContent: 'space-between', gap: spacing.sm }, + metaLeft: { + flex: 1, + minWidth: 0, + flexDirection: 'row', + alignItems: 'center', + flexWrap: 'wrap', + gap: spacing.xs + }, badge: { alignSelf: 'flex-start', paddingHorizontal: spacing.sm, @@ -76,6 +86,7 @@ export const mobilePrSidebarStyles = StyleSheet.create({ fontWeight: '700' }, prTitle: { + flex: 1, color: colors.textPrimary, fontSize: typography.titleSize, fontWeight: '700', @@ -85,6 +96,12 @@ export const mobilePrSidebarStyles = StyleSheet.create({ color: colors.textSecondary, fontSize: typography.metaSize }, + // #number in the meta row β€” stronger than author so the identity scans first. + prMetaStrong: { + color: colors.textPrimary, + fontSize: typography.metaSize, + fontWeight: '600' + }, // Title row: tappable area pairing the title with a trailing edit affordance. titleRow: { flexDirection: 'row', @@ -104,10 +121,11 @@ export const mobilePrSidebarStyles = StyleSheet.create({ gap: spacing.xs }, branchPill: { + flexShrink: 1, color: colors.textPrimary, fontSize: typography.metaSize, fontFamily: typography.monoFamily, - backgroundColor: colors.bgPanel, + backgroundColor: colors.bgRaised, paddingHorizontal: spacing.xs, paddingVertical: 2, borderRadius: radii.button @@ -146,6 +164,13 @@ export const mobilePrSidebarStyles = StyleSheet.create({ color: colors.textSecondary, fontSize: typography.metaSize }, + // Loading / empty status row inside Reviewers (and similar) section bodies. + reviewersStatus: { + minHeight: 44, + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm + }, summaryLabel: { fontSize: typography.bodySize, fontWeight: '700' @@ -274,8 +299,10 @@ export const mobilePrSidebarStyles = StyleSheet.create({ fontSize: typography.bodySize, marginBottom: spacing.sm }, + // No maxHeight / FlatList: the parent BottomDrawer scrolls this block so we + // never nest a VirtualizedList inside the PR page ScrollView. pickerList: { - maxHeight: 320 + gap: 0 }, pickerRow: { minHeight: 44, diff --git a/mobile/src/components/pr-sidebar/pr-actions-styles.ts b/mobile/src/components/pr-sidebar/pr-actions-styles.ts index 865516e34..6d89e75bf 100644 --- a/mobile/src/components/pr-sidebar/pr-actions-styles.ts +++ b/mobile/src/components/pr-sidebar/pr-actions-styles.ts @@ -5,6 +5,19 @@ import { colors, radii, spacing, typography } from '../../theme/mobile-theme' // line). Split out of mobile-pr-sidebar-styles to keep that file under the // 300-line cap. export const prActionsStyles = StyleSheet.create({ + // Bare block when identity + actions share one section card. + actionsBlock: { + gap: spacing.sm + }, + // Close/Reopen + Unlink share a row so secondary actions don't stack full-width. + secondaryRow: { + flexDirection: 'row', + alignItems: 'stretch', + gap: spacing.sm + }, + secondaryButton: { + flex: 1 + }, // Primary CTA (merge) and secondary action buttons (close/reopen/rerun/add). actionButton: { minHeight: 44, diff --git a/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts b/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts index 3d44a9e0c..e6cb4966c 100644 --- a/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts +++ b/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts @@ -5,7 +5,8 @@ import { colors, radii, spacing, typography } from '../../theme/mobile-theme' // match the PR comment timeline; split out to keep PRCommentComposer focused. export const prCommentComposerStyles = StyleSheet.create({ container: { - gap: spacing.sm + // Input β†’ Cancel/Save needs clear separation (title edit was flush without this). + gap: spacing.md }, input: { minHeight: 64, diff --git a/mobile/src/session/github-pr-rpc.test.ts b/mobile/src/session/github-pr-rpc.test.ts index 1adb72ec8..56904e482 100644 --- a/mobile/src/session/github-pr-rpc.test.ts +++ b/mobile/src/session/github-pr-rpc.test.ts @@ -172,6 +172,23 @@ describe('readWorkItemDetails', () => { expect(parsed?.item.latestReviews).toEqual([{ login: 'ok', state: null, avatarUrl: null }]) }) + it('accepts raw gh latestReviews author.login nesting', () => { + const parsed = readWorkItemDetails({ + item: { + id: 'n', + type: 'pr', + number: 1, + state: 'open', + latestReviews: [ + { author: { login: 'coderabbitai', avatarUrl: 'https://a' }, state: 'COMMENTED' } + ] + } + }) + expect(parsed?.item.latestReviews).toEqual([ + { login: 'coderabbitai', state: 'COMMENTED', avatarUrl: 'https://a' } + ]) + }) + it('returns null when item is unparseable', () => { expect(readWorkItemDetails({ item: { number: 1 } })).toBeNull() expect(readWorkItemDetails(null)).toBeNull() diff --git a/mobile/src/session/github-pr-value-readers.ts b/mobile/src/session/github-pr-value-readers.ts index e7dcf9313..6ba8ccd69 100644 --- a/mobile/src/session/github-pr-value-readers.ts +++ b/mobile/src/session/github-pr-value-readers.ts @@ -123,14 +123,23 @@ export function readReviewSummary(value: unknown): GitHubPRReviewSummary | null if (!isRecord(value)) { return null } - const login = readString(value.login) + // Desktop maps latestReviews to top-level `login`. Raw `gh pr view --json` + // keeps nested `author.login` β€” accept both so mobile never drops reviewers. + const nestedAuthor = isRecord(value.author) ? value.author : null + const login = + readString(value.login) ?? (nestedAuthor ? readString(nestedAuthor.login) : undefined) if (login === undefined) { return null } + const avatarUrl = + readString(value.avatarUrl) ?? + (nestedAuthor + ? (readString(nestedAuthor.avatarUrl) ?? readString(nestedAuthor.avatar_url) ?? null) + : null) return { login, state: readString(value.state) ?? null, - avatarUrl: readString(value.avatarUrl) ?? null + avatarUrl } } diff --git a/mobile/src/session/mobile-pr-sidebar-state.ts b/mobile/src/session/mobile-pr-sidebar-state.ts index 4ce32a7ce..1b4d9a1b5 100644 --- a/mobile/src/session/mobile-pr-sidebar-state.ts +++ b/mobile/src/session/mobile-pr-sidebar-state.ts @@ -157,6 +157,7 @@ export function resolvePrSidebarDetailsAfterPhase2(args: { } // Placeholder details so Description/Comments leave the spinner after a failed phase 2. +// The synthetic id is the only id shape we mint β€” real GitHub node ids never match. export function emptyPrSidebarDetails(pr: PRInfo): GitHubWorkItemDetails { return { item: { @@ -175,6 +176,20 @@ export function emptyPrSidebarDetails(pr: PRInfo): GitHubWorkItemDetails { } } +// True when phase 2 failed and we synthesized a stand-in (no body/comments/reviews). +export function isPrSidebarDetailsPlaceholder(details: GitHubWorkItemDetails): boolean { + return details.item.id === `pr-${details.item.number}` +} + +// Phase-2 still needs a real payload: null (in flight / not started) or a +// synthetic placeholder from a failed fetch. Real details (including empty body) +// do not qualify β€” ensure/retry must not re-pull those. +export function prSidebarDetailsNeedFetch( + details: GitHubWorkItemDetails | null | undefined +): boolean { + return details == null || isPrSidebarDetailsPlaceholder(details) +} + // Soft head refresh may restart an in-flight phase-1 load; only skip when there is // no visible/in-flight sidebar work (hidden or terminal error states). export function shouldSoftRefreshPrSidebarOnHeadChange(kind: PrSidebarState['kind']): boolean { diff --git a/mobile/src/session/pr-actions-engine.test.ts b/mobile/src/session/pr-actions-engine.test.ts index 72bde8724..d879454c5 100644 --- a/mobile/src/session/pr-actions-engine.test.ts +++ b/mobile/src/session/pr-actions-engine.test.ts @@ -80,6 +80,51 @@ describe('PrActionsEngine β€” transport-rejection-normalized outcomes settle cle expect(engine.error).toBe('socket hung up') expect(engine.busy).toBeNull() }) + + it('surfaces a refetch throw as error without rejecting the action promise', async () => { + const engine = new PrActionsEngine({ + mutations: { + mergePR: async () => ({ ok: true }), + setPRAutoMerge: async () => ({ ok: true }), + updatePRState: async () => ({ ok: true }), + requestReviewers: async () => ({ ok: true }), + removeReviewers: async () => ({ ok: true }), + rerunChecks: async () => ({ ok: true }) + }, + prNumber: 1, + refetch: async () => { + throw new Error('refresh failed') + }, + onChange: () => {} + }) + await expect(engine.merge()).resolves.toBeUndefined() + expect(engine.error).toBe('refresh failed') + expect(engine.busy).toBeNull() + }) + + it('does not notify on no-op setError(null) at action start', async () => { + const onChange = vi.fn() + const engine = new PrActionsEngine({ + mutations: { + mergePR: async () => ({ ok: true }), + setPRAutoMerge: async () => ({ ok: true }), + updatePRState: async () => ({ ok: true }), + requestReviewers: async () => ({ ok: true }), + removeReviewers: async () => ({ ok: true }), + rerunChecks: async () => ({ ok: true }) + }, + prNumber: 1, + refetch: () => {}, + onChange + }) + // Idle: error is already null. Action start must only notify for busy, not a + // redundant clearError path. + onChange.mockClear() + const p = engine.merge() + // First notify is setBusy only (setError(null) no-ops). + expect(onChange).toHaveBeenCalledTimes(1) + await p + }) }) describe('PrActionsEngine β€” PR identity changes', () => { diff --git a/mobile/src/session/pr-actions-engine.ts b/mobile/src/session/pr-actions-engine.ts index a647b62df..7c3495453 100644 --- a/mobile/src/session/pr-actions-engine.ts +++ b/mobile/src/session/pr-actions-engine.ts @@ -129,7 +129,12 @@ export class PrActionsEngine { return f } + // Why: action start pairs setBusy + setError(null); skip notify when unchanged + // so we don't force a full PR panel re-render for free. private setBusy(key: PrActionBusyKey | null): void { + if (key === null ? this.busy === null : busyKeyEquals(this.busy, key)) { + return + } this.busy = key this.cfg.onChange() } @@ -143,6 +148,9 @@ export class PrActionsEngine { } private setError(message: string | null): void { + if (this.error === message) { + return + } this.error = message this.cfg.onChange() } @@ -180,7 +188,14 @@ export class PrActionsEngine { } if (outcome.ok) { handlers.onSuccess() - await this.cfg.refetch() + // Why: void engine.merge() callers are fire-and-forget; refetch must not LogBox. + try { + await this.cfg.refetch() + } catch (err) { + if (this.identity === identity) { + this.setError(err instanceof Error ? err.message : 'Failed to refresh pull request.') + } + } return } // Both failure classes clear optimism to authoritative; only the message @@ -205,6 +220,10 @@ export class PrActionsEngine { prRepo: cfg.prRepo }) await this.settle(identity, outcome, { onSuccess: () => {}, onRevert: () => {} }) + } catch (err) { + if (this.identity === identity) { + this.setError(err instanceof Error ? err.message : 'Failed to merge pull request.') + } } finally { this.clearBusyIfOwned(identity, { kind: 'merge' }) } diff --git a/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts b/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts index de77433fc..f402f7b58 100644 --- a/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts +++ b/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts @@ -5,6 +5,8 @@ import type { GitHubPrReadOutcome } from './github-pr-rpc' import { classifyPrSidebarFailure, emptyPrSidebarDetails, + isPrSidebarDetailsPlaceholder, + prSidebarDetailsNeedFetch, loadPrSidebarData, loadPrSidebarDetails, resolvePrSidebarDetailsAfterPhase2, @@ -240,6 +242,29 @@ describe('resolvePrSidebarDetailsAfterPhase2', () => { expect(empty.comments).toEqual([]) expect(empty.item.number).toBe(7) expect(empty.item.type).toBe('pr') + expect(isPrSidebarDetailsPlaceholder(empty)).toBe(true) + }) +}) + +describe('isPrSidebarDetailsPlaceholder / prSidebarDetailsNeedFetch', () => { + it('detects the synthetic hyphen id, not the host pr:n shape', () => { + const placeholder = emptyPrSidebarDetails(PR) + expect(isPrSidebarDetailsPlaceholder(placeholder)).toBe(true) + expect(isPrSidebarDetailsPlaceholder(DETAILS)).toBe(false) + // Host work-item ids use a colon (`pr:7`); placeholders use a hyphen (`pr-7`). + expect( + isPrSidebarDetailsPlaceholder({ + ...DETAILS, + item: { ...DETAILS.item, id: `pr:${PR.number}` } + }) + ).toBe(false) + }) + + it('needs fetch for null and placeholders, not real details', () => { + expect(prSidebarDetailsNeedFetch(null)).toBe(true) + expect(prSidebarDetailsNeedFetch(undefined)).toBe(true) + expect(prSidebarDetailsNeedFetch(emptyPrSidebarDetails(PR))).toBe(true) + expect(prSidebarDetailsNeedFetch(DETAILS)).toBe(false) }) }) diff --git a/mobile/src/session/use-mobile-pr-sidebar-controller.ts b/mobile/src/session/use-mobile-pr-sidebar-controller.ts index 415a7581b..752b2ecb3 100644 --- a/mobile/src/session/use-mobile-pr-sidebar-controller.ts +++ b/mobile/src/session/use-mobile-pr-sidebar-controller.ts @@ -11,6 +11,7 @@ import { import { loadPrSidebarData, loadPrSidebarDetails, + prSidebarDetailsNeedFetch, resolvePrSidebarDetailsAfterPhase2, shouldApplyResult, shouldSoftRefreshPrSidebarOnHeadChange, @@ -103,12 +104,21 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { if (!probeReady || !client) { return } - void fetchGithubRepoSlug(client, worktreeId).then((outcome) => { - if (!cancelled) { - setIsGithubRepo(outcome.ok && outcome.result !== null) - setRepoProbeLoaded(true) - } - }) + void fetchGithubRepoSlug(client, worktreeId) + .then((outcome) => { + if (!cancelled) { + setIsGithubRepo(outcome.ok && outcome.result !== null) + setRepoProbeLoaded(true) + } + }) + .catch(() => { + // Why: sendGithubPrRead already normalizes throws, but a cancelled + // unmount + any unexpected rejection must not surface as LogBox. + if (!cancelled) { + setIsGithubRepo(false) + setRepoProbeLoaded(true) + } + }) return () => { cancelled = true } @@ -228,9 +238,12 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { // Phase-2 only β€” used when the hub opens the PR segment after a chip-only load. // Uses detailsSeqRef (not loadSeqRef) so it cannot cancel a concurrent soft phase-1. + // Retries synthetic placeholders too: a failed phase-2 installs non-null empty + // details so Description/Comments leave the spinner, and without this ensure + // would never re-fetch on tab re-open. const ensurePrSidebarDetails = useCallback(async () => { const current = stateRef.current - if (current.kind !== 'ready' || current.data.details != null) { + if (current.kind !== 'ready' || !prSidebarDetailsNeedFetch(current.data.details)) { return } const deps = buildDeps() @@ -303,7 +316,8 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { (state.kind !== 'ready' && state.kind !== 'loading') ) { void load({ includeDetails: true }) - } else if (state.kind === 'ready' && state.data.details == null) { + } else if (state.kind === 'ready' && prSidebarDetailsNeedFetch(state.data.details)) { + // Retries phase-2 placeholders too (failed details load, not only null). void ensurePrSidebarDetails() } }, [identity, state, load, ensurePrSidebarDetails]) diff --git a/mobile/src/source-control/MobileSourceControlPanel.tsx b/mobile/src/source-control/MobileSourceControlPanel.tsx index 332f62312..25656aae2 100644 --- a/mobile/src/source-control/MobileSourceControlPanel.tsx +++ b/mobile/src/source-control/MobileSourceControlPanel.tsx @@ -16,6 +16,7 @@ import type { SourceControlHubTab } from './mobile-source-control-hub-tab' import { buildMobilePrChipSummary, countUnresolvedReviewThreads } from './mobile-pr-chip-summary' import { isMobileConflictAborting } from './mobile-source-control-conflict-abort' import { useMobilePrSidebarController } from '../session/use-mobile-pr-sidebar-controller' +import { prSidebarDetailsNeedFetch } from '../session/mobile-pr-sidebar-state' import { MobilePrViewPanelBody } from '../components/pr-sidebar/MobilePrViewPanel' import { openMobilePrUrl } from '../components/MobilePrComposeSheet' @@ -177,8 +178,11 @@ export function MobileSourceControlPanel({ // none). Keyed by PR number β€” not a boolean β€” so a same-branch PR swap during a // chip-only soft refresh re-arms phase 2 for the new PR instead of leaving its // comments on a forever-spinner (a stale ensure bails on the number mismatch). + // Placeholder details (failed phase 2) also count as missing so reopening the + // PR tab retries instead of leaving empty Description/Comments forever. const prDetailsMissingFor = - prController.prSidebarState.kind === 'ready' && prController.prSidebarState.data.details == null + prController.prSidebarState.kind === 'ready' && + prSidebarDetailsNeedFetch(prController.prSidebarState.data.details) ? prController.prSidebarState.data.pr.number : null @@ -299,6 +303,8 @@ export function MobileSourceControlPanel({ // Controller + chip state still live for instant re-open without a full cold start. const showPrBody = ready && activeTab === 'pr' const conflictOperation = status?.conflictOperation ?? null + // Git status always reports a conflictOperation enum; 'unknown' means none. + const hasActiveConflict = conflictOperation != null && conflictOperation !== 'unknown' const conflictAborting = isMobileConflictAborting(busyAction, conflictOperation) return ( @@ -313,7 +319,11 @@ export function MobileSourceControlPanel({ - {ready ? ( + {/* Branch card + PR chip are the Changes/Commits glance layer. On the PR + tab they duplicate the ready PR body (#, state, checks rollup, branch + trajectory), so hide the whole card there and let the PR panel own it β€” + unless a merge/rebase conflict is active, which only this card can abort. */} + {ready && (activeTab !== 'pr' || hasActiveConflict) ? ( { expect(labels.some((l) => l.startsWith('Fast-forward'))).toBe(true) expect(labels).toContain('Rebase onto base') expect(labels).toContain('Switch branch') - expect(labels).toContain('History') + expect(labels).toContain('Commits') expect(labels).toContain('Create PR') }) @@ -90,7 +90,7 @@ describe('buildMobileSourceControlActions', () => { const handlers = noopHandlers() const actions = buildMobileSourceControlActions(args({ handlers })) action(actions, 'Switch branch')?.onPress() - action(actions, 'History')?.onPress() + action(actions, 'Commits')?.onPress() expect(handlers.checkout).toHaveBeenCalled() expect(handlers.history).toHaveBeenCalled() }) diff --git a/mobile/src/source-control/mobile-source-control-actions.ts b/mobile/src/source-control/mobile-source-control-actions.ts index 0e504c5a2..b06af8a6a 100644 --- a/mobile/src/source-control/mobile-source-control-actions.ts +++ b/mobile/src/source-control/mobile-source-control-actions.ts @@ -217,7 +217,7 @@ export function buildMobileSourceControlActions( onPress: handlers.checkout }, { - label: 'History', + label: 'Commits', iconKey: 'history', disabled: busy, onPress: handlers.history diff --git a/mobile/src/source-control/mobile-source-control-hub-styles.ts b/mobile/src/source-control/mobile-source-control-hub-styles.ts index 683fa3a1d..10d126177 100644 --- a/mobile/src/source-control/mobile-source-control-hub-styles.ts +++ b/mobile/src/source-control/mobile-source-control-hub-styles.ts @@ -4,29 +4,29 @@ import { colors, radii, spacing, typography } from '../theme/mobile-theme' // Styles for the hub's segmented control and the branch-card PR chip. Split from // mobile-source-control-styles.ts so neither file crosses the line limit. export const hubStyles = StyleSheet.create({ + // Full-width tab bar under the hub header. Edge-to-edge, no inset track and no + // inner padding β€” segments share height evenly so the control doesn't float in a + // pill frame with gaps above/below the active cell. segments: { flexDirection: 'row', - marginHorizontal: spacing.lg, - marginTop: spacing.sm, - padding: 3, - borderRadius: radii.button + 2, - backgroundColor: colors.bgRaised, - borderWidth: StyleSheet.hairlineWidth, - borderColor: colors.borderSubtle, - gap: 2 + alignItems: 'stretch', + width: '100%', + backgroundColor: colors.bgPanel, + borderBottomWidth: StyleSheet.hairlineWidth, + borderBottomColor: colors.borderSubtle }, segment: { flex: 1, - minHeight: 34, - borderRadius: radii.button, + minHeight: 40, alignItems: 'center', justifyContent: 'center', - paddingHorizontal: spacing.xs + paddingHorizontal: spacing.xs, + // Reserved so active underline doesn't change layout height. + borderBottomWidth: 2, + borderBottomColor: 'transparent' }, segmentActive: { - backgroundColor: colors.bgPanel, - borderWidth: StyleSheet.hairlineWidth, - borderColor: colors.borderSubtle + borderBottomColor: colors.textPrimary }, segmentPressed: { opacity: 0.7 diff --git a/mobile/src/source-control/mobile-source-control-hub-tab.ts b/mobile/src/source-control/mobile-source-control-hub-tab.ts index e243032a5..3a0a48406 100644 --- a/mobile/src/source-control/mobile-source-control-hub-tab.ts +++ b/mobile/src/source-control/mobile-source-control-hub-tab.ts @@ -13,7 +13,7 @@ export const SOURCE_CONTROL_HUB_TABS: readonly SourceControlHubTab[] = [ export const SOURCE_CONTROL_HUB_TAB_LABELS: Record = { changes: 'Changes', pr: 'Pull Request', - history: 'History' + history: 'Commits' } // Normalize a route param (possibly an array from expo-router, possibly unknown) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 3a308a3ad..6e4005421 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -848,6 +848,43 @@ async function fetchIssueWorkItem( return mapIssueWorkItem(JSON.parse(stdout) as Record) } +// REST /pulls/{n} has requested_reviewers but not latestReviews. When the JSON +// `gh pr view` path fails, still pull review fields from gh so mobile/desktop +// reviewer lists (CodeRabbit COMMENTED, etc.) are not silently empty. +const WORK_ITEM_PR_REVIEW_JSON_FIELDS = 'reviewRequests,latestReviews' + +async function fetchPullRequestReviewFields( + number: number, + ownerRepo: OwnerRepo | null, + ghOptions: GhExecOptions +): Promise> { + try { + const args = ownerRepo + ? [ + 'pr', + 'view', + String(number), + '--repo', + `${ownerRepo.owner}/${ownerRepo.repo}`, + '--json', + WORK_ITEM_PR_REVIEW_JSON_FIELDS + ] + : ['pr', 'view', String(number), '--json', WORK_ITEM_PR_REVIEW_JSON_FIELDS] + const { stdout } = await ghExecFileAsync(args, ghOptions) + const item = JSON.parse(stdout) as Record + return { + ...(item.reviewRequests !== undefined + ? { reviewRequests: usersFromUnknown(item.reviewRequests) } + : {}), + ...(item.latestReviews !== undefined + ? { latestReviews: latestReviewsFromUnknown(item.latestReviews) } + : {}) + } + } catch { + return {} + } +} + async function fetchPullRequestWorkItem( repoPath: string, ownerRepo: OwnerRepo | null, @@ -872,24 +909,36 @@ async function fetchPullRequestWorkItem( ) const item = JSON.parse(stdout) as Record const mapped = mapPullRequestWorkItem(item, ownerRepo) + // Why: merge-metadata GraphQL is best-effort. A failure here must not fall + // through to the REST path below β€” that path drops latestReviews and blanks + // the mobile/desktop reviewer list for bots that only left a review. const baseRefName = typeof item.baseRefName === 'string' ? item.baseRefName : undefined - const mergeMetadata = await detectRepositoryMergeMetadata(ownerRepo, baseRefName, ghOptions) - return { - ...mapped, - mergeQueueRequired: mergeMetadata.mergeQueueRequired, - ...(mergeMetadata.autoMergeAllowed !== null - ? { autoMergeAllowed: mergeMetadata.autoMergeAllowed } - : {}), - ...(mergeMetadata.mergeMethodSettings - ? { mergeMethodSettings: mergeMetadata.mergeMethodSettings } - : {}) + try { + const mergeMetadata = await detectRepositoryMergeMetadata(ownerRepo, baseRefName, ghOptions) + return { + ...mapped, + mergeQueueRequired: mergeMetadata.mergeQueueRequired, + ...(mergeMetadata.autoMergeAllowed !== null + ? { autoMergeAllowed: mergeMetadata.autoMergeAllowed } + : {}), + ...(mergeMetadata.mergeMethodSettings + ? { mergeMethodSettings: mergeMetadata.mergeMethodSettings } + : {}) + } + } catch { + return mapped } } catch { const { stdout } = await ghExecFileAsync( ['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`], ghOptions ) - return mapPullRequestWorkItem(JSON.parse(stdout) as Record, ownerRepo) + const mapped = mapPullRequestWorkItem( + JSON.parse(stdout) as Record, + ownerRepo + ) + const reviewFields = await fetchPullRequestReviewFields(number, ownerRepo, ghOptions) + return { ...mapped, ...reviewFields } } }