diff --git a/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx b/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx index 63946016d591f..b1ff0f10ee781 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx @@ -10,27 +10,42 @@ import { } from "./PullRequestMedia"; import { videoMimeType } from "@t3tools/shared/video"; import { createContext, useContext, useMemo, useState } from "react"; -import { changeRequestReferenceAutolinker } from "@t3tools/client-runtime/pull-request-markdown"; +import { + changeRequestLinkResolver, + changeRequestReferenceAutolinker, +} from "@t3tools/client-runtime/pull-request-markdown"; import type { ReferenceAutolink } from "@t3tools/client-runtime/source-control-clients"; +import { assetUrlStateFromResult } from "@t3tools/client-runtime/state/assets"; import { linkReferenceAutolinks } from "./pullRequestAutolinks"; +import { assetEnvironment } from "../../state/assets"; +import { usePreparedConnection } from "../../state/session"; +import { useAtomQueryRunner } from "../../state/use-atom-query-runner"; import { useOpenPullRequest } from "../../native/useOpenPullRequest"; import { SelectableMarkdownText } from "../../native/SelectableMarkdownText"; import { useMarkdownPreviewStyles } from "../files/FileMarkdownPreview"; import { markdownImageSources } from "../../lib/markdownImageSources"; import { FilePreviewGallery, useFilePreviewGallery } from "../../components/FilePreviewGallery"; -import { pullRequestImageSource } from "./pullRequestMediaSource"; +import { pullRequestImageSource, pullRequestMediaSource } from "./pullRequestMediaSource"; import { FilePreviewModal, type FilePreviewSource, type FilePreviewItem, } from "../../components/FilePreviewModal"; import { ThreadMarkdownImageView } from "../threads/ThreadMarkdownImage"; -import type { MarkdownImageRenderer } from "../../native/SelectableMarkdownText"; +import type { + MarkdownImageRenderer, + MarkdownImageSourceResolver, +} from "../../native/SelectableMarkdownText"; const AutolinksContext = createContext<((text: string) => ReadonlyArray) | null>( null, ); +/** `base` is the change request URL `resolve` reads relative destinations against. */ +const LinkResolverContext = createContext<{ + readonly resolve: (url: string) => string; + readonly base: string; +} | null>(null); export function PullRequestMarkdownRepository(props: { /** The change request the markdown belongs to, whose host decides what its text links. */ @@ -44,6 +59,8 @@ export function PullRequestMarkdownRepository(props: { provider === undefined || url === undefined ? null : changeRequestReferenceAutolinker({ provider, url }); + const resolveUrl = changeRequestLinkResolver(props.changeRequest); + const linkResolver = resolveUrl && url !== undefined ? { resolve: resolveUrl, base: url } : null; const target = pullRequestTarget(props.params); const project = useProject({ environmentId: target.environmentId, @@ -53,11 +70,46 @@ export function PullRequestMarkdownRepository(props: { const media = cwd ? { environmentId: target.environmentId, cwd } : null; return ( - {props.children} + + {props.children} + ); } +/** + * Enriched draws an image in a table, list or link itself, from this source, not `renderImage`. + * It keeps a resolved source for the life of the view, so the view is keyed to everything an + * answer depends on: the change request it resolves against, the media location and the origin. + */ +function usePullRequestImageSource(resolve: (url: string) => string): { + readonly resolveImageSource: MarkdownImageSourceResolver; + readonly sourceKey: string; +} { + const location = useContext(PullRequestMediaContext); + const connection = usePreparedConnection(location?.environmentId ?? null); + const httpBaseUrl = connection._tag === "Some" ? connection.value.httpBaseUrl : null; + const createUrl = useAtomQueryRunner(assetEnvironment.createUrl, { reportFailure: false }); + const resolveImageSource: MarkdownImageSourceResolver = async (image) => { + const url = resolve(image.href); + const source = pullRequestMediaSource(url, location); + if ("uri" in source) return /^https?:\/\//i.test(url) ? { uri: url } : null; + const state = assetUrlStateFromResult( + await createUrl({ + environmentId: source.environmentId, + input: { resource: source.resource }, + }), + httpBaseUrl, + ); + if (state._tag !== "Success") throw new Error("Pull request image is unavailable"); + return { uri: state.url }; + }; + return { + resolveImageSource, + sourceKey: `${location?.environmentId ?? ""}\n${location?.cwd ?? ""}\n${httpBaseUrl ?? ""}`, + }; +} + function MarkdownImage(props: { readonly href: string; readonly alt: string | null; @@ -107,20 +159,32 @@ export function PullRequestMarkdown(props: { ); } +/** The agent writes the guide about the checkout, so its relative links name workspace files. */ +export function PullRequestGuideMarkdown(props: { readonly markdown: string }) { + return ( + + + + ); +} + function PullRequestMarkdownContent(props: { readonly markdown: string; readonly imageGroupMarkdown?: string; }) { const autolinks = useContext(AutolinksContext); + const linkResolver = useContext(LinkResolverContext); const mediaContext = useContext(PullRequestMediaContext); const markdown = linkReferenceAutolinks(props.markdown, autolinks); const openPullRequest = useOpenPullRequest(); + const resolve = (href: string) => linkResolver?.resolve(href) ?? href; const openLink = (href: string) => { - void openPullRequest(href); + void openPullRequest(resolve(href)); }; const gallery: FilePreviewItem[] = markdownImageSources( props.imageGroupMarkdown ?? props.markdown, ) + .map(resolve) .filter( (uri) => /^https?:\/\//i.test(uri) && @@ -129,17 +193,21 @@ function PullRequestMarkdownContent(props: { .map((uri) => pullRequestImageSource(uri, mediaContext)); const [preview, setPreview] = useState(null); const renderImage: MarkdownImageRenderer = (image) => ( - + ); + const { resolveImageSource, sourceKey } = usePullRequestImageSource(resolve); + const nativeKey = `${linkResolver?.base ?? ""}\n${sourceKey}`; const styles = useMarkdownPreviewStyles(); return ( setPreview(pullRequestImageSource(href, mediaContext))} + resolveImageSource={resolveImageSource} + onImagePress={(href) => setPreview(pullRequestImageSource(resolve(href), mediaContext))} /> setPreview(null)} /> diff --git a/apps/mobile/src/features/pull-requests/guide/GuideOutline.tsx b/apps/mobile/src/features/pull-requests/guide/GuideOutline.tsx index 1308d4caee799..4301a84be72c2 100644 --- a/apps/mobile/src/features/pull-requests/guide/GuideOutline.tsx +++ b/apps/mobile/src/features/pull-requests/guide/GuideOutline.tsx @@ -4,7 +4,7 @@ import { stopFilePaths } from "@t3tools/client-runtime/review-guide-tour"; import { Pressable, View } from "react-native"; import { AppText as Text } from "../../../components/ui/AppText"; import { SymbolView } from "../../../components/ui/AppSymbol"; -import { PullRequestMarkdown } from "../PullRequestMarkdown"; +import { PullRequestGuideMarkdown } from "../PullRequestMarkdown"; import { Menu } from "../../../components/ui/Menu"; import { useAppearancePreferences } from "../../settings/appearance/AppearancePreferencesProvider"; @@ -59,7 +59,7 @@ export function GuideOutline({ {guide.stops[0]?.summary ? ( - + ) : null} {guide.stops.map((stop, position) => { diff --git a/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx b/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx index caa1e72938c87..9e9eedc6d6041 100644 --- a/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx +++ b/apps/mobile/src/features/pull-requests/guide/PullRequestGuideScreen.tsx @@ -21,7 +21,7 @@ import { IconButton } from "../../../components/ui/IconButton"; import { useAppearancePreferences } from "../../settings/appearance/AppearancePreferencesProvider"; import { NativeStackScreenOptions } from "../../../native/StackHeader"; import { PullRequestCardGroup, PullRequestCardRow } from "../PullRequestControls"; -import { PullRequestMarkdown, PullRequestMarkdownRepository } from "../PullRequestMarkdown"; +import { PullRequestGuideMarkdown, PullRequestMarkdownRepository } from "../PullRequestMarkdown"; import { usePullRequestDiff, type PullRequestParams } from "../usePullRequestDiff"; import { ReviewGuideExcerpt } from "./ReviewGuideExcerpt"; import { reviewGuideState } from "./reviewGuideState"; @@ -188,7 +188,7 @@ export function PullRequestGuideStopScreen({ route }: StaticScreenProps {stop.title} - + {stop.attention ? ( @@ -197,7 +197,7 @@ export function PullRequestGuideStopScreen({ route }: StaticScreenProps - + ) : null} {stale ? ( @@ -237,7 +237,7 @@ export function PullRequestGuideStopScreen({ route }: StaticScreenProps {note.path.split("/").at(-1)}:{note.line} - + ))} @@ -266,7 +266,7 @@ export function PullRequestGuideStopScreen({ route }: StaticScreenProps Questions to consider {stop.questions.map((question) => ( - + ))} ) : null} diff --git a/apps/mobile/src/features/pull-requests/guide/ReviewGuideExcerpt.tsx b/apps/mobile/src/features/pull-requests/guide/ReviewGuideExcerpt.tsx index aa5f9e8d87f34..4700a15520b3a 100644 --- a/apps/mobile/src/features/pull-requests/guide/ReviewGuideExcerpt.tsx +++ b/apps/mobile/src/features/pull-requests/guide/ReviewGuideExcerpt.tsx @@ -11,7 +11,7 @@ import { buildReviewParsedDiff } from "../../review/reviewModel"; import { useNativeReviewDiffBridge } from "../../review/useNativeReviewDiffBridge"; import { useAppearanceCodeSurface } from "../../settings/appearance/useAppearanceCodeSurface"; import { useAppearancePreferences } from "../../settings/appearance/AppearancePreferencesProvider"; -import { PullRequestMarkdown } from "../PullRequestMarkdown"; +import { PullRequestGuideMarkdown } from "../PullRequestMarkdown"; const EMPTY_IDS: readonly string[] = []; @@ -55,7 +55,7 @@ export function ReviewGuideExcerpt({ const [NativeDiff] = useState(() => resolveNativeReviewDiffView()); return ( - {excerpt.caption.trim() ? : null} + {excerpt.caption.trim() ? : null} { + setup(); + const resolveUrl = changeRequestLinkResolver({ + provider: "github", + url: "https://github.com/team/repo/pull/8", + })!; + await using screen = await renderText( + [ + 'before > after', + "", + `b`, + "", + 'docs [ref][r] `![c](../blob/abc/c.png)`', + "", + "[r]: ../blob/abc/ref.md", + ].join("\n"), + { resolveUrl }, + ); + await expect + .element(screen.getByAltText("before > after")) + .toHaveAttribute("src", "https://github.com/team/repo/blob/abc/a.png"); + await expect + .element(screen.getByAltText("b")) + .toHaveAttribute("src", "https://github.com/team/repo/blob/abc/b.png"); + await expect + .element(screen.getByRole("link", { name: "docs" })) + .toHaveAttribute("href", "https://github.com/team/repo/blob/abc/docs.md"); + await expect + .element(screen.getByRole("link", { name: "ref" })) + .toHaveAttribute("href", "https://github.com/team/repo/blob/abc/ref.md"); + await expect.element(screen.getByText("![c](../blob/abc/c.png)", { exact: true })).toBeVisible(); +}); + describe("ChatMarkdown favicon privacy", () => { it("suppresses private link images while preserving public links across updates", async () => { setup(); diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 99c2ebbc8f40f..5b8c4e82a42eb 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -245,6 +245,8 @@ interface ChatMarkdownProps { `cwd`'s; without it a private repository's uploads load unauthenticated and 404. */ sourceControlMedia?: boolean | undefined; headingLevelOffset?: number | undefined; + /** Rewrites every destination, raw HTML's included, before the renderer reads it. */ + resolveUrl?: ((url: string) => string) | undefined; } export interface ChatMarkdownContextReference { @@ -491,6 +493,7 @@ function useChatMarkdownState({ renderContextReference, headingLevelOffset = 0, sourceControlMedia = false, + resolveUrl, }: ChatMarkdownProps) { const resolvedTheme = useRenderedAppearance(); const markdownRef = useRef(null); @@ -644,7 +647,8 @@ function useChatMarkdownState({ }); }; const diffThemeName = resolveDiffThemeName(resolvedTheme); - const markdownUrlTransform = (href: string) => { + const markdownUrlTransform = (url: string) => { + const href = resolveUrl?.(url) ?? url; if (parseAssistantCitationHref(href)) return href; if (parseComposerContextHref(href)) return href; if (parseThreadLinkHref(href)) return href; diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 135ae2970ceb9..25eb1c84c58c6 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -1,6 +1,9 @@ import { projectChangeRequestUrl } from "@t3tools/client-runtime/change-request-links"; import { normalizeThreadPullRequestKey } from "@t3tools/client-runtime/thread-pull-requests"; -import { changeRequestReferenceAutolinker } from "@t3tools/client-runtime/pull-request-markdown"; +import { + changeRequestLinkResolver, + changeRequestReferenceAutolinker, +} from "@t3tools/client-runtime/pull-request-markdown"; import { useAtomValue } from "@effect/atom-react"; import { usePullRequestStack } from "~/state/usePullRequestStack"; import { @@ -239,7 +242,11 @@ function pullRequestMarkdownContext( detail: Pick | null, threadRef: ScopedThreadRef | null, ) { - return { autolinks: changeRequestReferenceAutolinker(detail), threadRef }; + return { + autolinks: changeRequestReferenceAutolinker(detail), + resolveUrl: changeRequestLinkResolver(detail), + threadRef, + }; } function pullRequestStackReference( diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index 7086d5441bc5a..3ca14b9932ee5 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -23,6 +23,7 @@ import { remarkPullRequestAutolinks, splitPullRequestBody } from "./pullRequestM export const PullRequestMarkdownContext = createContext<{ autolinks: ((text: string) => ReadonlyArray) | null; + resolveUrl: ((url: string) => string) | null; threadRef: ScopedThreadRef | null; } | null>(null); @@ -93,6 +94,7 @@ export function PullRequestMarkdown({ pullRequestPanelRef={resolvedThreadRef ?? PULL_REQUESTS_PANEL_REF} environmentId={environmentId} extraRemarkPlugins={extraRemarkPlugins} + resolveUrl={context?.resolveUrl ?? undefined} sourceControlMedia /> ); diff --git a/apps/web/src/components/pullRequest/guide/GuideProse.tsx b/apps/web/src/components/pullRequest/guide/GuideProse.tsx index e117191fb97a6..f2643046a1360 100644 --- a/apps/web/src/components/pullRequest/guide/GuideProse.tsx +++ b/apps/web/src/components/pullRequest/guide/GuideProse.tsx @@ -3,7 +3,7 @@ import { createContext, useContext } from "react"; import { cn } from "~/lib/utils"; -import { PullRequestMarkdown } from "../PullRequestMarkdown"; +import { PullRequestMarkdown, PullRequestMarkdownContext } from "../PullRequestMarkdown"; export const GuideProseContext = createContext<{ readonly environmentId: EnvironmentId; @@ -20,13 +20,19 @@ export function GuideProse({ className?: string; }) { const context = useContext(GuideProseContext); + const markdownContext = useContext(PullRequestMarkdownContext); if (plain || context === null) return text; return ( - + // The agent writes the guide about the checkout, so its relative links name workspace files. + + + ); } diff --git a/packages/client-runtime/src/pullRequestMarkdown.ts b/packages/client-runtime/src/pullRequestMarkdown.ts index 1e03c32509e9a..63f9974d85dd3 100644 --- a/packages/client-runtime/src/pullRequestMarkdown.ts +++ b/packages/client-runtime/src/pullRequestMarkdown.ts @@ -132,6 +132,18 @@ export function splitPullRequestBody(body: string): ReadonlyArray string) | null { + if (!changeRequest) return null; + const resolve = sourceControlClients.get(changeRequest.provider).resolveRelativeUrl; + return resolve === undefined ? null : (url) => resolve(url, changeRequest.url) ?? url; +} + /** * Finds the references the change request's host links in plain text, resolved against the change * request's repository. Null when the host links none. diff --git a/packages/source-control-core/src/shared/definition.ts b/packages/source-control-core/src/shared/definition.ts index e9e68325b7418..4c51de24a08cd 100644 --- a/packages/source-control-core/src/shared/definition.ts +++ b/packages/source-control-core/src/shared/definition.ts @@ -286,6 +286,12 @@ export interface SourceControlDefinition { text: string, repositoryUrl: string, ) => ReadonlyArray; + /** + * The absolute URL for a link destination this host's bodies write relative to the change + * request's page at `changeRequestUrl`, or null for any other destination. Absent for a host + * whose bodies link absolutely. + */ + readonly resolveRelativeUrl?: (url: string, changeRequestUrl: string) => string | null; /** Where the host lists a change request's checks. Absent means the change request's own page. */ readonly changeRequestChecksUrl?: (changeRequestUrl: string) => string; /** diff --git a/packages/source-control-github/src/shared/definition.test.ts b/packages/source-control-github/src/shared/definition.test.ts index 3aadd4697b6fe..3fde741aebf30 100644 --- a/packages/source-control-github/src/shared/definition.test.ts +++ b/packages/source-control-github/src/shared/definition.test.ts @@ -159,3 +159,26 @@ describe("GitHub media URLs", () => { } }); }); + +describe("GitHub relative URLs", () => { + const resolve = (url: string) => + definition.resolveRelativeUrl?.(url, "https://github.com/o/r/pull/5"); + + it("resolves a pull request body's relative destinations against the pull request page", () => { + expect(resolve("../blob/dd60e3cd/docs/shot.png?raw=true")).toBe( + "https://github.com/o/r/blob/dd60e3cd/docs/shot.png?raw=true", + ); + expect(resolve("/other/r/blob/801d91ff/docs/shot.webp?raw=true")).toBe( + "https://github.com/other/r/blob/801d91ff/docs/shot.webp?raw=true", + ); + for (const url of [ + "https://example.com/shot.png", + "#x", + "mailto:a@example.com", + "//evil.example/shot.png", + "\\\\evil.example\\shot.png", + ]) { + expect(resolve(url), url).toBeNull(); + } + }); +}); diff --git a/packages/source-control-github/src/shared/definition.ts b/packages/source-control-github/src/shared/definition.ts index 6ac8aa305f3cd..9d9170750c2eb 100644 --- a/packages/source-control-github/src/shared/definition.ts +++ b/packages/source-control-github/src/shared/definition.ts @@ -44,6 +44,21 @@ function isRepositoryPath(repository: string): boolean { ); } +const ABSOLUTE_OR_PAGE_LOCAL = /^(?:[a-z][a-z\d+.-]*:|[#?]|$)/iu; + +/** As github.com's browser reads a body's destinations: relative to the pull request's page. */ +function resolveRelativeUrl(url: string, changeRequestUrl: string): string | null { + if (ABSOLUTE_OR_PAGE_LOCAL.test(url)) return null; + try { + const page = new URL(changeRequestUrl); + const resolved = new URL(url, page); + // `//host/x`, and the `\\host/x` a browser reads the same way, leave the page's origin. + return resolved.origin === page.origin ? resolved.href : null; + } catch { + return null; + } +} + /** Signing in through the T3 Code for GitHub app lifts the per-user API rate limit to the app's. */ export const APP_SIGN_IN: SourceControlAppSignIn = { appName: "T3 Code for GitHub", @@ -127,6 +142,7 @@ export const definition = defineSourceControl({ ? null : new URL(`/${encodeURIComponent(login)}`, repositoryUrl).toString(), referenceAutolinks: gitHubReferenceAutolinks, + resolveRelativeUrl, changeRequestChecksUrl: (changeRequestUrl) => `${changeRequestUrl.replace(/\/+$/u, "")}/checks`, // Before servers reported their actions, GitHub was the only host they acted on from a list. changeRequestActionsWhenUnreported: new Set(["close", "merge", "ready", "reopen"] as const),