The `--sandbox` flag (or terminal sandboxing) in Google Antigravity is a (#8017)
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!
This commit is contained in:
Jinjing
GitHub
parent
008ad62d3f
commit
3a3e33f14b
|
|
@ -1165,7 +1165,7 @@
|
|||
Changes
|
||||
</button>
|
||||
<button class="segment" data-seg="pr" onclick="switchSeg('pr')">Pull Request</button>
|
||||
<button class="segment" data-seg="history" onclick="switchSeg('history')">History</button>
|
||||
<button class="segment" data-seg="history" onclick="switchSeg('history')">Commits</button>
|
||||
</div>
|
||||
|
||||
<!-- Persistent branch card -->
|
||||
|
|
|
|||
|
|
@ -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(() => {})
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
>
|
||||
<PrSidebarContent
|
||||
|
|
@ -111,6 +114,7 @@ export function MobilePRSidebar({
|
|||
commentActions={commentActions}
|
||||
titleAction={titleAction}
|
||||
triage={triage}
|
||||
showOpenOnWeb={showOpenOnWeb}
|
||||
/>
|
||||
</ScrollView>
|
||||
)
|
||||
|
|
@ -129,7 +133,8 @@ function PrSidebarContent({
|
|||
actions,
|
||||
commentActions,
|
||||
titleAction,
|
||||
triage
|
||||
triage,
|
||||
showOpenOnWeb
|
||||
}: {
|
||||
branch: ReturnType<typeof prSidebarRenderBranch>
|
||||
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<PrSidebarState, { kind: 'ready' }>['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 (
|
||||
<>
|
||||
<PRSidebarHeader pr={data.pr} details={data.details} titleAction={titleAction} />
|
||||
{/* Conflicting-files section mirrors desktop order: directly below the header,
|
||||
before actions/checks. Renders only when the PR has merge conflicts. */}
|
||||
<View style={styles.section}>
|
||||
<View style={styles.sectionBody}>
|
||||
<PRSidebarHeader
|
||||
pr={data.pr}
|
||||
details={data.details}
|
||||
titleAction={titleAction}
|
||||
showOpenOnWeb={showOpenOnWeb}
|
||||
bare
|
||||
/>
|
||||
<PRActionsSection
|
||||
pr={data.pr}
|
||||
actions={actions}
|
||||
client={client}
|
||||
worktreeId={worktreeId}
|
||||
onUnlinked={refetch}
|
||||
/>
|
||||
</View>
|
||||
</View>
|
||||
{/* Own titled section when present; null otherwise (no empty chrome). */}
|
||||
<PRConflictingFilesSection pr={data.pr} triage={conflictsTriage} />
|
||||
<PRActionsSection
|
||||
pr={data.pr}
|
||||
actions={actions}
|
||||
client={client}
|
||||
worktreeId={worktreeId}
|
||||
onUnlinked={refetch}
|
||||
/>
|
||||
<PRReviewersSection
|
||||
details={data.details}
|
||||
actions={actions}
|
||||
|
|
|
|||
|
|
@ -45,5 +45,8 @@ export function MobilePrComposeSheet({
|
|||
}
|
||||
|
||||
export function openMobilePrUrl(url: string): void {
|
||||
void Linking.openURL(url)
|
||||
// Why: Linking.openURL rejects when iOS/Android can't open the URL (no app,
|
||||
// bad scheme, etc.). Without a catch that surfaces as LogBox "Uncaught
|
||||
// (in promise) Error: Unable to open URL…" over the PR screen.
|
||||
void Linking.openURL(url).catch(() => {})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
/>
|
||||
</View>
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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<Confirm | null>(null)
|
||||
const [unlinking, setUnlinking] = useState(false)
|
||||
// Local unlink errors — unlink is not routed through the actions engine.
|
||||
const [unlinkError, setUnlinkError] = useState<string | null>(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<void> => {
|
||||
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 (
|
||||
<PRSection title="Actions">
|
||||
{/* Merge controls only while the PR can still be merged (open/draft). */}
|
||||
<View style={styles.actionsBlock}>
|
||||
{avail.canMerge ? (
|
||||
<Pressable
|
||||
style={[
|
||||
|
|
@ -108,7 +117,10 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked }
|
|||
styles.actionButtonMerge,
|
||||
mergeBusy && styles.actionButtonDisabled
|
||||
]}
|
||||
onPress={() => 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 }
|
|||
</Pressable>
|
||||
) : null}
|
||||
|
||||
{/* Auto-merge toggle — optimistic, reverts on transient failure. */}
|
||||
{showAutoMerge ? (
|
||||
<View style={styles.toggleRow}>
|
||||
<Text style={styles.toggleLabel}>Auto-merge when ready</Text>
|
||||
<Pressable
|
||||
style={[styles.togglePill, autoMerge && styles.togglePillOn]}
|
||||
onPress={() => 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 }
|
|||
</View>
|
||||
) : null}
|
||||
|
||||
{/* Close (open PRs) / Reopen (closed PRs) — confirmed before firing (R5). */}
|
||||
{avail.canClose || avail.canReopen ? (
|
||||
<Pressable
|
||||
style={[styles.actionButton, stateBusy && styles.actionButtonDisabled]}
|
||||
onPress={() => setConfirm({ kind: 'state', state: avail.canClose ? 'closed' : 'open' })}
|
||||
disabled={stateBusy}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel={avail.canClose ? 'Close pull request' : 'Reopen pull request'}
|
||||
>
|
||||
{stateBusy ? <ActivityIndicator color={colors.textSecondary} /> : null}
|
||||
<Text
|
||||
style={[styles.actionButtonText, avail.canClose && styles.actionButtonDestructiveText]}
|
||||
>
|
||||
{avail.canClose ? 'Close' : 'Reopen'}
|
||||
</Text>
|
||||
</Pressable>
|
||||
{showSecondary ? (
|
||||
<View style={styles.secondaryRow}>
|
||||
{avail.canClose || avail.canReopen ? (
|
||||
<Pressable
|
||||
style={[
|
||||
styles.actionButton,
|
||||
styles.secondaryButton,
|
||||
stateBusy && styles.actionButtonDisabled
|
||||
]}
|
||||
onPress={() => {
|
||||
setUnlinkError(null)
|
||||
setConfirm({ kind: 'state', state: avail.canClose ? 'closed' : 'open' })
|
||||
}}
|
||||
disabled={stateBusy}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel={avail.canClose ? 'Close pull request' : 'Reopen pull request'}
|
||||
>
|
||||
{stateBusy ? <ActivityIndicator color={colors.textSecondary} /> : null}
|
||||
<Text
|
||||
style={[
|
||||
styles.actionButtonText,
|
||||
avail.canClose && styles.actionButtonDestructiveText
|
||||
]}
|
||||
>
|
||||
{avail.canClose ? 'Close' : 'Reopen'}
|
||||
</Text>
|
||||
</Pressable>
|
||||
) : null}
|
||||
{avail.canUnlink ? (
|
||||
<Pressable
|
||||
style={[
|
||||
styles.actionButton,
|
||||
styles.secondaryButton,
|
||||
unlinkBusy && styles.actionButtonDisabled
|
||||
]}
|
||||
onPress={() => void unlink()}
|
||||
disabled={unlinkBusy}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel="Unlink pull request"
|
||||
>
|
||||
{unlinking ? (
|
||||
<ActivityIndicator color={colors.textSecondary} />
|
||||
) : (
|
||||
<Link2Off size={16} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
)}
|
||||
<Text style={styles.actionButtonText}>Unlink</Text>
|
||||
</Pressable>
|
||||
) : null}
|
||||
</View>
|
||||
) : 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 ? (
|
||||
<Pressable
|
||||
style={[
|
||||
styles.actionButton,
|
||||
(unlinking || mergeBusy || autoMergeBusy || stateBusy) && styles.actionButtonDisabled
|
||||
]}
|
||||
onPress={() => void unlink()}
|
||||
disabled={unlinking || mergeBusy || autoMergeBusy || stateBusy}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel="Unlink pull request"
|
||||
>
|
||||
{unlinking ? (
|
||||
<ActivityIndicator color={colors.textSecondary} />
|
||||
) : (
|
||||
<Link2Off size={16} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
)}
|
||||
<Text style={styles.actionButtonText}>Unlink</Text>
|
||||
</Pressable>
|
||||
) : null}
|
||||
{actionError ? <Text style={styles.actionError}>{actionError}</Text> : null}
|
||||
|
||||
{actions.error ? <Text style={styles.actionError}>{actions.error}</Text> : null}
|
||||
|
||||
{/* A Modal is taken out of the flex flow, so it adds no body gap here. */}
|
||||
<ConfirmModal
|
||||
visible={confirm !== null}
|
||||
title={copy.title}
|
||||
|
|
@ -199,6 +223,6 @@ export function PRActionsSection({ pr, actions, client, worktreeId, onUnlinked }
|
|||
onConfirm={runConfirmed}
|
||||
onCancel={() => setConfirm(null)}
|
||||
/>
|
||||
</PRSection>
|
||||
</View>
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
|||
<PRSection title="Description">
|
||||
{loadingDetails ? (
|
||||
<ActivityIndicator color={colors.textSecondary} />
|
||||
) : detailsFailed ? (
|
||||
<Text style={styles.noDescription}>
|
||||
Could not load description. Tap refresh to try again.
|
||||
</Text>
|
||||
) : body.trim() ? (
|
||||
<CommentMarkdown content={body} variant="document" />
|
||||
) : (
|
||||
|
|
@ -113,6 +123,8 @@ export function PRCommentsSection({ details, prState, prRepo, actions }: Props)
|
|||
>
|
||||
{loadingDetails ? (
|
||||
<ActivityIndicator color={colors.textSecondary} />
|
||||
) : detailsFailed ? (
|
||||
<Text style={styles.empty}>Could not load comments. Tap refresh to try again.</Text>
|
||||
) : (
|
||||
<View style={styles.list}>
|
||||
{comments.length === 0 ? (
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<PRSection
|
||||
title="Reviewers"
|
||||
trailing={
|
||||
<Pressable
|
||||
style={styles.iconButton}
|
||||
onPress={() => setPickerOpen(true)}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel="Add or remove reviewers"
|
||||
>
|
||||
<UserPlus size={16} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
</Pressable>
|
||||
}
|
||||
const addButton = (
|
||||
<Pressable
|
||||
style={styles.iconButton}
|
||||
onPress={() => setPickerOpen(true)}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel="Add or remove reviewers"
|
||||
>
|
||||
{rows.length === 0 ? (
|
||||
<UserPlus size={16} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
</Pressable>
|
||||
)
|
||||
|
||||
return (
|
||||
<PRSection title="Reviewers" trailing={addButton}>
|
||||
{loadingDetails ? (
|
||||
<View style={styles.reviewersStatus}>
|
||||
<ActivityIndicator color={colors.textSecondary} />
|
||||
<Text style={styles.emptyText}>Loading reviewers…</Text>
|
||||
</View>
|
||||
) : detailsFailed ? (
|
||||
<Text style={styles.emptyText}>Could not load reviewers. Tap refresh to try again.</Text>
|
||||
) : rows.length === 0 ? (
|
||||
<Text style={styles.emptyText}>No reviewers requested</Text>
|
||||
) : (
|
||||
rows.map((row) => {
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<View style={styles.section}>
|
||||
<View style={styles.sectionHeader}>
|
||||
<Text style={styles.sectionLabel}>{title}</Text>
|
||||
{trailing ? <View style={styles.sectionHeaderTrailing}>{trailing}</View> : null}
|
||||
</View>
|
||||
{showHeader ? (
|
||||
<View style={styles.sectionHeader}>
|
||||
{title ? <Text style={styles.sectionLabel}>{title}</Text> : null}
|
||||
{trailing ? <View style={styles.sectionHeaderTrailing}>{trailing}</View> : null}
|
||||
</View>
|
||||
) : null}
|
||||
<View style={styles.sectionBody}>{children}</View>
|
||||
</View>
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<View style={styles.section}>
|
||||
<View style={styles.sectionBody}>
|
||||
<View style={styles.badgeRow}>
|
||||
const body = (
|
||||
<>
|
||||
<View style={styles.metaRow}>
|
||||
<View style={styles.metaLeft}>
|
||||
<Pressable
|
||||
onPress={openPr}
|
||||
disabled={!openPr}
|
||||
|
|
@ -50,50 +58,60 @@ export function PRSidebarHeader({ pr, details, titleAction }: Props) {
|
|||
>
|
||||
<Text style={[styles.badgeText, { color: badgeColor }]}>{badge.label}</Text>
|
||||
</Pressable>
|
||||
{openPr ? (
|
||||
<Pressable
|
||||
onPress={openPr}
|
||||
hitSlop={8}
|
||||
accessibilityRole="link"
|
||||
accessibilityLabel={`Open pull request #${pr.number} in browser`}
|
||||
style={({ pressed }) => [styles.iconButton, pressed && { opacity: 0.6 }]}
|
||||
>
|
||||
<ExternalLink size={16} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
</Pressable>
|
||||
) : null}
|
||||
<Text
|
||||
style={styles.prMetaStrong}
|
||||
onPress={openPr}
|
||||
accessibilityRole="link"
|
||||
accessibilityLabel={`Open pull request #${pr.number} on the web`}
|
||||
>
|
||||
#{pr.number}
|
||||
</Text>
|
||||
{author ? <Text style={styles.prMeta}>· {author}</Text> : null}
|
||||
</View>
|
||||
<PRTitle
|
||||
title={title}
|
||||
number={pr.number}
|
||||
editable={editable}
|
||||
openPr={openPr}
|
||||
titleAction={titleAction}
|
||||
/>
|
||||
{author ? <Text style={styles.prMeta}>by {author}</Text> : null}
|
||||
{baseRef && headRef ? (
|
||||
// head -> base reads in merge direction (desktop ChecksPanel parity).
|
||||
<View style={styles.branchRow}>
|
||||
<Text style={styles.branchPill}>{headRef}</Text>
|
||||
<ArrowRight size={12} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
<Text style={styles.branchPill}>{baseRef}</Text>
|
||||
</View>
|
||||
{showOpenOnWeb && openPr ? (
|
||||
<Pressable
|
||||
onPress={openPr}
|
||||
hitSlop={8}
|
||||
accessibilityRole="link"
|
||||
accessibilityLabel={`Open pull request #${pr.number} in browser`}
|
||||
style={({ pressed }) => [styles.iconButton, pressed && { opacity: 0.6 }]}
|
||||
>
|
||||
<ExternalLink size={16} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
</Pressable>
|
||||
) : null}
|
||||
</View>
|
||||
<PRTitle title={title} editable={editable} titleAction={titleAction} />
|
||||
{baseRef && headRef ? (
|
||||
<View style={styles.branchRow}>
|
||||
<Text style={styles.branchPill} numberOfLines={1}>
|
||||
{headRef}
|
||||
</Text>
|
||||
<ArrowRight size={12} color={colors.textSecondary} strokeWidth={2.2} />
|
||||
<Text style={styles.branchPill} numberOfLines={1}>
|
||||
{baseRef}
|
||||
</Text>
|
||||
</View>
|
||||
) : null}
|
||||
</>
|
||||
)
|
||||
|
||||
if (bare) {
|
||||
return <View style={styles.identityBlock}>{body}</View>
|
||||
}
|
||||
return (
|
||||
<View style={styles.section}>
|
||||
<View style={styles.sectionBody}>{body}</View>
|
||||
</View>
|
||||
)
|
||||
}
|
||||
|
||||
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 (
|
||||
<View>
|
||||
<View style={composerStyles.container}>
|
||||
<TextInput
|
||||
style={composerStyles.input}
|
||||
value={draft}
|
||||
|
|
@ -165,17 +183,7 @@ function PRTitle({
|
|||
accessibilityRole={editable ? 'button' : undefined}
|
||||
accessibilityLabel={editable ? 'Edit pull request title' : undefined}
|
||||
>
|
||||
<Text style={styles.prTitle}>
|
||||
{title}{' '}
|
||||
<Text
|
||||
style={styles.prMeta}
|
||||
onPress={openPr}
|
||||
accessibilityRole="link"
|
||||
accessibilityLabel={`Open pull request #${number} on the web`}
|
||||
>
|
||||
#{number}
|
||||
</Text>
|
||||
</Text>
|
||||
<Text style={styles.prTitle}>{title}</Text>
|
||||
{editable ? (
|
||||
<View style={styles.titleEditButton}>
|
||||
<Pencil size={14} color={colors.textSecondary} strokeWidth={2} />
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
<Text style={styles.emptyText}>No matching people</Text>
|
||||
</View>
|
||||
) : (
|
||||
<FlatList
|
||||
style={styles.pickerList}
|
||||
data={ordered}
|
||||
keyExtractor={(u) => u.login}
|
||||
keyboardShouldPersistTaps="handled"
|
||||
renderItem={({ item }) => {
|
||||
<View style={styles.pickerList}>
|
||||
{ordered.map((item) => {
|
||||
const requested = isRequested(item.login)
|
||||
return (
|
||||
<Pressable
|
||||
key={item.login}
|
||||
style={styles.pickerRow}
|
||||
onPress={() => onToggle(item.login)}
|
||||
accessibilityRole="button"
|
||||
|
|
@ -134,8 +138,8 @@ export function ReviewerPickerDrawer({
|
|||
</View>
|
||||
</Pressable>
|
||||
)
|
||||
}}
|
||||
/>
|
||||
})}
|
||||
</View>
|
||||
)}
|
||||
</BottomDrawer>
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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' })
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -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])
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
|
||||
<MobileSourceControlSegments active={activeTab} onSelect={selectTab} />
|
||||
|
||||
{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) ? (
|
||||
<MobileSourceControlBranchCard
|
||||
branchLabel={branchLabel}
|
||||
syncLabel={syncLabel}
|
||||
|
|
|
|||
|
|
@ -52,7 +52,7 @@ describe('buildMobileSourceControlActions', () => {
|
|||
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()
|
||||
})
|
||||
|
|
|
|||
|
|
@ -217,7 +217,7 @@ export function buildMobileSourceControlActions(
|
|||
onPress: handlers.checkout
|
||||
},
|
||||
{
|
||||
label: 'History',
|
||||
label: 'Commits',
|
||||
iconKey: 'history',
|
||||
disabled: busy,
|
||||
onPress: handlers.history
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ export const SOURCE_CONTROL_HUB_TABS: readonly SourceControlHubTab[] = [
|
|||
export const SOURCE_CONTROL_HUB_TAB_LABELS: Record<SourceControlHubTab, string> = {
|
||||
changes: 'Changes',
|
||||
pr: 'Pull Request',
|
||||
history: 'History'
|
||||
history: 'Commits'
|
||||
}
|
||||
|
||||
// Normalize a route param (possibly an array from expo-router, possibly unknown)
|
||||
|
|
|
|||
|
|
@ -848,6 +848,43 @@ async function fetchIssueWorkItem(
|
|||
return mapIssueWorkItem(JSON.parse(stdout) as Record<string, unknown>)
|
||||
}
|
||||
|
||||
// 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<Pick<MainWorkItem, 'reviewRequests' | 'latestReviews'>> {
|
||||
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<string, unknown>
|
||||
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<string, unknown>
|
||||
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<string, unknown>, ownerRepo)
|
||||
const mapped = mapPullRequestWorkItem(
|
||||
JSON.parse(stdout) as Record<string, unknown>,
|
||||
ownerRepo
|
||||
)
|
||||
const reviewFields = await fetchPullRequestReviewFields(number, ownerRepo, ghOptions)
|
||||
return { ...mapped, ...reviewFields }
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue