From d07a07f33c0a1b317dea6234636976231c3688b5 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 11 Oct 2026 14:19:53 -0700 Subject: [PATCH] feat(mobile): keep pull requests available offline (#18351) --- .../connection/pull-request-cache-store.ts | 22 +++ .../pull-requests/PullRequestControls.tsx | 9 +- .../pull-requests/PullRequestDescription.tsx | 107 ++++++------- .../PullRequestDiscussionScreen.tsx | 1 + .../pull-requests/PullRequestFiles.tsx | 23 +-- .../pull-requests/PullRequestMarkdown.tsx | 105 +++++++++++-- .../pull-requests/PullRequestMedia.tsx | 142 +++++++++++++++++ .../PullRequestMergeControls.tsx | 12 +- .../PullRequestOverviewLoading.tsx | 3 +- .../PullRequestReviewersScreen.tsx | 3 + .../pull-requests/PullRequestRouteScreen.tsx | 13 +- .../PullRequestSectionHeading.tsx | 6 + .../pull-requests/PullRequestStatusRows.tsx | 3 + .../pull-requests/PullRequestsScreen.tsx | 32 ++-- .../pull-requests/guide/GuideNotice.tsx | 85 ++++++++++ .../guide/PullRequestGuideScreen.tsx | 125 ++++----------- .../guide/reviewGuideState.test.ts | 57 ++++++- .../pull-requests/guide/reviewGuideState.ts | 61 ++++++++ .../pull-requests/guide/useReviewGuide.ts | 7 +- .../pullRequestDescription.test.ts | 4 +- .../pullRequestDescriptionContent.test.ts | 38 +++++ .../pullRequestDescriptionContent.ts | 27 +++- .../pullRequestMediaSource.test.ts | 37 +++++ .../pull-requests/pullRequestMediaSource.ts | 26 ++++ .../usePullRequestAgentActions.test.ts | 55 +++++++ .../usePullRequestAgentActions.ts | 4 +- .../pull-requests/usePullRequestConnected.ts | 8 + .../usePullRequestFilesViewed.ts | 5 +- .../usePullRequestPermissions.ts | 22 ++- .../pull-requests/usePullRequestThreads.ts | 8 +- apps/mobile/src/lib/videoPreviewSource.ts | 26 ++-- apps/mobile/src/persistence/layer.ts | 2 + .../src/persistence/mobile-database.test.ts | 85 ++++++++++ .../mobile/src/persistence/mobile-database.ts | 66 ++++++-- .../pullRequest/PullRequestMarkdown.tsx | 4 +- .../pullRequest/pullRequestMarkdown.logic.ts | 147 +----------------- docs/user/source-control.md | 4 + packages/client-runtime/package.json | 8 + .../src/pullRequestMarkdown.test.ts | 22 ++- .../client-runtime/src/pullRequestMarkdown.ts | 146 +++++++++++++++++ .../src/state/pullRequestStatsPersistence.ts | 43 +++++ .../src/state/pullRequests.test.ts | 46 ++++++ .../client-runtime/src/state/pullRequests.ts | 29 +++- .../src/state/queryPersistence.test.ts | 71 +++++++++ .../src/state/queryPersistence.ts | 49 ++++++ .../client-runtime/src/state/runtime.test.ts | 98 +++++++++++- packages/client-runtime/src/state/runtime.ts | 89 +++++++---- 47 files changed, 1566 insertions(+), 419 deletions(-) create mode 100644 apps/mobile/src/connection/pull-request-cache-store.ts create mode 100644 apps/mobile/src/features/pull-requests/PullRequestMedia.tsx create mode 100644 apps/mobile/src/features/pull-requests/PullRequestSectionHeading.tsx create mode 100644 apps/mobile/src/features/pull-requests/guide/GuideNotice.tsx create mode 100644 apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.test.ts create mode 100644 apps/mobile/src/features/pull-requests/pullRequestMediaSource.test.ts create mode 100644 apps/mobile/src/features/pull-requests/pullRequestMediaSource.ts create mode 100644 apps/mobile/src/features/pull-requests/usePullRequestAgentActions.test.ts create mode 100644 apps/mobile/src/features/pull-requests/usePullRequestConnected.ts rename apps/web/src/components/pullRequest/pullRequestMarkdown.logic.test.ts => packages/client-runtime/src/pullRequestMarkdown.test.ts (86%) create mode 100644 packages/client-runtime/src/pullRequestMarkdown.ts create mode 100644 packages/client-runtime/src/state/pullRequestStatsPersistence.ts create mode 100644 packages/client-runtime/src/state/queryPersistence.test.ts create mode 100644 packages/client-runtime/src/state/queryPersistence.ts diff --git a/apps/mobile/src/connection/pull-request-cache-store.ts b/apps/mobile/src/connection/pull-request-cache-store.ts new file mode 100644 index 0000000000000000000000000000000000000000..3eab93296e1403d0b9f27e9587f92313b2228950 --- /dev/null +++ b/apps/mobile/src/connection/pull-request-cache-store.ts @@ -0,0 +1,22 @@ +import { EnvironmentQueryStore } from "@t3tools/client-runtime/state/query-persistence"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; + +import { MobileDatabase } from "../persistence/mobile-database"; + +export const layer = Layer.effect( + EnvironmentQueryStore, + Effect.gen(function* () { + const database = yield* MobileDatabase; + return { + load: (environmentId, key) => + database + .loadCache(environmentId, "pull-request", `v1:${key}`) + .pipe(Effect.catch(() => Effect.succeedNone)), + save: (environmentId, key, payload) => + database + .saveCache(environmentId, "pull-request", `v1:${key}`, 1, payload) + .pipe(Effect.catch(() => Effect.logWarning("Offline query database save failed"))), + } satisfies NonNullable; + }), +); diff --git a/apps/mobile/src/features/pull-requests/PullRequestControls.tsx b/apps/mobile/src/features/pull-requests/PullRequestControls.tsx index 0a7fe6b34f08fbc952600ea7b0a107592abb422b..156b36b2155311c3561f5aecfa8e3f550a4d5345 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestControls.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestControls.tsx @@ -123,7 +123,8 @@ export function PullRequestActionToolbar(props: { const agent = usePullRequestAgentActions(props.params, props.pr, props.filePath); const canReview = permissions.verdicts.length > 0; const pendingReview = canReview && draft.comments.length > 0; - const canOpenThread = agent.threads.length > 0 || agent.canAsk; + const showAgentControls = agent.canAsk || !permissions.connected; + const canOpenThread = agent.threads.length > 0 || showAgentControls; const threadLabel = agent.threads.length ? "Continue thread" : "Work on PR"; const primaryLabel = pendingReview ? `Submit review · ${draft.comments.length}` : threadLabel; const chooseThread = agent.threads.length > 1; @@ -142,7 +143,7 @@ export function PullRequestActionToolbar(props: { // Reserve the native outer margins, custom-button padding, and item gaps. // UIKit needs this space to keep every action out of its overflow menu. // Duo's side rail uses an intrinsic icon item instead. - const sideControls = Number(agent.canAsk) + 1; + const sideControls = Number(showAgentControls) + 1; const primaryWidth = verticalControls ? undefined : Math.max( @@ -166,7 +167,7 @@ export function PullRequestActionToolbar(props: { className="flex-row items-center justify-between gap-3 border-t border-border-subtle bg-screen px-4 pt-2" style={{ paddingBottom: Math.max(insets.bottom, 8) }} > - {agent.canAsk ? ( + {showAgentControls ? ( agent.menuAction(action.task))} @@ -259,7 +260,7 @@ export function PullRequestActionToolbar(props: { /> ) ) : null; - const agentControl = agent.canAsk ? ( + const agentControl = showAgentControls ? ( (null); - const openingUri = preview && "uri" in preview ? preview.uri : null; + const mediaContext = useContext(PullRequestMediaContext); const { themeVariables: colors } = useAppearancePreferences(); const content = useMemo(() => { const document = nativeMarkdownWithHtml( parseMarkdownWithOptions(props.markdown, { gfm: true, html: true, math: false }), (markdown) => parseMarkdownWithOptions(markdown, { gfm: true, html: false, math: false }), ); - return pullRequestDescriptionContent(document); + return pullRequestDescriptionContent(document, splitPullRequestBody(props.markdown)); }, [props.markdown]); - const gallery = useMemo( + const gallery = useMemo( () => - content.attachments.map((attachment) => ({ - kind: "image" as const, - uri: attachment.uri, - name: new URL(attachment.uri).pathname.split("/").pop() || "Image", - })), - [content], + content.attachments + .filter((attachment) => attachment.kind === "image") + .map((attachment) => { + return { + ...pullRequestImageSource(attachment.uri, mediaContext), + name: new URL(attachment.uri).pathname.split("/").pop() || "Image", + }; + }), + [content, mediaContext], ); const canCollapse = bodyHeight > COLLAPSED_HEIGHT; const collapsed = canCollapse && !expanded; return ( - Description + Description {props.markdown.trim() ? ( {content.attachments.length ? ( - Attachments + Attachments - {content.attachments.map((attachment) => ( - - setPreview({ - kind: "image", - uri: attachment.uri, - name: gallery.find((item) => item.uri === attachment.uri)?.name ?? "Image", - gallery, - }) - } - className="gap-2 active:opacity-70" - style={{ width: 176 }} - > - - - {openingUri === attachment.uri ? ( - - - - ) : null} - - - {attachment.caption} - - - ))} + {content.attachments.map((attachment) => + attachment.kind !== "image" ? ( + + ) : ( + item.kind === "image") + .indexOf(attachment) + ]! + } + gallery={gallery} + /> + ), + )} ) : null} - setPreview(null)} /> ); } diff --git a/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx index 81f389b8598bf47985f387d413f2a24761889394..795f72bcd48bcb66f4ef2d7be4813fc1d4259369 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx @@ -118,6 +118,7 @@ function Discussion({ params }: { params: PullRequestParams & { readonly discuss const resolveLabel = thread?.isResolved ? "Unresolve" : "Resolve"; return ( diff --git a/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx b/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx index ea0ab44b68b868bd6b5186301ce380bc53a5aa7d..d51671955a272b6b86111a69956ae4d12baa1ba6 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestFiles.tsx @@ -1,3 +1,4 @@ +import { PullRequestSectionHeading } from "./PullRequestSectionHeading"; import { AnimatedLegendList } from "@legendapp/list/reanimated"; import Animated from "react-native-reanimated"; import { PR_ROW_LAYOUT, PR_ROW_ENTER, PR_ROW_EXIT } from "./pullRequestAnimations"; @@ -167,15 +168,15 @@ export function usePullRequestFilesContent(props: { accessibilityState={{ busy: true }} className="min-h-17 flex-row items-center gap-2 py-3" > - - - {fileCount === undefined - ? "Changed files" - : `${fileCount} changed ${fileCount === 1 ? "file" : "files"}`} - - {filter.hideTestFiles ? ( + + Changed files + {fileCount !== undefined ? ( + + {fileCount} {fileCount === 1 ? "file" : "files"} + + ) : ( - ) : null} + )} {summary?.additions !== undefined && summary.deletions !== undefined ? ( - + Changed files + {parsed.fileCount} {filter.hiddenCount ? "visible" : "changed"}{" "} {parsed.fileCount === 1 ? "file" : "files"} {viewed.supported ? ` · ${parsed.files.filter((file) => viewed.isViewed(file.path)).length} viewed` : ""} - - + {" · "} {filter.hideTestFiles ? `${filter.hiddenCount} test ${filter.hiddenCount === 1 ? "file" : "files"} hidden` : "Test files included"} diff --git a/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx b/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx index bf5654eed4c43ee66548d6e6623dab764039fc07..b18bdec364e438371ef9fb28a95edff1d81cea0f 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx @@ -1,3 +1,13 @@ +import { splitPullRequestBody } from "@t3tools/client-runtime/pull-request-markdown"; +import type { ReactNode } from "react"; +import { View } from "react-native"; +import { useProject } from "../../state/entities"; +import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff"; +import { + PullRequestMedia, + PullRequestMediaContext, + usePullRequestMediaUrl, +} from "./PullRequestMedia"; import { videoMimeType } from "@t3tools/shared/video"; import { createContext, useContext, useCallback, useMemo, useState } from "react"; import { githubMarkdownLinks } from "@t3tools/shared/githubMarkdownLinks"; @@ -16,7 +26,8 @@ import { } from "../../native/SelectableMarkdownText"; import { useMarkdownPreviewStyles } from "../files/FileMarkdownPreview"; import { markdownImageSources } from "../../lib/markdownImageSources"; -import { FilePreviewGallery } from "../../components/FilePreviewGallery"; +import { FilePreviewGallery, useFilePreviewGallery } from "../../components/FilePreviewGallery"; +import { pullRequestImageSource } from "./pullRequestMediaSource"; import { FilePreviewModal, type FilePreviewSource, @@ -25,7 +36,78 @@ import { import { ThreadMarkdownImageView } from "../threads/ThreadMarkdownImage"; import type { MarkdownImageRenderer } from "../../native/SelectableMarkdownText"; -export const PullRequestMarkdownRepository = createContext(null); +const RepositoryContext = createContext(null); + +export function PullRequestMarkdownRepository(props: { + readonly value: string | null; + readonly params: PullRequestParams; + readonly children: ReactNode; +}) { + const target = pullRequestTarget(props.params); + const project = useProject({ + environmentId: target.environmentId, + projectId: target.input.projectId, + }); + const cwd = project?.workspaceRoot; + const media = useMemo( + () => (cwd ? { environmentId: target.environmentId, cwd } : null), + [target.environmentId, cwd], + ); + return ( + + {props.children} + + ); +} + +function MarkdownImage(props: { + readonly href: string; + readonly alt: string | null; + readonly onPreview: (source: FilePreviewSource) => void; +}) { + const media = usePullRequestMediaUrl(props.href); + const context = useContext(PullRequestMediaContext); + const withGallery = useFilePreviewGallery(); + return ( + + props.onPreview( + withGallery({ + ...pullRequestImageSource(props.href, context), + name: source.name, + sourceIdentifier: source.sourceIdentifier, + }), + ) + } + /> + ); +} + +export function PullRequestMarkdown(props: { + readonly markdown: string; + readonly imageGroupMarkdown?: string; +}) { + const segments = useMemo(() => splitPullRequestBody(props.markdown), [props.markdown]); + return ( + + {segments.map((segment) => + segment.kind === "markdown" ? ( + + ) : ( + + ), + )} + + ); +} function renderLink( { href, children }: CustomRendererProps, @@ -61,11 +143,12 @@ function renderHtmlBlock( ) : null; } -export function PullRequestMarkdown(props: { +function PullRequestMarkdownContent(props: { readonly markdown: string; readonly imageGroupMarkdown?: string; }) { - const repositoryUrl = useContext(PullRequestMarkdownRepository); + const repositoryUrl = useContext(RepositoryContext); + const mediaContext = useContext(PullRequestMediaContext); const markdown = useMemo( () => githubMarkdownLinks(props.markdown, repositoryUrl), [props.markdown, repositoryUrl], @@ -85,20 +168,12 @@ export function PullRequestMarkdown(props: { /^https?:\/\//i.test(uri) && videoMimeType({ name: uri.split(/[?#]/)[0] ?? uri, mimeType: "" }) === null, ) - .map((uri) => ({ kind: "image", uri })), - [props.markdown, props.imageGroupMarkdown], + .map((uri) => pullRequestImageSource(uri, mediaContext)), + [props.markdown, props.imageGroupMarkdown, mediaContext], ); const [preview, setPreview] = useState(null); const renderImage = useCallback( - (image) => ( - - ), + (image) => , [], ); const styles = useMarkdownPreviewStyles(renderImage); diff --git a/apps/mobile/src/features/pull-requests/PullRequestMedia.tsx b/apps/mobile/src/features/pull-requests/PullRequestMedia.tsx new file mode 100644 index 0000000000000000000000000000000000000000..3ff9a2a82ec7c8cd8086d3d21cc38d3f26662209 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/PullRequestMedia.tsx @@ -0,0 +1,142 @@ +import { createContext, useContext, useMemo, useState } from "react"; +import { pullRequestMediaSource, pullRequestImageSource } from "./pullRequestMediaSource"; +import { useFilePreviewGallery } from "../../components/FilePreviewGallery"; +import type { AssetResource, EnvironmentId } from "@t3tools/contracts"; +import { githubMediaFetchUrl } from "@t3tools/shared/githubMedia"; +import { videoMimeType } from "@t3tools/shared/video"; +import { Pressable, View } from "react-native"; +import { Image } from "expo-image"; +import { AppText } from "../../components/AppText"; +import { useAssetUrlState, useRefreshAssetUrl } from "../../state/assets"; +import { MediaVideoPlayer } from "../../components/MediaVideoPlayer"; +import { VideoAttachmentTile } from "../../components/VideoAttachmentTile"; +import { VideoPreviewModal, type VideoPreviewSource } from "../../components/VideoPreviewModal"; +import { ThreadMarkdownImageView } from "../threads/ThreadMarkdownImage"; +import { + FilePreviewModal, + type FilePreviewSource, + type FilePreviewItem, +} from "../../components/FilePreviewModal"; + +export const PullRequestMediaContext = createContext<{ + readonly environmentId: EnvironmentId; + readonly cwd: string; +} | null>(null); + +/** Only GitHub-hosted media needs the environment's authenticated asset path. */ +export function usePullRequestMediaUrl(url: string) { + const context = useContext(PullRequestMediaContext); + const fetchUrl = githubMediaFetchUrl(url); + const resource = useMemo( + () => (context && fetchUrl ? { _tag: "github-media", cwd: context.cwd, url: fetchUrl } : null), + [context, fetchUrl], + ); + const asset = useAssetUrlState(context?.environmentId ?? null, resource); + const refresh = useRefreshAssetUrl(context?.environmentId ?? null, resource); + return { + uri: asset._tag === "Success" ? asset.url : resource ? null : url, + unavailable: resource !== null && asset._tag === "Failure", + source: pullRequestMediaSource(url, context), + resolvePlaybackUri: resource ? async () => (await refresh()) ?? null : undefined, + }; +} + +export function PullRequestMedia(props: { + readonly url: string; + readonly kind: "image" | "video" | "unknown"; + readonly caption?: string; + readonly compact?: boolean; +}) { + const media = usePullRequestMediaUrl(props.url); + const context = useContext(PullRequestMediaContext); + const withGallery = useFilePreviewGallery(); + const [image, setImage] = useState(null); + const [video, setVideo] = useState(null); + const name = props.caption || new URL(props.url).pathname.split("/").pop() || "Attachment"; + if (props.kind === "image") + return ( + <> + + setImage( + withGallery({ + ...pullRequestImageSource(props.url, context), + name, + sourceIdentifier: source.sourceIdentifier, + }), + ) + } + /> + setImage(null)} /> + + ); + return ( + + {props.compact ? ( + + setVideo({ + type: "media", + ...media.source, + name, + mimeType: + videoMimeType({ name: props.url.split(/[?#]/)[0] ?? props.url, mimeType: "" }) ?? + "video/mp4", + }) + } + /> + ) : ( + + )} + setVideo(null)} /> + + ); +} + +export function PullRequestImageAttachment(props: { + readonly url: string; + readonly caption: string; + readonly source: FilePreviewItem; + readonly gallery: readonly FilePreviewItem[]; +}) { + const media = usePullRequestMediaUrl(props.url); + const [preview, setPreview] = useState(null); + return ( + <> + setPreview({ ...props.source, gallery: props.gallery })} + className="gap-2 active:opacity-70" + style={{ width: 176 }} + > + + + + + {props.caption} + + + setPreview(null)} /> + + ); +} diff --git a/apps/mobile/src/features/pull-requests/PullRequestMergeControls.tsx b/apps/mobile/src/features/pull-requests/PullRequestMergeControls.tsx index 865541fbca02df99a84264b0ddecd5ddff4c4665..5ead045fa2b04200a6577d27a461daf599e678b2 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestMergeControls.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestMergeControls.tsx @@ -54,9 +54,12 @@ export function PullRequestMergeControls(props: { hasStack: stack.stack !== null, isFresh: stack.isFresh, }); - const checkingStack = stack.supportsStackActions && !stack.isFresh && !stack.error; + const checkingStack = + permissions.connected && stack.supportsStackActions && !stack.isFresh && !stack.error; const reason = - mergeUnavailableReason(pr, permissions.canWrite) ?? + (!permissions.connected + ? "Reconnect to merge." + : mergeUnavailableReason(pr, permissions.canWrite)) ?? (stack.stack ? !stack.supportsStackActions ? "This environment can display stacks but cannot merge them." @@ -185,7 +188,7 @@ export function PullRequestMergeControls(props: { - {busy ? "Working…" : label} + {busy ? "Working…" : !permissions.connected ? "Reconnect to merge" : label} {methods.length > 0 || canAutoMerge ? ( @@ -230,7 +233,8 @@ export function PullRequestMergeControls(props: { ) : null} - {reason && + {permissions.connected && + reason && reason !== MERGE_CONFLICT_REASON && reason !== "Checking stack membership…" && !(checkingStack && reason === "Refresh the stack before merging.") ? ( diff --git a/apps/mobile/src/features/pull-requests/PullRequestOverviewLoading.tsx b/apps/mobile/src/features/pull-requests/PullRequestOverviewLoading.tsx index 8b5b81ce40495c0b272ef0fa227526a309d6a2b0..7e6e4f490ae9480793a525ddaacfd92a0f09f8a2 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestOverviewLoading.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestOverviewLoading.tsx @@ -1,3 +1,4 @@ +import { PullRequestSectionHeading } from "./PullRequestSectionHeading"; import type { PullRequestListEntry } from "@t3tools/contracts"; import { View } from "react-native"; import { AppText as Text } from "../../components/AppText"; @@ -97,7 +98,7 @@ export function PullRequestOverviewLoading({ ) : null} - Description + Description diff --git a/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx index 924bbaa0b341d2841a19d0bf02e9f7c799f78c0b..597cf63b9107bea9d4ca916dc02b8bedab5f177b 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx @@ -1,3 +1,4 @@ +import { usePullRequestConnected } from "./usePullRequestConnected"; import { useAtomValue } from "@effect/atom-react"; import { useNavigation, type StaticScreenProps } from "@react-navigation/native"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; @@ -60,7 +61,9 @@ export function PullRequestReviewersScreen({ route }: StaticScreenProps("overview"); // Keep each tab's native list and measured rows alive when it is hidden. const requestedSection = useRef(section); @@ -350,18 +352,23 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps + {Platform.OS === "android" ? ( navigation.goBack()} actions={[ ...(permissions.verdicts.length diff --git a/apps/mobile/src/features/pull-requests/PullRequestSectionHeading.tsx b/apps/mobile/src/features/pull-requests/PullRequestSectionHeading.tsx new file mode 100644 index 0000000000000000000000000000000000000000..8fd85199a5581f0239db162e9352225df76ded82 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/PullRequestSectionHeading.tsx @@ -0,0 +1,6 @@ +import type { ReactNode } from "react"; +import { AppText } from "../../components/AppText"; + +export function PullRequestSectionHeading({ children }: { readonly children: ReactNode }) { + return {children}; +} diff --git a/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx b/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx index c331b6b7d15f566e011a344b11936767cead6ec0..c39cea3187ab72ebdb859b84c35976e07c8cab6a 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx @@ -1,3 +1,4 @@ +import { usePullRequestConnected } from "./usePullRequestConnected"; import { useAtomValue } from "@effect/atom-react"; import { useNavigation } from "@react-navigation/native"; import type { PullRequestAction, PullRequestDetail } from "@t3tools/contracts"; @@ -237,7 +238,9 @@ export function PullRequestReviewersRow(props: RowProps) { pullRequestTarget(props.params).environmentId, ), ); + const connected = usePullRequestConnected(props.params.environmentId); const canRequest = + connected && canWrite && props.pr.state === "open" && props.pr.capabilities.reviewers.request && diff --git a/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx index 53ed9bbef2fa459a75a2ab8032d5d7404bed84f1..f32a36357262dde717af664956ad98394af5df8d 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx @@ -99,7 +99,6 @@ export function PullRequestsScreen() { environments .filter( (environment) => - environment.connectionState === "connected" && configs.get(environment.environmentId)?.environment.capabilities.pullRequests === true && (environmentId === null || environment.environmentId === environmentId) && @@ -121,8 +120,15 @@ export function PullRequestsScreen() { [environments, environmentId, configs, controls, query], ); const result = usePullRequestList(targets); + const connectedTargets = targets.filter((target) => + environments.some( + (environment) => + environment.environmentId === target.environmentId && + environment.connectionState === "connected", + ), + ); const { actions, select, busyKey } = usePullRequestListActions( - targets.map((target) => target.environmentId), + connectedTargets.map((target) => target.environmentId), result.updateEntry, ); const selectedKey = primaryColumn?.selectedPullRequestKey; @@ -309,7 +315,7 @@ export function PullRequestsScreen() { ) : undefined } ListEmptyComponent={ - result.isPending ? ( + result.isPending && connectedTargets.length > 0 ? ( @@ -319,17 +325,21 @@ export function PullRequestsScreen() { title={ result.error ? "Couldn't load pull requests" - : targets.length - ? query - ? "No results" - : "No matching pull requests" - : "Connect an environment" + : targets.length && connectedTargets.length === 0 + ? "Environment offline" + : targets.length + ? query + ? "No results" + : "No matching pull requests" + : "Connect an environment" } detail={ result.error ?? - (targets.length - ? "Pull requests from the selected environments' projects appear here." - : "Add an environment in Settings to browse its pull requests.") + (targets.length && connectedTargets.length === 0 + ? "Reconnect to load pull requests that aren't saved on this device." + : targets.length + ? "Pull requests from the selected environments' projects appear here." + : "Add an environment in Settings to browse its pull requests.") } variant="plain" /> diff --git a/apps/mobile/src/features/pull-requests/guide/GuideNotice.tsx b/apps/mobile/src/features/pull-requests/guide/GuideNotice.tsx new file mode 100644 index 0000000000000000000000000000000000000000..9701a496904b6849b664f0c21d3dc2c7deb578f2 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/guide/GuideNotice.tsx @@ -0,0 +1,85 @@ +import { Pressable, View } from "react-native"; +import Animated, { FadeIn, ReduceMotion } from "react-native-reanimated"; +import { AppText as Text } from "../../../components/AppText"; +import { SymbolView } from "../../../components/AppSymbol"; +import { PR_ROW_EXIT, PR_ROW_LAYOUT } from "../pullRequestAnimations"; +import { guideNoticeState } from "./reviewGuideState"; +import type { useReviewGuide } from "./useReviewGuide"; + +const NOTICE_ENTER = FadeIn.delay(180).duration(120).reduceMotion(ReduceMotion.System); + +/** Keep the action in one place as loading, writing, and retry states change. */ +export function GuideNotice({ state }: { readonly state: ReturnType }) { + const notice = guideNoticeState(state); + if (notice.kind === "ready") return null; + const compact = state.data?.guide != null; + const failed = notice.kind === "failed" || notice.kind === "unavailable"; + const busy = notice.kind === "loading" || notice.kind === "writing"; + return ( + + + + + {compact ? notice.title : "Review guide"} + + + + {notice.description} + + + {state.canWrite || busy || notice.kind === "unavailable" ? ( + (notice.kind === "unavailable" ? state.refresh() : void state.write())} + > + + {notice.action} + + + ) : ( + + {notice.kind === "failed" + ? "Reconnect or ask someone with write access to retry." + : "Writing a guide requires a connection and write access."} + + )} + {notice.kind === "writing" && state.data?.run?._tag === "Running" ? ( + + Started{" "} + {new Date(state.data.run.startedAt).toLocaleTimeString([], { + hour: "2-digit", + minute: "2-digit", + })} + + ) : null} + + + ); +} diff --git a/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx b/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx index 1bf9fc02c2d815fb86a15f195893e2e2e6910bfb..cb280ea4b782e24e600ed9e5d14101ca4cebce51 100644 --- a/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx +++ b/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx @@ -13,7 +13,6 @@ import { import { SymbolView } from "../../../components/AppSymbol"; import { GuideOutline } from "./GuideOutline"; import { AppText as Text } from "../../../components/AppText"; -import { MaterialButton } from "../../../components/MaterialButton"; import { ScreenScrollView } from "../../../components/ScreenScrollView"; import { ControlPillMenu } from "../../../components/ControlPillMenu"; import { MaterialIconButton } from "../../../components/MaterialIconButton"; @@ -24,6 +23,9 @@ import { PullRequestMarkdown, PullRequestMarkdownRepository } from "../PullReque import { usePullRequestDiff, type PullRequestParams } from "../usePullRequestDiff"; import { ReviewGuideExcerpt } from "./ReviewGuideExcerpt"; import { reviewGuideState } from "./reviewGuideState"; +import { GuideNotice } from "./GuideNotice"; +import Animated from "react-native-reanimated"; +import { PR_ROW_ENTER, PR_ROW_EXIT, PR_ROW_LAYOUT } from "../pullRequestAnimations"; import { useEnvironmentQuery } from "../../../state/query"; import { composerPullRequests } from "../../../state/pull-requests"; import { usePullRequestPermissions } from "../usePullRequestPermissions"; @@ -44,111 +46,47 @@ const attentionLabels = { accessibility: "An accessibility call", }; -function GuideNotice({ state }: { readonly state: ReturnType }) { - const { guide, stale, failure } = reviewGuideState(state.data); - const error = state.startError ?? failure ?? state.error; - const label = state.running - ? guide - ? "Rewriting review guide" - : "Writing review guide" - : error - ? "Could not write guide" - : stale - ? "Guide is out of date" - : "No review guide yet"; - if (guide && !stale && !error && !state.running) return null; - return ( - - - - - {label} - - - - {error ?? - (state.running - ? "An agent is reading the change. You can leave this screen and return when it’s ready." - : stale - ? "There are new changes since this guide was written. Rewrite it to see the latest code." - : "Walk through the important code in order, with notes on what changed and where to look closely.")} - - {state.running ? ( - - {state.data?.run?._tag === "Running" - ? `Started at ${new Date(state.data.run.startedAt).toLocaleTimeString([], { hour: "2-digit", minute: "2-digit" })}` - : "Starting the guide…"} - - ) : state.canWrite ? ( - void state.write()} - /> - ) : null} - - ); -} - export function PullRequestGuideContent({ params }: { readonly params: PullRequestParams }) { const navigation = useNavigation(); const state = useReviewGuide(params); const { guide, stale } = reviewGuideState(state.data); return ( - + {!state.enabled ? ( Review guides are disabled for this server. ) : ( <> - {state.isPending && !state.data ? ( - Loading review guide… - ) : ( - - )} + {guide ? ( - - navigation.dispatch( - StackActions.push("PullRequestGuideStop", { - ...params, - position, - guideHeadSha: guide.headSha, - guideGeneratedAt: guide.generatedAt, - }), - ) - } - onRewrite={ - state.canWrite && !state.running && !stale && !state.startError && !state.error - ? () => void state.write() - : undefined - } - /> + + + navigation.dispatch( + StackActions.push("PullRequestGuideStop", { + ...params, + position, + guideHeadSha: guide.headSha, + guideGeneratedAt: guide.generatedAt, + }), + ) + } + onRewrite={ + state.canWrite && !state.running && !stale && !state.startError && !state.error + ? () => void state.write() + : undefined + } + /> + ) : null} )} - + ); } @@ -240,6 +178,7 @@ export function PullRequestGuideStopScreen({ route }: StaticScreenProps = { headSha: "old-head", @@ -46,3 +46,58 @@ describe("mobile review guide state", () => { }); }); }); + +const view = { + data: null, + isPending: false, + running: false, + startError: null, + error: null, +}; + +describe("guide notice transitions", () => { + it("does not flash an empty guide before the first response", () => { + expect(guideNoticeState({ ...view, isPending: true }).kind).toBe("loading"); + expect( + guideNoticeState({ ...view, data: { guide: null, run: null, currentHeadSha: "head" } }).kind, + ).toBe("empty"); + }); + + it("keeps cached guides visible while refreshing", () => { + expect( + guideNoticeState({ + ...view, + isPending: true, + data: { guide, run: null, currentHeadSha: guide.headSha }, + }).kind, + ).toBe("ready"); + }); + + it("hides the previous failure as soon as a retry starts", () => { + const data = { + guide: null, + currentHeadSha: "head", + run: { + _tag: "Failed" as const, + headSha: "head", + failedAt: guide.generatedAt, + detail: "Writer disconnected", + }, + }; + expect(guideNoticeState({ ...view, data }).kind).toBe("failed"); + expect(guideNoticeState({ ...view, data, running: true }).kind).toBe("writing"); + }); + + it("keeps a stale guide readable through rewriting and completion", () => { + const data = { guide, currentHeadSha: "head", run: null }; + expect(guideNoticeState({ ...view, data }).kind).toBe("stale"); + expect(guideNoticeState({ ...view, data, running: true }).kind).toBe("writing"); + expect( + guideNoticeState({ ...view, data: { ...data, guide: { ...guide, headSha: "head" } } }).kind, + ).toBe("ready"); + }); +}); + +it("distinguishes a failed read from a failed writer", () => { + expect(guideNoticeState({ ...view, error: "Connection lost" }).kind).toBe("unavailable"); +}); diff --git a/apps/mobile/src/features/pull-requests/guide/reviewGuideState.ts b/apps/mobile/src/features/pull-requests/guide/reviewGuideState.ts index da8bf7315e968c35ca32fc6f46d2e4247fe51363..702cc8bd4ee47b0a6c35dfe9460136128869c568 100644 --- a/apps/mobile/src/features/pull-requests/guide/reviewGuideState.ts +++ b/apps/mobile/src/features/pull-requests/guide/reviewGuideState.ts @@ -13,3 +13,64 @@ export function reviewGuideState(data: PullRequestReviewGuideResult | null) { : null, }; } + +/** A pending refresh must not hide cached content, or a new run reveal an old failure. */ +export function guideNoticeState(state: { + readonly data: PullRequestReviewGuideResult | null; + readonly isPending: boolean; + readonly running: boolean; + readonly startError: string | null; + readonly error: string | null; +}) { + const { guide, stale, failure } = reviewGuideState(state.data); + if (state.running) { + return { + kind: "writing" as const, + title: "Rewriting guide", + description: guide + ? "You can keep reading this guide while the agent writes the update." + : "An agent is reading the change and writing the tour. You can leave and come back when it’s ready.", + action: guide ? "Rewriting…" : "Writing guide…", + }; + } + const error = state.startError ?? failure; + if (error) { + return { + kind: "failed" as const, + title: "Could not write guide", + description: error, + action: "Try again", + }; + } + if (state.error) { + return { + kind: "unavailable" as const, + title: "Could not refresh guide", + description: state.error, + action: "Try again", + }; + } + if (!state.data) { + return { + kind: "loading" as const, + title: "Review guide", + description: "Checking for a saved guide…", + action: "Loading guide…", + }; + } + if (stale) { + return { + kind: "stale" as const, + title: "New changes since this guide", + description: "You can read the previous guide, or rewrite it for the latest changes.", + action: "Rewrite guide", + }; + } + return { + kind: guide ? ("ready" as const) : ("empty" as const), + title: "Review guide", + description: + "Get a tour of the change, with code excerpts and notes on what needs a closer look.", + action: "Write guide", + }; +} diff --git a/apps/mobile/src/features/pull-requests/guide/useReviewGuide.ts b/apps/mobile/src/features/pull-requests/guide/useReviewGuide.ts index f6850109c4cdf3ed1948285009a195e3579f93d4..e915804c651e943ed786619bf7874f5d70e6e2a9 100644 --- a/apps/mobile/src/features/pull-requests/guide/useReviewGuide.ts +++ b/apps/mobile/src/features/pull-requests/guide/useReviewGuide.ts @@ -2,6 +2,7 @@ import { useAtomValue } from "@effect/atom-react"; import { useFocusEffect } from "@react-navigation/native"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import { useCallback, useRef, useState } from "react"; +import { usePullRequestConnected } from "../usePullRequestConnected"; import { composerPullRequests } from "../../../state/pull-requests"; import { useEnvironmentQuery } from "../../../state/query"; import { serverEnvironment } from "../../../state/server"; @@ -19,9 +20,11 @@ export function useReviewGuide(params: PullRequestParams) { const target = pullRequestTarget(params); const enabled = useReviewGuidesEnabled(params); const query = useEnvironmentQuery(enabled ? composerPullRequests.reviewGuide(target) : null); - const canWrite = useAtomValue( + const connected = usePullRequestConnected(target.environmentId); + const permitted = useAtomValue( composerPullRequests.generateReviewGuide.permissionAtom(target.environmentId), ); + const canWrite = connected && permitted; const generate = useAtomCommand(composerPullRequests.generateReviewGuide, { reportFailure: false, }); @@ -49,8 +52,6 @@ export function useReviewGuide(params: PullRequestParams) { if (result._tag === "Failure") { const cause = squashAtomCommandFailure(result); setError(cause instanceof Error ? cause.message : "Could not start the review guide."); - } else { - refresh(); } } finally { pending.current = false; diff --git a/apps/mobile/src/features/pull-requests/pullRequestDescription.test.ts b/apps/mobile/src/features/pull-requests/pullRequestDescription.test.ts index 5247ca1d5ff1a3ac8817218ba87c0dc23b04531f..f7e0c1ad4b21ae40e49393070b28c1d67db5c2d7 100644 --- a/apps/mobile/src/features/pull-requests/pullRequestDescription.test.ts +++ b/apps/mobile/src/features/pull-requests/pullRequestDescription.test.ts @@ -15,8 +15,8 @@ describe("PR description overview", () => { ], }); expect(result.attachments).toEqual([ - { uri: "https://example.com/before.png", caption: "Before" }, - { uri: "https://example.com/after.png", caption: "After" }, + { uri: "https://example.com/before.png", caption: "Before", kind: "image" }, + { uri: "https://example.com/after.png", caption: "After", kind: "image" }, ]); expect(result.preview).toContain("Fixes reconnects."); expect(result.preview).not.toContain("Duplicate"); diff --git a/apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.test.ts b/apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..c69faff2cec295b569cb0a60c52930415624cff9 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "vite-plus/test"; +import type { MarkdownNode } from "react-native-nitro-markdown/headless"; +import { splitPullRequestBody } from "@t3tools/client-runtime/pull-request-markdown"; +import { pullRequestDescriptionContent } from "./pullRequestDescriptionContent"; + +describe("PR attachments", () => { + it("combines embedded images and detected media without duplicate URLs", () => { + const uri = "https://drop.example.com/shot.png"; + const document = { + type: "document", + children: [ + { type: "image", href: uri, alt: "Screenshot" }, + { type: "image", href: uri, alt: "Duplicate" }, + ], + } as MarkdownNode; + const media = splitPullRequestBody( + `${uri}\n\nhttps://drop.example.com/demo.mp4\n\nhttps://github.com/user-attachments/assets/123-abc`, + ); + expect(pullRequestDescriptionContent(document, media).attachments).toEqual([ + { uri, caption: "Screenshot", kind: "image" }, + { uri: "https://drop.example.com/demo.mp4", caption: "Video", kind: "video" }, + { + uri: "https://github.com/user-attachments/assets/123-abc", + caption: "Video", + kind: "video", + }, + ]); + }); + + it("does not collect ordinary links or media examples in code", () => { + const media = splitPullRequestBody( + "https://example.com/docs\n\n```\nhttps://drop.example.com/demo.mp4\n```", + ); + expect( + pullRequestDescriptionContent({ type: "document" } as MarkdownNode, media).attachments, + ).toEqual([]); + }); +}); diff --git a/apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.ts b/apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.ts index 709c58997b89fd3171629cfc22f03545b7c911cb..76e394ad2c4cd4ba88ae3951fcc3250f40f088c0 100644 --- a/apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.ts +++ b/apps/mobile/src/features/pull-requests/pullRequestDescriptionContent.ts @@ -1,13 +1,24 @@ +import type { PullRequestBodySegment } from "@t3tools/client-runtime/pull-request-markdown"; import type { MarkdownNode } from "react-native-nitro-markdown/headless"; /** Read media from the parsed document so code examples never become attachments. */ -export function pullRequestDescriptionContent(document: MarkdownNode) { - const attachments = new Map(); +export function pullRequestDescriptionContent( + document: MarkdownNode, + media: readonly PullRequestBodySegment[] = [], +) { + const attachments = new Map< + string, + { readonly uri: string; readonly caption: string; readonly kind: "image" | "video" | "unknown" } + >(); const text: string[] = []; const visit = (node: MarkdownNode) => { if (node.type === "image") { if (node.href && /^https?:\/\//i.test(node.href) && !attachments.has(node.href)) { - attachments.set(node.href, { uri: node.href, caption: node.alt || node.title || "Image" }); + attachments.set(node.href, { + uri: node.href, + caption: node.alt || node.title || "Image", + kind: "image", + }); } return; } @@ -18,5 +29,15 @@ export function pullRequestDescriptionContent(document: MarkdownNode) { if (node.type === "soft_break" || node.type === "line_break") text.push(" "); }; visit(document); + for (const segment of media) { + if (segment.kind === "attachment" && !attachments.has(segment.url)) { + const name = new URL(segment.url).pathname.split("/").pop(); + attachments.set(segment.url, { + uri: segment.url, + caption: segment.media === "video" ? "Video" : name || "Image", + kind: segment.media, + }); + } + } return { preview: text.join("").trim(), attachments: [...attachments.values()] }; } diff --git a/apps/mobile/src/features/pull-requests/pullRequestMediaSource.test.ts b/apps/mobile/src/features/pull-requests/pullRequestMediaSource.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..b5e90f7deddbbd1e5ade53a4f064a216baa2df00 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/pullRequestMediaSource.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vite-plus/test"; +import { EnvironmentId } from "@t3tools/contracts"; +import { filePreviewGallery } from "../../lib/filePreviewGallery"; +import { mediaVideoPreviewUri, mediaVideoThumbnailKey } from "../../lib/videoPreviewSource"; +import { pullRequestImageSource, pullRequestMediaSource } from "./pullRequestMediaSource"; + +const location = { environmentId: EnvironmentId.make("test"), cwd: "/repo" }; +const url = "https://github.com/user-attachments/assets/abc-123"; + +describe("PR media previews", () => { + it("keeps a video resource pending until resolved and accepts refreshed capabilities", () => { + const source = { + type: "media" as const, + name: "Video", + mimeType: "video/mp4", + ...pullRequestMediaSource(url, location), + }; + expect(mediaVideoPreviewUri(source, null)).toBeNull(); + expect(mediaVideoPreviewUri(source, "https://signed/first")).toBe("https://signed/first"); + expect(mediaVideoPreviewUri(source, "https://signed/renewed")).toBe("https://signed/renewed"); + expect(mediaVideoThumbnailKey(source)).toContain("github-media"); + }); + + it("keeps every gallery image resolvable and the tapped image appears only once", () => { + const first = pullRequestImageSource(url, location); + const second = pullRequestImageSource(`${url}-second`, location); + const selected = { ...second, sourceIdentifier: "tapped" }; + const gallery = filePreviewGallery(selected, [first, second]); + expect(gallery).toEqual([first, selected]); + expect(gallery.every((item) => "resource" in item)).toBe(true); + }); + + it("leaves custom-domain media as direct URLs", () => { + const external = "https://drop.example/file.mp4"; + expect(pullRequestMediaSource(external, location)).toEqual({ uri: external }); + }); +}); diff --git a/apps/mobile/src/features/pull-requests/pullRequestMediaSource.ts b/apps/mobile/src/features/pull-requests/pullRequestMediaSource.ts new file mode 100644 index 0000000000000000000000000000000000000000..e45cd548bcc559b9b765b6c76b9844568732d8bc --- /dev/null +++ b/apps/mobile/src/features/pull-requests/pullRequestMediaSource.ts @@ -0,0 +1,26 @@ +import type { EnvironmentId } from "@t3tools/contracts"; +import { githubMediaFetchUrl } from "@t3tools/shared/githubMedia"; +import type { FilePreviewItem } from "../../components/FilePreviewModal.types"; + +export interface PullRequestMediaLocation { + readonly environmentId: EnvironmentId; + readonly cwd: string; +} + +/** Keep the original resource identity so previewers can resolve and renew its capability. */ +export function pullRequestMediaSource(url: string, location: PullRequestMediaLocation | null) { + const fetchUrl = githubMediaFetchUrl(url); + return location && fetchUrl + ? { + environmentId: location.environmentId, + resource: { _tag: "github-media" as const, cwd: location.cwd, url: fetchUrl }, + } + : { uri: url }; +} + +export function pullRequestImageSource( + url: string, + location: PullRequestMediaLocation | null, +): FilePreviewItem { + return { kind: "image", ...pullRequestMediaSource(url, location) }; +} diff --git a/apps/mobile/src/features/pull-requests/usePullRequestAgentActions.test.ts b/apps/mobile/src/features/pull-requests/usePullRequestAgentActions.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..db9100ba76577de231d9c401468f08ea8a75ccb4 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/usePullRequestAgentActions.test.ts @@ -0,0 +1,55 @@ +import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import type { PullRequestDetail } from "@t3tools/contracts"; +import { usePullRequestAgentActions } from "./usePullRequestAgentActions"; + +const state = vi.hoisted(() => ({ + connected: false, + canAsk: true, + ask: vi.fn(), +})); +vi.mock("./usePullRequestConnected", () => ({ + usePullRequestConnected: () => state.connected, +})); +vi.mock("./usePullRequestThreads", () => ({ + usePullRequestThreads: () => ({ + threads: [], + canAsk: state.canAsk, + canWatch: true, + busy: false, + isPending: true, + ask: state.ask, + }), +})); +const params = { + environmentId: "environment", + projectId: "project", + repository: "owner/repo", + number: 1, +}; +const pr = { state: "open", mergeability: "conflicting" } as PullRequestDetail; + +beforeEach(() => { + state.connected = false; + state.canAsk = true; + state.ask.mockClear(); +}); + +describe("PR agent drafts", () => { + it("opens offline drafts without waiting for linked-thread discovery", () => { + const actions = usePullRequestAgentActions(params, pr); + expect(actions.disabled).toBe(false); + actions.start("ask"); + actions.start("explain"); + expect(state.ask.mock.calls).toEqual([ + [undefined, "ask"], + [undefined, "explain"], + ]); + }); + + it("does not open drafts when the connection denies task access", () => { + state.connected = true; + state.canAsk = false; + usePullRequestAgentActions(params, pr).start("explain"); + expect(state.ask).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/mobile/src/features/pull-requests/usePullRequestAgentActions.ts b/apps/mobile/src/features/pull-requests/usePullRequestAgentActions.ts index a9d14a51cea74932aefa99654ace6fa8f4eee3d1..b3f2c38603a0deaed41fa105a37ea628fddd8570 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestAgentActions.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestAgentActions.ts @@ -5,6 +5,7 @@ import { type PullRequestAgentTask, } from "./pullRequestAgentTasks"; import type { PullRequestParams } from "./usePullRequestDiff"; +import { usePullRequestConnected } from "./usePullRequestConnected"; import { usePullRequestThreads } from "./usePullRequestThreads"; /** Shared destinations for toolbar helpers and actions beside PR readiness rows. */ @@ -14,8 +15,9 @@ export function usePullRequestAgentActions( filePath?: string, ) { const links = usePullRequestThreads(params, pr, filePath); + const connected = usePullRequestConnected(params.environmentId); const actions = pr ? pullRequestAgentActions(pr, links.canWatch) : []; - const disabled = links.busy || !pr || (links.isPending && !links.threads.length); + const disabled = links.busy || !pr || (connected && links.isPending && !links.threads.length); const start = (task: PullRequestAgentTask, threadId?: string) => { if (disabled) return; const thread = pullRequestAgentThread(links.threads, threadId); diff --git a/apps/mobile/src/features/pull-requests/usePullRequestConnected.ts b/apps/mobile/src/features/pull-requests/usePullRequestConnected.ts new file mode 100644 index 0000000000000000000000000000000000000000..ce2eff93f379fc4680f530fa6bfa14bf40d8b3e9 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/usePullRequestConnected.ts @@ -0,0 +1,8 @@ +import { useWorkspaceEnvironments } from "../../state/workspace"; + +export function usePullRequestConnected(environmentId: string) { + return useWorkspaceEnvironments().some( + (environment) => + environment.environmentId === environmentId && environment.connectionState === "connected", + ); +} diff --git a/apps/mobile/src/features/pull-requests/usePullRequestFilesViewed.ts b/apps/mobile/src/features/pull-requests/usePullRequestFilesViewed.ts index 860d39fe1a6ef037a3fed42ae92fc1fcca383c7e..cf882ac0daee21dce980878a80f02f3b8a66d9cb 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestFilesViewed.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestFilesViewed.ts @@ -1,3 +1,4 @@ +import { usePullRequestConnected } from "./usePullRequestConnected"; import { useAtomRefresh, useAtomValue } from "@effect/atom-react"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import type { PullRequestDetail } from "@t3tools/contracts"; @@ -12,9 +13,11 @@ export function usePullRequestFilesViewed(params: PullRequestParams, pr: PullReq const target = pullRequestTarget(params); const scopeKey = pullRequestReviewKey(params); const supported = Boolean(pr?.capabilities.viewedFiles); - const allowed = useAtomValue( + const permission = useAtomValue( composerPullRequests.setFilesViewed.permissionAtom(target.environmentId), ); + const connected = usePullRequestConnected(params.environmentId); + const allowed = connected && permission; const query = useEnvironmentQuery(supported ? composerPullRequests.filesViewed(target) : null); const refresh = useAtomRefresh(composerPullRequests.filesViewed(target)); const write = useAtomCommand(composerPullRequests.setFilesViewed, { reportFailure: false }); diff --git a/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts b/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts index bc4e84a6d075486b42452d6c1f03e29d673a4686..bde66bf9a4b898f6a6fe2f022d89abc1cf9bbac1 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestPermissions.ts @@ -1,10 +1,12 @@ import { useAtomValue } from "@effect/atom-react"; import type { PullRequestDetail } from "@t3tools/contracts"; +import { usePullRequestConnected } from "./usePullRequestConnected"; import { composerPullRequests } from "../../state/pull-requests"; import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff"; export function usePullRequestPermissions(params: PullRequestParams, pr: PullRequestDetail | null) { const { environmentId } = pullRequestTarget(params); + const connected = usePullRequestConnected(environmentId); const canWrite = useAtomValue(composerPullRequests.runAction.permissionAtom(environmentId)); const canComment = useAtomValue(composerPullRequests.comment.permissionAtom(environmentId)); const canReview = useAtomValue(composerPullRequests.submitReview.permissionAtom(environmentId)); @@ -13,7 +15,9 @@ export function usePullRequestPermissions(params: PullRequestParams, pr: PullReq composerPullRequests.setThreadResolution.permissionAtom(environmentId), ); return { + connected, canInlineComment: + connected && canReview && pr?.state === "open" && pr.capabilities.review.inlineComment && @@ -21,13 +25,21 @@ export function usePullRequestPermissions(params: PullRequestParams, pr: PullReq pr.capabilities.review.verdicts.some((verdict) => pr.viewerPermissions.verdicts.includes(verdict), ), - canReply: canReply && pr?.capabilities.review.reply === true && pr.viewerPermissions.comment, + canReply: + connected && + canReply && + pr?.capabilities.review.reply === true && + pr.viewerPermissions.comment, canResolve: - canResolve && pr?.capabilities.review.resolve === true && pr.viewerPermissions.resolve, - canWrite, - canComment: canComment && pr?.capabilities.comment === true && pr.viewerPermissions.comment, + connected && + canResolve && + pr?.capabilities.review.resolve === true && + pr.viewerPermissions.resolve, + canWrite: connected && canWrite, + canComment: + connected && canComment && pr?.capabilities.comment === true && pr.viewerPermissions.comment, verdicts: - canReview && pr?.state === "open" + connected && canReview && pr?.state === "open" ? pr.capabilities.review.verdicts.filter((verdict) => pr.viewerPermissions.verdicts.includes(verdict), ) diff --git a/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts b/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts index 075b62986aee433e36d8ad7b2de5f8bb1710ff4a..d38ec7d0bc2e9d23ae9281c009be904386ece5f4 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestThreads.ts @@ -27,6 +27,7 @@ import { updateComposerDraftSettings, waitForComposerDraftsLoaded, } from "../../state/use-composer-drafts"; +import { usePullRequestConnected } from "./usePullRequestConnected"; import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff"; import { appendPullRequestAgentPrompt, @@ -63,7 +64,10 @@ export function usePullRequestThreads( setLastRelations({ key, data: relations.data }); const threads = (relations.data ?? (lastRelations.key === key ? lastRelations.data : null))?.threads ?? []; - const canAsk = useEnvironmentScope(target.environmentId, AuthOrchestrationOperateScope); + const connected = usePullRequestConnected(params.environmentId); + const canOperate = useEnvironmentScope(target.environmentId, AuthOrchestrationOperateScope); + // Composing offline only writes a local draft. The outbox checks grants on delivery. + const canAsk = !connected || canOperate; const canCheckout = useEnvironmentScope(target.environmentId, AuthSourceControlWriteScope); const navigation = useNavigation(); const pending = useRef(false); @@ -76,7 +80,7 @@ export function usePullRequestThreads( pending.current = true; setBusy(true); try { - if (!thread && !canCheckout) + if (connected && !thread && !canCheckout) throw new Error("Starting work from this PR requires source control write access."); await waitForComposerDraftsLoaded(); const draftKey = thread diff --git a/apps/mobile/src/lib/videoPreviewSource.ts b/apps/mobile/src/lib/videoPreviewSource.ts index 78a5acf3f331fe93179aa3a86c49e8ca7bf82984..673db6d2c4dc78230b97560927da2b1996c2423d 100644 --- a/apps/mobile/src/lib/videoPreviewSource.ts +++ b/apps/mobile/src/lib/videoPreviewSource.ts @@ -17,7 +17,7 @@ export type MediaVideoPreviewSource = { readonly environmentId: EnvironmentId; readonly resource: Extract< AssetResource, - { readonly _tag: "attachment" | "media-file" | "draft-workspace-file" } + { readonly _tag: "attachment" | "media-file" | "draft-workspace-file" | "github-media" } >; } ); @@ -47,14 +47,22 @@ export function mediaVideoThumbnailKey(source: MediaVideoPreviewSource): string source.resource.path, source.srcFragment ?? "", ] - : [ - "media-video", - "draft-workspace-file", - source.environmentId, - source.resource.cwd, - source.resource.path, - source.srcFragment ?? "", - ], + : source.resource._tag === "github-media" + ? [ + "media-video", + "github-media", + source.environmentId, + source.resource.cwd, + source.resource.url, + ] + : [ + "media-video", + "draft-workspace-file", + source.environmentId, + source.resource.cwd, + source.resource.path, + source.srcFragment ?? "", + ], ); } diff --git a/apps/mobile/src/persistence/layer.ts b/apps/mobile/src/persistence/layer.ts index 965a9cc7f4ce2c0b9c014754928be7e85b1beced..c64adc015471ba3c580ccfb4f6ec982c09432ec0 100644 --- a/apps/mobile/src/persistence/layer.ts +++ b/apps/mobile/src/persistence/layer.ts @@ -1,5 +1,6 @@ import * as Layer from "effect/Layer"; +import * as PullRequestCacheStore from "../connection/pull-request-cache-store"; import * as EnvironmentCacheStore from "../connection/environment-cache-store"; import * as MobileDatabase from "./mobile-database"; import * as MobilePreferences from "./mobile-preferences"; @@ -11,6 +12,7 @@ const layerDependent = Layer.mergeAll( MobilePreferences.layer, MobileStorage.layer, EnvironmentCacheStore.layer, + PullRequestCacheStore.layer, ).pipe(Layer.provide(layerBase)); export const layer = Layer.merge(layerBase, layerDependent); diff --git a/apps/mobile/src/persistence/mobile-database.test.ts b/apps/mobile/src/persistence/mobile-database.test.ts index 244331471fd0875a065fc0237f342730c94da5c5..b790d03b2d94fa59c725b90b8969a14c96f3c4c7 100644 --- a/apps/mobile/src/persistence/mobile-database.test.ts +++ b/apps/mobile/src/persistence/mobile-database.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; import { vi } from "vite-plus/test"; const openDatabaseAsync = vi.hoisted(() => vi.fn()); @@ -69,3 +70,87 @@ describe("mobile database legacy cache migration", () => { ).toBeNull(); }); }); + +// Exercise the production SQL against SQLite, including the eviction window functions. +describe("offline pull request cache", () => { + const environmentId = "cached-environment" as import("@t3tools/contracts").EnvironmentId; + const otherEnvironment = "other-environment" as import("@t3tools/contracts").EnvironmentId; + + async function sqliteDatabase() { + const { DatabaseSync } = await import("node:sqlite"); + const sqlite = new DatabaseSync(":memory:"); + sqlite.exec("PRAGMA user_version = 1"); + const database = { + execAsync: async (sql: string) => { + sqlite.exec(sql); + }, + runAsync: async (sql: string, ...args: Array) => + sqlite.prepare(sql).run(...args), + getFirstAsync: async (sql: string, ...args: Array) => + sqlite.prepare(sql).get(...args), + getAllAsync: async (sql: string, ...args: Array) => + sqlite.prepare(sql).all(...args), + withExclusiveTransactionAsync: async (run: (transaction: unknown) => Promise) => { + await run(database); + }, + closeAsync: async () => { + sqlite.close(); + }, + }; + openDatabaseAsync.mockResolvedValueOnce(database); + return sqlite; + } + + it.effect("restores saved records and clears them with the environment", () => + Effect.scoped( + Effect.gen(function* () { + yield* Effect.promise(sqliteDatabase); + const database = yield* MobileDatabase.make; + yield* database.saveCache(environmentId, "pull-request", "v1:list", 1, '{"entries":[]}'); + yield* database.saveCache(otherEnvironment, "pull-request", "v1:list", 1, "other"); + expect(yield* database.loadCache(environmentId, "pull-request", "v1:list")).toEqual( + Option.some('{"entries":[]}'), + ); + yield* database.clearEnvironmentCache(environmentId); + expect((yield* database.loadCache(environmentId, "pull-request", "v1:list"))._tag).toBe( + "None", + ); + expect((yield* database.loadCache(otherEnvironment, "pull-request", "v1:list"))._tag).toBe( + "Some", + ); + }), + ), + ); + + it.effect( + "expires old records and bounds count and payload size without evicting other cache kinds", + () => + Effect.scoped( + Effect.gen(function* () { + const sqlite = yield* Effect.promise(sqliteDatabase); + const database = yield* MobileDatabase.make; + yield* database.saveCache(environmentId, "thread", "thread", 1, "thread"); + yield* database.saveCache(environmentId, "pull-request", "old", 1, "old"); + sqlite.prepare("UPDATE client_cache SET updated_at = 0 WHERE cache_key = 'old'").run(); + expect((yield* database.loadCache(environmentId, "pull-request", "old"))._tag).toBe( + "None", + ); + for (let i = 0; i < 260; i++) + yield* database.saveCache(environmentId, "pull-request", `page-${i}`, 1, "page"); + expect((yield* database.listCache("pull-request")).length).toBe(256); + const large = "x".repeat(4 * 1024 * 1024); + for (let i = 0; i < 10; i++) + yield* database.saveCache(environmentId, "pull-request", `diff-${i}`, 1, large); + const summary = yield* database.inspectCaches; + const prs = summary.find((row) => row.kind === "pull-request"); + expect(prs?.payloadBytes).toBeLessThanOrEqual(32 * 1024 * 1024); + expect(prs?.recordCount).toBeLessThanOrEqual(256); + expect((yield* database.loadCache(environmentId, "thread", "thread"))._tag).toBe("Some"); + yield* database.saveCache(environmentId, "pull-request", "oversized", 1, large + "x"); + expect((yield* database.loadCache(environmentId, "pull-request", "oversized"))._tag).toBe( + "None", + ); + }), + ), + ); +}); diff --git a/apps/mobile/src/persistence/mobile-database.ts b/apps/mobile/src/persistence/mobile-database.ts index 7c7b6bc5dedf7cd50511cbec9642219379bf8bcf..6796e93674ffb2f0f1797da0311cc60e0fbb46cd 100644 --- a/apps/mobile/src/persistence/mobile-database.ts +++ b/apps/mobile/src/persistence/mobile-database.ts @@ -22,6 +22,7 @@ export const ClientCacheKind = Schema.Literals([ "server-config", "vcs-refs", "project-favicon", + "pull-request", ]); export type ClientCacheKind = typeof ClientCacheKind.Type; @@ -294,10 +295,12 @@ const makeAvailable = Effect.gen(function* () { database.getFirstAsync<{ readonly payload: string }>( `SELECT payload FROM client_cache - WHERE environment_id = ? AND kind = ? AND cache_key = ?`, + WHERE environment_id = ? AND kind = ? AND cache_key = ? + AND (kind != 'pull-request' OR updated_at >= ?)`, environmentId, kind, cacheKey, + Date.now() - 30 * 24 * 60 * 60_000, ), catch: databaseError("load-cache"), }).pipe(Effect.map((row) => Option.fromNullishOr(row?.payload))), @@ -315,22 +318,51 @@ const makeAvailable = Effect.gen(function* () { saveCache: Effect.fn("MobileDatabase.saveCache")( (environmentId, kind, cacheKey, schemaVersion, payload) => Effect.tryPromise({ - try: () => - database.runAsync( - `INSERT INTO client_cache - (environment_id, kind, cache_key, schema_version, payload, updated_at) - VALUES (?, ?, ?, ?, ?, ?) - ON CONFLICT (environment_id, kind, cache_key) DO UPDATE SET - schema_version = excluded.schema_version, - payload = excluded.payload, - updated_at = excluded.updated_at`, - environmentId, - kind, - cacheKey, - schemaVersion, - payload, - Date.now(), - ), + try: async () => { + // Diffs can be large. Keep the offline PR cache within 32 MiB and 256 + // responses per environment, and don't let one huge diff evict it all. + if ( + kind === "pull-request" && + new TextEncoder().encode(payload).length > 4 * 1024 * 1024 + ) + return; + const save = async (transaction: SQLiteDatabase) => { + await transaction.runAsync( + `INSERT INTO client_cache + (environment_id, kind, cache_key, schema_version, payload, updated_at) + VALUES (?, ?, ?, ?, ?, ?) + ON CONFLICT (environment_id, kind, cache_key) DO UPDATE SET + schema_version = excluded.schema_version, + payload = excluded.payload, + updated_at = excluded.updated_at`, + environmentId, + kind, + cacheKey, + schemaVersion, + payload, + Date.now(), + ); + if (kind === "pull-request") { + await transaction.runAsync( + `DELETE FROM client_cache WHERE environment_id = ? AND kind = 'pull-request' + AND cache_key IN ( + SELECT cache_key FROM ( + SELECT cache_key, updated_at, + ROW_NUMBER() OVER (ORDER BY updated_at DESC, cache_key) AS position, + SUM(LENGTH(CAST(payload AS BLOB))) OVER ( + ORDER BY updated_at DESC, cache_key + ) AS total_bytes + FROM client_cache WHERE environment_id = ? AND kind = 'pull-request' + ) WHERE position > 256 OR total_bytes > 33554432 OR updated_at < ? + )`, + environmentId, + environmentId, + Date.now() - 30 * 24 * 60 * 60_000, + ); + } + }; + await save(database); + }, catch: databaseError("save-cache"), }).pipe(Effect.asVoid), ), diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index 6ffb54569ef3e9c74323cea9215dc35a29bab396..27fe1bc6b505bfbaed72a6f3ee6a3d89e6808318 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -90,11 +90,11 @@ export function PullRequestMarkdown({ data-image-gallery > {segments.map((segment) => { - if (segment.kind === "markdown") { + if (segment.kind === "markdown" || segment.media === "image") { return ( )`} cwd={cwd} threadRef={resolvedThreadRef} pullRequestPanelRef={resolvedThreadRef ?? PULL_REQUESTS_PANEL_REF} diff --git a/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts b/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts index 79ae60e928cad219aa719c871b2531ac64af83dc..cfcb355a538350991a3fe64e2a6f91b0d1639a1e 100644 --- a/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts @@ -4,149 +4,10 @@ import { type TextMatch, } from "~/vendor/mdast-find-and-replace"; -/** `id` is positional on purpose: the same attachment can be embedded twice in one body. */ -export type PullRequestBodySegment = - | { readonly id: string; readonly kind: "markdown"; readonly text: string } - | { - readonly id: string; - readonly kind: "attachment"; - readonly url: string; - /** - * What the reader can be told the upload is. The uuid says nothing on its own — the type - * only appears in the signed redirect GitHub answers with — but the shape does: GitHub - * writes a dropped image into the body as an `` tag, so a bare attachment link on its - * own line is what it does with a video. Anything else keeps the neutral name. - */ - readonly media: "video" | "unknown"; - }; - -const FENCE_PATTERN = /^\s{0,3}((?:`{3,})|(?:~{3,}))(.*)$/u; -/** - * How far a `