diff --git a/apps/mobile/src/app/(app)/agent-chat/[session-id].mounted.test.tsx b/apps/mobile/src/app/(app)/agent-chat/[session-id].mounted.test.tsx index 11353f6908..faa6896e98 100644 --- a/apps/mobile/src/app/(app)/agent-chat/[session-id].mounted.test.tsx +++ b/apps/mobile/src/app/(app)/agent-chat/[session-id].mounted.test.tsx @@ -17,15 +17,13 @@ import { } from '@kilocode/cloud-agent-sdk'; import { kiloId, stubTextPart, stubUserMessage } from '@kilocode/cloud-agent-sdk/test-helpers'; -import { i18n } from '@/i18n'; -import { sessionResumeUrl } from '@kilocode/app-shared/universal-links'; import { AgentSessionProvider, useSessionManager } from '@/components/agents/session-provider'; -import { SessionCopyLinkAction } from '@/components/agents/session-copy-link-action'; import { UserWebConnectionProvider } from '@/components/agents/user-web-connection-provider'; import { useSessionDetailRename } from '@/components/agents/use-session-detail-rename'; import { QueryError } from '@/components/query-error'; import { ScreenHeader } from '@/components/screen-header'; import { Button } from '@/components/ui/button'; +import { i18n } from '@/i18n'; import { clearActiveToken, setActiveToken, setSignOutTeardownActive } from '@/lib/auth/token-owner'; import { bumpAuthEpoch, currentAuthEpoch } from '@/lib/auth/auth-epoch'; import { setSignOutActive } from '@/lib/auth/sign-out-state'; @@ -671,104 +669,32 @@ describe('SessionDetailScreen valid session-id', () => { expect(propOf(findByType(renderer.root, 'SessionDetailContent')[0], 'resumeAt')).toBeNull(); }); - it('keeps the route anchor on the loading header Copy-link action', async () => { + // Owner request item 4 moved the Copy link action off the conversation header + // and into the context details sheet. The loading header therefore reserves + // the loaded header's context pill only: it carries no copy control, because + // the sheet — the copy affordance's home — mounts with SessionDetailContent + // below. Rendering one here would resurrect the control the request removed + // and shift the pill at the loading -> loaded swap. + it('reserves the context pill without a copy control on the loading header', async () => { useLocalSearchParamsMock.mockReturnValue({ 'session-id': 'sess-1', at: 'msg_42' }); queryState.data = null; queryState.isPending = true; const renderer = await mountRoute(); - // The transcript has not loaded, so the skeleton header is mounted... expect(findByType(renderer.root, 'SessionSkeletonMessages')).toHaveLength(1); expect(findByType(renderer.root, 'SessionDetailContent')).toHaveLength(0); - // ...and its Copy-link action must copy the position the route already - // holds, not an anchor-less session link. - const copyActions = renderer.root.findByType(ScreenHeader).findAllByType(SessionCopyLinkAction); - expect(copyActions).toHaveLength(1); - expect(propOf(copyActions[0], 'anchorMessageId')).toBe('msg_42'); - }); -}); - -// The session header's Copy-link action copies the same universal link the OS -// handoff advertises, anchored at the position the transcript is showing. -describe('SessionDetailScreen copy link action', () => { - beforeEach(() => { - clipboardSetStringAsync.mockReset(); - hapticsSelection.mockReset(); - toastSuccess.mockReset(); - toastError.mockReset(); - }); - - async function mountCopyAction(anchorMessageId: string | null) { - const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; - await act(async () => { - ref.current = TestRenderer.create( - createElement(SessionCopyLinkAction, { sessionId: 'sess-1', anchorMessageId }) - ); - await Promise.resolve(); - }); - if (!ref.current) { - throw new Error('copy action did not render'); - } - const renderer = ref.current; - onTestFinished(() => { - act(() => { - renderer.unmount(); - }); - }); - return renderer; - } - - function copyControl(renderer: TestRenderer.ReactTestRenderer) { - return renderer.root.findByProps({ accessibilityLabel: i18n.t('common.copyLink') }); - } - - it('copies the resume URL of the shown position and confirms it', async () => { - clipboardSetStringAsync.mockResolvedValue(true); - const renderer = await mountCopyAction('msg_42'); - - await act(async () => { - pressControl(copyControl(renderer)); - await Promise.resolve(); - }); - - expect(clipboardSetStringAsync).toHaveBeenCalledWith( - sessionResumeUrl({ sessionId: 'sess-1', anchorMessageId: 'msg_42' }) - ); - expect(toastSuccess).toHaveBeenCalledWith(i18n.t('agentChat.chatLink.linkCopied'), { - // Longer than the Sonner default: on Android the system clipboard preview - // covers the bottom-center toast region for its whole default life. - duration: expect.any(Number), - }); - expect(hapticsSelection).toHaveBeenCalledTimes(1); - }); - - it('surfaces a retryable failure when the clipboard rejects', async () => { - clipboardSetStringAsync.mockRejectedValueOnce(new Error('clipboard unavailable')); - const renderer = await mountCopyAction('msg_42'); - - await act(async () => { - pressControl(copyControl(renderer)); - await Promise.resolve(); - }); - - expect(toastError).toHaveBeenCalledWith(i18n.t('agentChat.chatLink.couldNotCopyLink'), { - action: { label: i18n.t('common.tryAgain'), onClick: expect.any(Function) }, - }); - expect(toastSuccess).not.toHaveBeenCalled(); - }); - - it('copies the session link without a position when the position is unknown', async () => { - clipboardSetStringAsync.mockResolvedValue(true); - const renderer = await mountCopyAction(null); - - await act(async () => { - pressControl(copyControl(renderer)); - await Promise.resolve(); - }); - - expect(clipboardSetStringAsync).toHaveBeenCalledWith( - sessionResumeUrl({ sessionId: 'sess-1', anchorMessageId: null }) - ); + const header = renderer.root.findByType(ScreenHeader); + const metrics = findByType(header, 'SessionContextMetrics'); + expect(metrics).toHaveLength(1); + expect(propOf(metrics[0], 'loading')).toBe(true); + // No `onPress`: the context sheet, which owns both copy rows, is not + // mounted until SessionDetailContent takes over. + expect(propOf(metrics[0], 'onPress')).toBeUndefined(); + expect( + findByType(header, 'Pressable').filter( + node => propOf(node, 'accessibilityLabel') === i18n.t('common.copyLink') + ) + ).toHaveLength(0); }); }); @@ -1426,20 +1352,6 @@ describe.each([ expect(findByType(renderer.root, 'SessionSkeletonMessages')).toHaveLength(1); expect(findByType(renderer.root, 'SessionDetailContent')).toHaveLength(0); expect(transcriptText(renderer, 'RootText')).toBe(''); - // The loading header reserves the loaded header's Copy-link action, so the - // 44pt control appearing at the swap cannot narrow and re-wrap the title. - const loadingHeader = renderer.root.findByType(ScreenHeader); - const loadingCopyActions = loadingHeader.findAllByType(SessionCopyLinkAction); - expect(loadingCopyActions).toHaveLength(1); - const loadingCopyAction = loadingCopyActions[0]; - if (!loadingCopyAction) { - throw new Error('loading header did not render the copy-link action'); - } - expect(propOf(loadingCopyAction, 'sessionId')).toBe('sess-1'); - expect(propOf(loadingCopyAction, 'anchorMessageId')).toBeNull(); - const loadingCopyPressable = loadingCopyAction.findAllByType('Pressable'); - expect(loadingCopyPressable).toHaveLength(1); - expect(propOf(loadingCopyPressable[0], 'className')).toContain('h-11 w-11'); await act(async () => { identity.resolve({ id: 'user-B' }); await vi.advanceTimersByTimeAsync(0); diff --git a/apps/mobile/src/app/(app)/agent-chat/[session-id].tsx b/apps/mobile/src/app/(app)/agent-chat/[session-id].tsx index 8b6a81e368..83842eb853 100644 --- a/apps/mobile/src/app/(app)/agent-chat/[session-id].tsx +++ b/apps/mobile/src/app/(app)/agent-chat/[session-id].tsx @@ -17,7 +17,6 @@ import { SessionSkeletonMessages, } from '@/components/agents/session-detail-skeleton'; import { SessionContextMetrics } from '@/components/agents/session-context-metrics'; -import { SessionCopyLinkAction } from '@/components/agents/session-copy-link-action'; import { AgentSessionProvider } from '@/components/agents/session-provider'; import { useIdentityConfirmation } from '@/components/agents/user-web-connection-provider'; import { buildTerminalErrorCopyText } from '@/components/agents/session-terminal-error'; @@ -154,11 +153,12 @@ export default function SessionDetailScreen() { ) { // The composer placeholder holds its own height: nothing may shift when // the query resolves. Route title hints are not bound to an account. - // The right cluster reserves the loaded header's Copy-link action too, so - // the 44pt control appearing at the swap cannot narrow and re-wrap the - // title. The route already holds the `?at=` anchor, so copying the link - // while the transcript loads keeps the same position the loaded header - // falls back to; with no usable anchor it copies the session-top link. + // The loading header reserves the loaded header's context pill (the loaded + // right cluster is that pill plus an optional PR badge) so the swap cannot + // re-wrap the title. Copying the session link belongs to the context + // details sheet, which mounts with SessionDetailContent below, so this + // header deliberately renders no copy control while the session is + // unresolved. return ( - } /> diff --git a/apps/mobile/src/components/agents/session-context-sheet.mounted.test.tsx b/apps/mobile/src/components/agents/session-context-sheet.mounted.test.tsx index 2e9c3ea36a..b94089e54d 100644 --- a/apps/mobile/src/components/agents/session-context-sheet.mounted.test.tsx +++ b/apps/mobile/src/components/agents/session-context-sheet.mounted.test.tsx @@ -4,6 +4,7 @@ import { type ComponentProps, createElement, type ReactElement } from 'react'; import type * as ReactI18next from 'react-i18next'; import { act, TestRenderer } from '@/test/renderer'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { sessionResumeUrl } from '@kilocode/app-shared/universal-links'; import { Text } from '@/components/ui/text'; import { i18n } from '@/i18n'; @@ -17,6 +18,8 @@ const holder = vi.hoisted(() => ({ isPending: false, copied: [] as string[], copyResult: true as boolean | Promise, + copiedLinks: [] as { sessionId: string; anchorMessageId: string | null }[], + linkCopyResult: true as boolean | Promise, })); vi.mock('@tanstack/react-query', () => ({ @@ -33,6 +36,11 @@ vi.mock('./session-row-actions', () => ({ holder.copied.push(id); return holder.copyResult; }, + copySessionLink: async (sessionId: string, anchorMessageId: string | null) => { + await Promise.resolve(); + holder.copiedLinks.push({ sessionId, anchorMessageId }); + return holder.linkCopyResult; + }, })); vi.mock('react-i18next', async importOriginal => { const actual = await importOriginal(); @@ -101,6 +109,7 @@ function renderSheet( visible: true, info: INFO, sessionId: 'ses-123', + anchorMessageId: null, sessionTitle: 'Greeting', activeSessionType: null, ownerConnectionId: null, @@ -280,6 +289,7 @@ function sheetElement( visible: true, info: INFO, sessionId: 'ses-123', + anchorMessageId: null, sessionTitle: 'Greeting', activeSessionType: null, ownerConnectionId: null, @@ -340,6 +350,8 @@ beforeEach(() => { holder.isPending = false; holder.copied = []; holder.copyResult = true; + holder.copiedLinks = []; + holder.linkCopyResult = true; }); describe('SessionContextSheet session id and running on', () => { @@ -508,6 +520,93 @@ describe('SessionContextSheet session id and running on', () => { }); }); +describe('SessionContextSheet copy link row', () => { + it('shows the resume URL as the row value and copies it from the call to action', async () => { + const renderer = await mountSheet(); + const expected = sessionResumeUrl({ sessionId: 'ses-123', anchorMessageId: null }); + expect(textValues(renderer)).toContain(i18n.t('common.copyLink')); + expect(textValues(renderer)).toContain(expected); + + await act(async () => { + pressByTestID(renderer, 'session-context-sheet-copy-link'); + await Promise.resolve(); + }); + + expect(holder.copiedLinks).toEqual([{ sessionId: 'ses-123', anchorMessageId: null }]); + const values = textValues(renderer); + expect(values).toContain(i18n.t('agentChat.chatLink.linkCopied')); + // The row keeps its call-to-action name beside the outcome, so the sheet + // still names the row after the copy completes. + expect(values).toContain(i18n.t('common.copyLink')); + await unmount(renderer); + }); + + it('copies the row value anchored at the position the sheet was given', async () => { + const renderer = await mountSheet({ anchorMessageId: 'msg-42' }); + const expected = sessionResumeUrl({ sessionId: 'ses-123', anchorMessageId: 'msg-42' }); + expect(textValues(renderer)).toContain(expected); + + await act(async () => { + pressByTestID(renderer, 'session-context-sheet-copy-link'); + await Promise.resolve(); + }); + + expect(holder.copiedLinks).toEqual([{ sessionId: 'ses-123', anchorMessageId: 'msg-42' }]); + await unmount(renderer); + }); + + it('shows the could-not-copy outcome and retries from the same row', async () => { + holder.linkCopyResult = false; + const renderer = await mountSheet(); + await act(async () => { + pressByTestID(renderer, 'session-context-sheet-copy-link'); + await Promise.resolve(); + }); + const values = textValues(renderer); + expect(values).toContain(i18n.t('agentChat.chatLink.couldNotCopyLink')); + expect(values).not.toContain(i18n.t('agentChat.chatLink.linkCopied')); + expect(values).toContain(i18n.t('common.copyLink')); + + holder.linkCopyResult = true; + await act(async () => { + pressByTestID(renderer, 'session-context-sheet-copy-link'); + await Promise.resolve(); + }); + expect(holder.copiedLinks).toHaveLength(2); + expect(textValues(renderer)).toContain(i18n.t('agentChat.chatLink.linkCopied')); + expect(textValues(renderer)).not.toContain(i18n.t('agentChat.chatLink.couldNotCopyLink')); + await unmount(renderer); + }); + + it('resets the link feedback to the call to action when the sheet closes', async () => { + const renderer = await mountSheet(); + await act(async () => { + pressByTestID(renderer, 'session-context-sheet-copy-link'); + await Promise.resolve(); + }); + expect(textValues(renderer)).toContain(i18n.t('agentChat.chatLink.linkCopied')); + await act(async () => { + renderer.update(sheetElement({ visible: false })); + await Promise.resolve(); + }); + expect(textValues(renderer)).toContain(i18n.t('common.copyLink')); + expect(textValues(renderer)).not.toContain(i18n.t('agentChat.chatLink.linkCopied')); + await unmount(renderer); + }); + + it('keeps the link feedback independent of the session id feedback', async () => { + const renderer = await mountSheet(); + await act(async () => { + pressByTestID(renderer, 'session-context-sheet-copy-link'); + await Promise.resolve(); + }); + const values = textValues(renderer); + expect(values).toContain(i18n.t('agentChat.chatLink.linkCopied')); + expect(values).not.toContain(i18n.t('agents.sessionRow.idCopied')); + await unmount(renderer); + }); +}); + describe('SessionContextSheet connection row', () => { it.each([ { display: 'connected' as const, copy: 'common.connected' }, diff --git a/apps/mobile/src/components/agents/session-context-sheet.tsx b/apps/mobile/src/components/agents/session-context-sheet.tsx index f31f6f6318..38ae016ed9 100644 --- a/apps/mobile/src/components/agents/session-context-sheet.tsx +++ b/apps/mobile/src/components/agents/session-context-sheet.tsx @@ -5,6 +5,7 @@ import { Pressable, ScrollView, View } from 'react-native'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { useTranslation } from 'react-i18next'; import { ChevronDown } from '@/components/ui/icons'; +import { sessionResumeUrl } from '@kilocode/app-shared/universal-links'; import { type ResolvedSession, type StoredMessage } from '@kilocode/cloud-agent-sdk'; import { SheetHeader } from '@/components/sheet-header'; @@ -40,13 +41,15 @@ import { } from './session-cost-breakdown'; import { friendlyModelName, resolveModelProviderName } from './session-model-display'; import { SessionPageSheet } from './session-page-sheet'; -import { copySessionId } from './session-row-actions'; +import { copySessionId, copySessionLink } from './session-row-actions'; import { type SessionConnectionDisplay } from './session-connection-indicator-state'; type SessionContextSheetProps = { visible: boolean; info: SessionContextInfo | undefined; sessionId: string; + /** Message the copied link resumes at; null copies the session link without a position. */ + anchorMessageId: string | null; sessionTitle: string; activeSessionType: ResolvedSession['type'] | null; ownerConnectionId: string | null; @@ -81,20 +84,66 @@ type RunningOnState = { kind: 'hidden' } | { kind: 'pending' } | { kind: 'label' type CopyFeedbackState = 'idle' | 'copied' | 'failed'; -function copyStatusLabel(state: CopyFeedbackState, t: (key: string) => string): string | null { +/** Catalog keys for a copy row's inline outcome, per outcome. */ +type CopyRowMessages = { readonly copied: string; readonly failed: string }; + +function copyStatusLabel( + state: CopyFeedbackState, + t: (key: string) => string, + messages: CopyRowMessages +): string | null { if (state === 'copied') { - return t('agents.sessionRow.idCopied'); + return t(messages.copied); } if (state === 'failed') { - return t('agents.sessionRow.couldNotCopyId'); + return t(messages.failed); } return null; } +/** + * Inline feedback for a copy row inside the sheet. sonner toasts render in the + * app root, behind this Modal's window, so the row shows the outcome itself + * instead of relying on the toast. Closing the sheet clears the feedback and + * invalidates pending results so a reopen starts from the call to action even + * if an earlier copy finishes late. + */ +function useCopyRowFeedback( + copy: () => Promise, + messages: CopyRowMessages, + visible: boolean +) { + const { t } = useTranslation(); + const [state, setState] = useState('idle'); + const generation = useRef(0); + useEffect(() => { + if (!visible) { + setState('idle'); + } + return () => { + generation.current += 1; + }; + }, [visible]); + return { + state, + status: copyStatusLabel(state, t, messages), + handlePress: () => { + void (async () => { + const current = generation.current; + const success = await copy(); + if (current === generation.current) { + setState(success ? 'copied' : 'failed'); + } + })(); + }, + }; +} + export function SessionContextSheet({ visible, info, sessionId, + anchorMessageId, sessionTitle, activeSessionType, ownerConnectionId, @@ -113,21 +162,22 @@ export function SessionContextSheet({ const insets = useSafeAreaInsets(); const { t } = useTranslation(); const runningOn = useRunningOnLabel(activeSessionType, ownerConnectionId, visible); - const [copyState, setCopyState] = useState('idle'); - const copyFeedbackGeneration = useRef(0); - // sonner toasts render in the app root, behind this Modal's window, so the - // copy row shows the outcome inline instead of relying on the toast. - // Closing the sheet clears feedback and invalidates pending results so a - // reopen starts from the CTA even if an earlier copy finishes late. - useEffect(() => { - if (!visible) { - setCopyState('idle'); - } - return () => { - copyFeedbackGeneration.current += 1; - }; - }, [visible]); - const copyStatus = copyStatusLabel(copyState, t); + const idCopy = useCopyRowFeedback( + async () => { + const copied = await copySessionId(sessionId); + return copied; + }, + { copied: 'agents.sessionRow.idCopied', failed: 'agents.sessionRow.couldNotCopyId' }, + visible + ); + const linkCopy = useCopyRowFeedback( + async () => { + const copied = await copySessionLink(sessionId, anchorMessageId); + return copied; + }, + { copied: 'agentChat.chatLink.linkCopied', failed: 'agentChat.chatLink.couldNotCopyLink' }, + visible + ); let connectionLabel = t('agentChat.sessionConnection.connecting'); if (connectionDisplay === 'connected') { connectionLabel = t('common.connected'); @@ -272,43 +322,26 @@ export function SessionContextSheet({ - { - void (async () => { - const generation = copyFeedbackGeneration.current; - const success = await copySessionId(sessionId); - if (generation === copyFeedbackGeneration.current) { - setCopyState(success ? 'copied' : 'failed'); - } - })(); - }} - accessibilityRole="button" - className="gap-1 active:opacity-70" + - {/* The row keeps its call-to-action name in every state; the copy - outcome renders beside it, so one capture of the sheet shows - both the row the scenario names and the feedback it demands. - The child texts are the accessible name in reading order. */} - - - {t('agents.sessionRow.copyId')} - - {copyStatus ? ( - - {copyStatus} - - ) : null} - - - {sessionId} - - + label={t('agents.sessionRow.copyId')} + value={sessionId} + state={idCopy.state} + status={idCopy.status} + onPress={idCopy.handlePress} + /> + + {/* The link row sits under the id row: both copy this session's + value, and the sheet keeps the destination (resume URL) visible + beside its call to action. */} + {runningOn.kind !== 'hidden' ? ( @@ -426,6 +459,52 @@ function useRunningOnLabel( return isRemote && isPending ? { kind: 'pending' } : { kind: 'hidden' }; } +function CopyRow({ + testID, + label, + value, + state, + status, + onPress, +}: Readonly<{ + testID: string; + label: string; + value: string; + state: CopyFeedbackState; + status: string | null; + onPress: () => void; +}>) { + return ( + + {/* The row keeps its call-to-action name in every state; the copy + outcome renders beside it, so one capture of the sheet shows both the + row the scenario names and the feedback it demands. The child texts + are the accessible name in reading order. */} + + {label} + {status ? ( + + {status} + + ) : null} + + + {value} + + + ); +} + function Row({ label, children }: Readonly<{ label: string; children: React.ReactNode }>) { return ( diff --git a/apps/mobile/src/components/agents/session-copy-link-action.tsx b/apps/mobile/src/components/agents/session-copy-link-action.tsx deleted file mode 100644 index 61f11e26d4..0000000000 --- a/apps/mobile/src/components/agents/session-copy-link-action.tsx +++ /dev/null @@ -1,42 +0,0 @@ -import { sessionResumeUrl } from '@kilocode/app-shared/universal-links'; -import * as Haptics from 'expo-haptics'; -import { Pressable } from 'react-native'; -import { useTranslation } from 'react-i18next'; - -import { performChatLinkAction } from '@/components/agents/chat-link-actions'; -import { Link2 } from '@/components/ui/icons'; -import { useThemeColors } from '@/lib/hooks/use-theme-colors'; - -type SessionCopyLinkActionProps = { - readonly sessionId: string; - /** Message the link resumes at; null copies the session link without a position. */ - readonly anchorMessageId: string | null; -}; - -/** - * The session header's Copy-link action. It copies the same universal link the - * OS handoff advertises (`sessionResumeUrl`), so the two can never drift, and - * reuses the chat-link copy path for the success toast and the retryable - * failure toast. - */ -export function SessionCopyLinkAction({ sessionId, anchorMessageId }: SessionCopyLinkActionProps) { - const { t } = useTranslation(); - const colors = useThemeColors(); - - return ( - { - // Selection haptic for the commit: a capability iOS and Android both - // have, served by the one cross-platform call. - void Haptics.selectionAsync(); - void performChatLinkAction('copy', sessionResumeUrl({ sessionId, anchorMessageId })); - }} - className="h-11 w-11 shrink-0 items-center justify-center active:opacity-70" - > - - - ); -} diff --git a/apps/mobile/src/components/agents/session-detail-content.test.ts b/apps/mobile/src/components/agents/session-detail-content.test.ts index deb9c32ba0..b8fa16c5c0 100644 --- a/apps/mobile/src/components/agents/session-detail-content.test.ts +++ b/apps/mobile/src/components/agents/session-detail-content.test.ts @@ -9,8 +9,6 @@ import { } from 'react'; import { createStore, Provider } from 'jotai'; import { QueryClientProvider } from '@tanstack/react-query'; -import * as Clipboard from 'expo-clipboard'; -import { sessionResumeUrl } from '@kilocode/app-shared/universal-links'; import { act, type ReactTestInstance, type ReactTestRenderer } from '@/test/renderer'; import { type Pressable } from 'react-native'; import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest'; @@ -230,12 +228,14 @@ vi.mock('@/components/agents/context-usage-ring', () => ({ ContextUsageRing: 'ContextUsageRing', })); // The real context sheet (rendered so the auto-approve row can be asserted) -// reaches `copySessionId`, which imports the native `expo-clipboard` module that -// cannot load in this DOM-free node suite. Mock the boundary, as the mounted -// context-sheet suite does. The session header's copy-link action reaches the -// same native module through the real chat-link copy path. +// reaches `copySessionId`/`copySessionLink`, which import the native +// `expo-clipboard` module that cannot load in this DOM-free node suite. Mock +// the boundary, as the mounted context-sheet suite does. vi.mock('expo-clipboard', () => ({ setStringAsync: vi.fn() })); -vi.mock('@/components/agents/session-row-actions', () => ({ copySessionId: vi.fn() })); +vi.mock('@/components/agents/session-row-actions', () => ({ + copySessionId: vi.fn(), + copySessionLink: vi.fn(), +})); // The copy-link path reaches the browser helper; its native module cannot load here. vi.mock('@/lib/external-link', () => ({ openExternalUrl: vi.fn() })); // The handoff advertiser owns the OS entry point (Head plus Android's launcher @@ -2291,11 +2291,10 @@ describe('SessionDetailContent goal edit dialog', () => { }); // The screen's live position: the transcript list reports the topmost visible -// message, and the screen publishes it to the OS handoff, the copy-link action, -// and the route's search params. +// message, and the screen publishes it to the OS handoff and the route's +// search params. describe('SessionDetailContent live position', () => { - it('publishes the transcript position to the handoff, the copy action, and the route', async () => { - vi.mocked(Clipboard.setStringAsync).mockResolvedValue(true); + it('publishes the transcript position to the handoff and the route', async () => { routerSetParams.mockClear(); handoffAdvertiserCalls.props.length = 0; const view = await mountDetails([childMessage(ROOT_ID, 'shown row')]); @@ -2311,18 +2310,6 @@ describe('SessionDetailContent live position', () => { // The handoff advertises the position the transcript is showing. expect(handoffAdvertiserCalls.props.at(-1)?.anchorMessageId).toBe('msg-77'); - // The header's copy action copies that same position's universal link. - const copy = view.renderer.root.findByProps({ - accessibilityLabel: i18n.t('common.copyLink'), - }); - await act(async () => { - (copy.props as { onPress: () => void }).onPress(); - await Promise.resolve(); - }); - expect(Clipboard.setStringAsync).toHaveBeenCalledWith( - sessionResumeUrl({ sessionId: ROOT_ID, anchorMessageId: 'msg-77' }) - ); - // The route's search params carry it after the publish debounce. await act(async () => { await new Promise(resolve => { diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index a80bf99318..d305a1df91 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -78,7 +78,6 @@ import { useSessionAutoApproveEnabled, } from '@/components/agents/session-auto-approve'; import { SessionPrBadge } from '@/components/agents/session-pr-badge'; -import { SessionCopyLinkAction } from '@/components/agents/session-copy-link-action'; import { selectSessionCostInputs } from '@/components/agents/session-list-helpers'; import { buildRemoteAttachmentParts } from '@/components/agents/mobile-session-manager-helpers'; import { isCancelQueuedUpgradeRequired } from '@/components/agents/mobile-session-manager'; @@ -1524,7 +1523,6 @@ export function SessionDetailContent({ }); }} /> - ); const blockingInteraction = getBlockingInteraction({ activeQuestion, activePermission }); @@ -1943,6 +1941,7 @@ export function SessionDetailContent({ visible={sheetMountState.visible} info={sheetMountState.info} sessionId={sessionId} + anchorMessageId={anchor ?? resumeAnchor} sessionTitle={rename.title} activeSessionType={activeSessionType} ownerConnectionId={remoteModelState.ownerConnectionId} diff --git a/apps/mobile/src/components/agents/session-row-actions.test.ts b/apps/mobile/src/components/agents/session-row-actions.test.ts index 2f42c8cba1..30cc778668 100644 --- a/apps/mobile/src/components/agents/session-row-actions.test.ts +++ b/apps/mobile/src/components/agents/session-row-actions.test.ts @@ -1,6 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { sessionResumeUrl } from '@kilocode/app-shared/universal-links'; +import * as Clipboard from 'expo-clipboard'; +import * as Haptics from 'expo-haptics'; -import { showSessionActionMenu } from './session-row-actions'; +import { copySessionLink, showSessionActionMenu } from './session-row-actions'; const reactNativeMock = vi.hoisted(() => ({ alert: vi.fn(), @@ -202,3 +205,48 @@ describe('showSessionActionMenu', () => { expect(onDelete).not.toHaveBeenCalled(); }); }); + +describe('copySessionLink', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('copies the anchored resume URL and commits with the success haptic', async () => { + vi.mocked(Clipboard.setStringAsync).mockResolvedValue(true); + + await expect(copySessionLink('ses-1', 'msg_7')).resolves.toBe(true); + + expect(Clipboard.setStringAsync).toHaveBeenCalledWith( + sessionResumeUrl({ sessionId: 'ses-1', anchorMessageId: 'msg_7' }) + ); + expect(Haptics.notificationAsync).toHaveBeenCalledWith( + Haptics.NotificationFeedbackType.Success + ); + }); + + it('copies the session-top link when the position is unknown', async () => { + vi.mocked(Clipboard.setStringAsync).mockResolvedValue(true); + + await expect(copySessionLink('ses-1', null)).resolves.toBe(true); + + expect(Clipboard.setStringAsync).toHaveBeenCalledWith( + sessionResumeUrl({ sessionId: 'ses-1', anchorMessageId: null }) + ); + }); + + it('reports failure without the success haptic when the clipboard rejects', async () => { + vi.mocked(Clipboard.setStringAsync).mockRejectedValue(new Error('clipboard unavailable')); + + await expect(copySessionLink('ses-1', 'msg_7')).resolves.toBe(false); + + expect(Haptics.notificationAsync).not.toHaveBeenCalled(); + }); + + it('reports failure when the clipboard resolves false', async () => { + vi.mocked(Clipboard.setStringAsync).mockResolvedValue(false); + + await expect(copySessionLink('ses-1', null)).resolves.toBe(false); + + expect(Haptics.notificationAsync).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/mobile/src/components/agents/session-row-actions.ts b/apps/mobile/src/components/agents/session-row-actions.ts index fe6b50f6ff..f5aa58f2fe 100644 --- a/apps/mobile/src/components/agents/session-row-actions.ts +++ b/apps/mobile/src/components/agents/session-row-actions.ts @@ -1,4 +1,5 @@ import { type ActionSheetOptions } from '@expo/react-native-action-sheet'; +import { sessionResumeUrl } from '@kilocode/app-shared/universal-links'; import * as Clipboard from 'expo-clipboard'; import * as Haptics from 'expo-haptics'; import { Alert } from 'react-native'; @@ -56,6 +57,29 @@ export async function copySessionId(sessionId: string): Promise { } } +/** + * Copies the session's resume link — the same universal link the OS handoff + * advertises (`sessionResumeUrl`), anchored at the position the transcript is + * showing — and returns whether it succeeded. No toast: the copy-link row + * lives inside the context sheet, whose Modal window hides app-root toasts, so + * the caller renders the outcome inline from this result. + */ +export async function copySessionLink( + sessionId: string, + anchorMessageId: string | null +): Promise { + try { + const copied = await Clipboard.setStringAsync(sessionResumeUrl({ sessionId, anchorMessageId })); + if (!copied) { + throw new Error('Clipboard rejected session link'); + } + void Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); + return true; + } catch { + return false; + } +} + type SessionActionMenuOptions = { showActionSheetWithOptions: ( options: ActionSheetOptions, diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.test.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.test.tsx index 58417d4b46..4196d07add 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.test.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.test.tsx @@ -120,3 +120,26 @@ describe('PrDiffFileListHeader side insets (landscape)', () => { expect(routerPush).toHaveBeenCalledWith('/(app)/pr-review/octocat/hello/7/file-navigator'); }); }); + +describe('PrDiffFileListHeader container padding', () => { + beforeEach(() => { + insets.top = 0; + insets.bottom = 0; + insets.left = 0; + insets.right = 0; + }); + + it('pads the summary row above and below the file list', () => { + const renderer = mountHeader(); + const pressable = findNavigatorPressable(renderer.root); + const container = pressable.parent?.parent; + + // The owner reported the row sitting flush against the tab bar above and + // the first file path below; `py-4` gives 16pt on both edges while the + // `px-4` gutter, background and hairline are unchanged. + expect(container?.props.className).toContain('px-4'); + expect(container?.props.className).toContain('py-4'); + expect(container?.props.className).toContain('bg-background'); + expect(container?.props.className).toContain('border-b'); + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx index dea2e10338..656e66d914 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-file-list-header.tsx @@ -79,7 +79,7 @@ export function PrDiffFileListHeader({ }, [router, navigatorHref]); return ( - + { return (root.props as { style?: { paddingBottom?: number } }).style?.paddingBottom; } - it('pads the bar by 24 points at a zero inset', () => { - expect(rootPaddingBottom()).toBe(24); + it('floors the bar padding at 8 points at a zero inset', () => { + expect(rootPaddingBottom()).toBe(8); }); - it('adds the system inset to the 24-point base padding', () => { + it('clears the system inset when it exceeds the 8-point floor', () => { insets.bottom = 34; - expect(rootPaddingBottom()).toBe(58); + expect(rootPaddingBottom()).toBe(34); }); it('renders in-flow, not as an overlay over the list', () => { @@ -369,7 +369,7 @@ describe('PrDiffFloatingActions side insets (landscape)', () => { // portrait untouched (inline style wins over className). expect(style.paddingLeft).toBeUndefined(); expect(style.paddingRight).toBeUndefined(); - expect(style.paddingBottom).toBe(24); + expect(style.paddingBottom).toBe(8); }); it('clears the sensor housing with the landscape side insets', () => { @@ -381,6 +381,6 @@ describe('PrDiffFloatingActions side insets (landscape)', () => { expect(style.paddingRight).toBe(59); // The card shrink is horizontal-only: the paddingBottom that feeds the // measured onLayout height (and `prDiffListBottomPadding`) is unchanged. - expect(style.paddingBottom).toBe(24); + expect(style.paddingBottom).toBe(8); }); }); diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx index 587e0ee44b..9c13cddcbf 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-floating-actions.tsx @@ -67,11 +67,14 @@ export function PrDiffFloatingActions({ const pending = usePendingReview(); // The footer is an in-flow bar below the list, so no row is ever clipped // by it and nothing shows through around the opaque (`bg-background`) - // card. Its bottom padding must include the Android system inset. The - // landscape side insets (`insets.left` / `insets.right`) clear the sensor - // housing; like ScreenHeader they are spread only when nonzero, so the - // `px-4` gutter survives portrait (inline style wins over className), and - // they are horizontal-only, so the bottom padding stays untouched. + // card. Its bottom padding clears the Android system inset, floored at 8 + // points for devices that report none. It used to add a redundant 24 + // points on top of the inset, which left a blank band under the card + // (owner capture finish-review-space.png). The landscape side insets + // (`insets.left` / `insets.right`) clear the sensor housing; like + // ScreenHeader they are spread only when nonzero, so the `px-4` gutter + // survives portrait (inline style wins over className), and they are + // horizontal-only, so the bottom padding stays untouched. const insets = useSafeAreaInsets(); const showSelectionAction = viewMode === 'unified' && selection !== null; @@ -118,7 +121,7 @@ export function PrDiffFloatingActions({ 0 ? { paddingLeft: insets.left } : undefined), ...(insets.right > 0 ? { paddingRight: insets.right } : undefined), }} diff --git a/apps/mobile/src/components/pr-review/pr-review-screen.test.tsx b/apps/mobile/src/components/pr-review/pr-review-screen.test.tsx index 9e032e372d..6fe3f77c8d 100644 --- a/apps/mobile/src/components/pr-review/pr-review-screen.test.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-screen.test.tsx @@ -1,4 +1,4 @@ -/* eslint-disable max-lines -- Submit-review, share, and inset reachability tests share the direct-invocation screen harness. */ +/* eslint-disable max-lines -- Submit-review, share, merge, and inset reachability tests share the direct-invocation screen harness. */ // P1-F-46b: the "Submit review" affordance must be reachable from the // Overview tab (header right) and the Files tab (floating action bar, // see `pr-diff-floating-actions.test.tsx`). The Discussion tab is @@ -88,6 +88,7 @@ vi.mock('@tanstack/react-query', () => ({ vi.mock('@/components/ui/icons', () => ({ Check: () => null, + GitMerge: () => null, GitPullRequest: () => null, Share: () => null, })); @@ -288,7 +289,7 @@ describe('PrReviewScreen share action', () => { const element = PrReviewScreen({ owner: 'octocat', repo: 'hello', number: 7 }); const shareButton = findElement({ node: element, - type: 'Pressable', + type: 'Button', prop: 'accessibilityLabel', value: 'Share pull request', }); @@ -300,7 +301,7 @@ describe('PrReviewScreen share action', () => { const element = PrReviewScreen({ owner: 'octocat', repo: 'hello', number: 7 }); const shareButton = findElement({ node: element, - type: 'Pressable', + type: 'Button', prop: 'accessibilityLabel', value: 'Share pull request', }); @@ -327,7 +328,7 @@ describe('PrReviewScreen share action', () => { const element = PrReviewScreen({ owner: 'octocat', repo: 'hello', number: 7 }); const shareButton = findElement({ node: element, - type: 'Pressable', + type: 'Button', prop: 'accessibilityLabel', value: 'Share pull request', }); @@ -344,6 +345,118 @@ describe('PrReviewScreen share action', () => { }); }); +// Owner request item 3: the header carries a Merge icon button while the PR is +// mergeable, opening the same merge sheet the Overview section pushes. A +// merged/closed PR keeps Share + Submit review and gains no Merge affordance. +describe('PrReviewScreen Merge action (owner request item 3)', () => { + const MERGEABLE_OVERVIEW = { + state: 'open', + mergeable: true, + mergeableState: 'clean', + number: 7, + repo: { allowMergeCommit: true, allowSquashMerge: true, allowRebaseMerge: false }, + }; + + function findHeaderMergeButton(): React.ReactElement | null { + // eslint-disable-next-line new-cap + const element = PrReviewScreen({ owner: 'octocat', repo: 'hello', number: 7 }); + return findElement({ + node: element, + type: 'Button', + prop: 'accessibilityLabel', + value: 'Merge now', + }); + } + + beforeEach(() => { + routerPush.mockClear(); + }); + afterEach(() => { + routerPush.mockReset(); + vi.mocked(React.useContext).mockReturnValue(null); + }); + + it('renders the Merge affordance for a mergeable open pull request', () => { + prQueryResult = { + data: MERGEABLE_OVERVIEW, + isLoading: false, + isError: false, + isFetching: false, + }; + expect(findHeaderMergeButton()).not.toBeNull(); + }); + + it('opens the merge sheet with the default method on press', () => { + prQueryResult = { + data: MERGEABLE_OVERVIEW, + isLoading: false, + isError: false, + isFetching: false, + }; + const button = findHeaderMergeButton(); + if (!button) { + throw new Error('Merge button not found'); + } + const onPress = (button.props as { onPress?: () => void }).onPress; + onPress?.(); + + expect(routerPush).toHaveBeenCalledTimes(1); + expect(routerPush).toHaveBeenCalledWith({ + pathname: '/(app)/pr-review/[owner]/[repo]/[number]/merge', + params: { owner: 'octocat', repo: 'hello', number: '7', mode: 'merge', method: 'merge' }, + }); + }); + + it('does not render the Merge affordance for a merged pull request', () => { + prQueryResult = { + data: { state: 'merged', mergeable: null, mergeableState: null }, + isLoading: false, + isError: false, + isFetching: false, + }; + expect(findHeaderMergeButton()).toBeNull(); + }); + + it('offers Merge for an open provider merge request and pushes its own sheet route', () => { + // A GitLab/Bitbucket arm normalizes `mergeable` to null, so the gate keys + // off the request state and the sheet reads the provider restrictions. + vi.mocked(React.useContext).mockReturnValue({ + ref: { platform: 'gitlab', projectPath: 'group/sub/repo', mrIid: 12 }, + organizationId: null, + }); + prQueryResult = { + data: { + state: 'open', + mergeable: null, + mergeableState: null, + number: 12, + repo: { allowMergeCommit: true, allowSquashMerge: true, allowRebaseMerge: false }, + }, + isLoading: false, + isError: false, + isFetching: false, + }; + // eslint-disable-next-line new-cap + const element = PrReviewScreen({ owner: 'group/sub', repo: 'repo', number: 12 }); + const button = findElement({ + node: element, + type: 'Button', + prop: 'accessibilityLabel', + value: 'Merge now', + }); + expect(button).not.toBeNull(); + if (!button) { + throw new Error('Merge button not found for the provider arm'); + } + const onPress = (button.props as { onPress?: () => void }).onPress; + onPress?.(); + + expect(routerPush).toHaveBeenCalledWith( + '/(app)/pr-review/gitlab/group/sub/repo/12/merge?mode=merge&method=merge' + ); + }); +}); + describe('PrReviewScreen Overview scrolling', () => { beforeEach(() => { prQueryResult = { diff --git a/apps/mobile/src/components/pr-review/pr-review-screen.tsx b/apps/mobile/src/components/pr-review/pr-review-screen.tsx index 909bf75376..7b49e3daee 100644 --- a/apps/mobile/src/components/pr-review/pr-review-screen.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-screen.tsx @@ -1,9 +1,9 @@ import { useQuery, useQueryClient } from '@tanstack/react-query'; import { type Href, useFocusEffect, useRouter } from 'expo-router'; -import { Check, GitPullRequest, Share as ShareIcon } from '@/components/ui/icons'; +import { Check, GitMerge, GitPullRequest, Share as ShareIcon } from '@/components/ui/icons'; import { type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { Pressable, Share, View } from 'react-native'; +import { Share, View } from 'react-native'; import { RefreshControl } from '@/components/ui/refresh-control'; import { PrMergePartialSuccessBanner } from '@/components/pr-review/merge/pr-merge-partial-success-banner'; @@ -18,15 +18,18 @@ import { import { EmptyState } from '@/components/empty-state'; import { ScreenHeader } from '@/components/screen-header'; import { Button } from '@/components/ui/button'; -import { Text } from '@/components/ui/text'; +import { + defaultMergeMethodFor, + getMergeabilityStatus, +} from '@/lib/pr-review/merge/merge-blocked-reasons'; import { consumeMergePartialSuccess } from '@/lib/pr-review/merge/merge-result-banner-store'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { useProviderPrQueries } from '@/lib/pr-review/provider-pr-queries'; import { providerPrTriple, providerPrWebUrl } from '@/lib/pr-review/provider-pr-ref'; import { markRecentPrFailed, upsertRecentPr } from '@/lib/pr-review/recent-prs'; -import { cn } from '@/lib/utils'; const REVIEW_SUBMIT_PATH = '/(app)/pr-review/[owner]/[repo]/[number]/review-submit' as const; +const MERGE_PATH = '/(app)/pr-review/[owner]/[repo]/[number]/merge' as const; type PrReviewScreenProps = { readonly owner: string; @@ -99,6 +102,26 @@ export function PrReviewScreen({ owner, repo, number }: PrReviewScreenProps) { const overviewOptions = queries.overviewOptions(); const pr = useQuery(overviewOptions); + // Owner request item 3: the header Merge CTA opens the same confirmation + // sheet the Overview merge section pushes. GitHub carries the repo's default + // merge method on its own route; a provider arm pushes the sheet under the + // ref's own route (s6), which reads `getMergeState` for the restrictions. + const openMerge = useCallback(() => { + const data = pr.data; + if (!data) { + return; + } + const method = defaultMergeMethodFor(data.repo); + const href: Href = + queries.ref.platform === 'github' + ? { + pathname: MERGE_PATH, + params: { owner, repo, number: String(data.number), mode: 'merge', method }, + } + : providerPrSheetHref(queries.ref, 'merge', { mode: 'merge', method }); + router.push(href); + }, [router, owner, repo, queries.ref, pr.data]); + // Recents backfill. This is the ONLY writer that creates an entry: a // successful load upserts the real title with `lastResult: 'ok'`, which // also clears any previous `'failed'` marker. A never-authorized PR @@ -207,6 +230,19 @@ export function PrReviewScreen({ owner, repo, number }: PrReviewScreenProps) { // a Bitbucket PR without a selected organization waits at the boundary // instead of opening a sheet that cannot load. const canSubmitReview = queries.isReady; + // The header Merge affordance mirrors the Overview merge section's gate so + // the two never disagree: GitHub reads the mergeability off the overview DTO + // (a merged/closed PR is terminal, a blocked one keeps the section's + // blocked-reasons panel), while a GitLab/Bitbucket arm normalizes `mergeable` + // to null and offers the action for any open request — the sheet itself + // reads the provider restrictions and refuses an unsafe merge. + const canMerge = + queries.isReady && + !loadFailed && + pr.data !== undefined && + (queries.ref.platform === 'github' + ? getMergeabilityStatus(pr.data) === 'mergeable' + : pr.data.state === 'open'); let body: ReactNode = null; if (!queries.isReady) { @@ -271,20 +307,19 @@ export function PrReviewScreen({ owner, repo, number }: PrReviewScreenProps) { headerRight={ {webUrl ? ( - - + ) : null} {/* P1-F-46b: the Submit-review affordance is reachable from the Overview tab (header right) and the Files tab (floating @@ -292,14 +327,26 @@ export function PrReviewScreen({ owner, repo, number }: PrReviewScreenProps) { a submit affordance — comment threads there are read-only. */} {tab === 'overview' && canSubmitReview ? ( + ) : null} + {/* Owner request item 3: a Merge CTA at the top right, present only + while the PR is actually mergeable. The Overview merge section + stays the source of truth for why a blocked PR cannot merge. */} + {canMerge ? ( + ) : null}