Fix pull request image previews for uploaded attachments (#1486)
This commit is contained in:
@@ -303,6 +303,7 @@ function pullRequestTimelineJson(overrides: Record<string, unknown> = {}): strin
|
|||||||
id: "PRR_approved",
|
id: "PRR_approved",
|
||||||
state: "APPROVED",
|
state: "APPROVED",
|
||||||
body: "Looks good to me.",
|
body: "Looks good to me.",
|
||||||
|
bodyHTML: "<p>Looks good to me.</p>",
|
||||||
url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-1",
|
url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-1",
|
||||||
submittedAt: "2026-04-02T13:52:14Z",
|
submittedAt: "2026-04-02T13:52:14Z",
|
||||||
author: {
|
author: {
|
||||||
@@ -315,6 +316,7 @@ function pullRequestTimelineJson(overrides: Record<string, unknown> = {}): strin
|
|||||||
id: "PRR_empty_commented",
|
id: "PRR_empty_commented",
|
||||||
state: "COMMENTED",
|
state: "COMMENTED",
|
||||||
body: "",
|
body: "",
|
||||||
|
bodyHTML: "",
|
||||||
url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-2",
|
url: "https://github.com/parentOwner/parentRepo/pull/42#pullrequestreview-2",
|
||||||
submittedAt: "2026-04-02T13:50:00Z",
|
submittedAt: "2026-04-02T13:50:00Z",
|
||||||
author: null,
|
author: null,
|
||||||
@@ -327,6 +329,7 @@ function pullRequestTimelineJson(overrides: Record<string, unknown> = {}): strin
|
|||||||
{
|
{
|
||||||
id: "IC_later",
|
id: "IC_later",
|
||||||
body: "Can we add a regression test?",
|
body: "Can we add a regression test?",
|
||||||
|
bodyHTML: "<p>Can we add a regression test?</p>",
|
||||||
url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-3",
|
url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-3",
|
||||||
createdAt: "2026-04-02T13:55:00Z",
|
createdAt: "2026-04-02T13:55:00Z",
|
||||||
author: {
|
author: {
|
||||||
@@ -875,6 +878,7 @@ describe("GitHubService", () => {
|
|||||||
});
|
});
|
||||||
expect(runner.calls[0]?.args[3]).toContain("reviews(first: 100)");
|
expect(runner.calls[0]?.args[3]).toContain("reviews(first: 100)");
|
||||||
expect(runner.calls[0]?.args[3]).toContain("comments(first: 100)");
|
expect(runner.calls[0]?.args[3]).toContain("comments(first: 100)");
|
||||||
|
expect(runner.calls[0]?.args[3]).toContain("bodyHTML");
|
||||||
expect(runner.calls[0]?.args[3]).toContain("avatarUrl");
|
expect(runner.calls[0]?.args[3]).toContain("avatarUrl");
|
||||||
expect(runner.calls[0]?.args[3]).toContain("reviewThreads(first: 100)");
|
expect(runner.calls[0]?.args[3]).toContain("reviewThreads(first: 100)");
|
||||||
expect(timeline).toEqual({
|
expect(timeline).toEqual({
|
||||||
@@ -920,6 +924,200 @@ describe("GitHubService", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("rewrites GitHub attachment image URLs in timeline comments", async () => {
|
||||||
|
const privateAttachmentUrl =
|
||||||
|
"https://private-user-images.githubusercontent.com/123/asset.png?jwt=abc&expires=123";
|
||||||
|
const runner = createRunner([
|
||||||
|
pullRequestTimelineJson({
|
||||||
|
reviews: {
|
||||||
|
nodes: [],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
comments: {
|
||||||
|
nodes: [
|
||||||
|
{
|
||||||
|
id: "IC_attachment",
|
||||||
|
body: "Screenshot: ",
|
||||||
|
bodyHTML: `<p>Screenshot: <img alt="bug" src="${privateAttachmentUrl.replaceAll("&", "&")}" /></p>`,
|
||||||
|
url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-4",
|
||||||
|
createdAt: "2026-04-02T13:56:00Z",
|
||||||
|
author: null,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
const service = createGitHubService({
|
||||||
|
runner: runner.runner,
|
||||||
|
resolveGhPath: async () => "/usr/bin/gh",
|
||||||
|
now: () => 100,
|
||||||
|
});
|
||||||
|
|
||||||
|
const timeline = await service.getPullRequestTimeline({
|
||||||
|
cwd: "/repo",
|
||||||
|
prNumber: 42,
|
||||||
|
repoOwner: "parentOwner",
|
||||||
|
repoName: "parentRepo",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(timeline.items).toHaveLength(1);
|
||||||
|
expect(timeline.items[0]).toMatchObject({
|
||||||
|
kind: "comment",
|
||||||
|
id: "IC_attachment",
|
||||||
|
body: `Screenshot: `,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rewrites GitHub attachment image URLs in review thread comments", async () => {
|
||||||
|
const privateAttachmentUrl =
|
||||||
|
"https://private-user-images.githubusercontent.com/123/thread.png?jwt=thread";
|
||||||
|
const runner = createRunner([
|
||||||
|
pullRequestTimelineJson({
|
||||||
|
reviews: {
|
||||||
|
nodes: [],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
comments: {
|
||||||
|
nodes: [],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
reviewThreads: {
|
||||||
|
nodes: [
|
||||||
|
{
|
||||||
|
id: "PRRT_attachment",
|
||||||
|
path: "packages/app/src/git/pull-request-panel/data.ts",
|
||||||
|
line: 24,
|
||||||
|
startLine: 20,
|
||||||
|
isResolved: false,
|
||||||
|
isOutdated: false,
|
||||||
|
comments: {
|
||||||
|
nodes: [
|
||||||
|
{
|
||||||
|
id: "PRRC_attachment",
|
||||||
|
body: '<img src="https://github.com/user-attachments/assets/thread-asset" alt="thread" />',
|
||||||
|
bodyHTML: `<p><img alt="thread" src="${privateAttachmentUrl}" /></p>`,
|
||||||
|
url: "https://github.com/parentOwner/parentRepo/pull/42#discussion_r2",
|
||||||
|
createdAt: "2026-04-02T13:51:00Z",
|
||||||
|
author: null,
|
||||||
|
pullRequestReview: { id: "PRR_empty_commented" },
|
||||||
|
},
|
||||||
|
],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
const service = createGitHubService({
|
||||||
|
runner: runner.runner,
|
||||||
|
resolveGhPath: async () => "/usr/bin/gh",
|
||||||
|
now: () => 100,
|
||||||
|
});
|
||||||
|
|
||||||
|
const timeline = await service.getPullRequestTimeline({
|
||||||
|
cwd: "/repo",
|
||||||
|
prNumber: 42,
|
||||||
|
repoOwner: "parentOwner",
|
||||||
|
repoName: "parentRepo",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(timeline.items).toHaveLength(1);
|
||||||
|
expect(timeline.items[0]).toMatchObject({
|
||||||
|
kind: "comment",
|
||||||
|
id: "PRRC_attachment",
|
||||||
|
body: `<img src="${privateAttachmentUrl}" alt="thread" />`,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves external badge images unchanged", async () => {
|
||||||
|
const runner = createRunner([
|
||||||
|
pullRequestTimelineJson({
|
||||||
|
reviews: {
|
||||||
|
nodes: [],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
comments: {
|
||||||
|
nodes: [
|
||||||
|
{
|
||||||
|
id: "IC_badge",
|
||||||
|
body: "",
|
||||||
|
bodyHTML:
|
||||||
|
'<p><img alt="build" src="https://camo.githubusercontent.com/badge-signature" /></p>',
|
||||||
|
url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-5",
|
||||||
|
createdAt: "2026-04-02T13:57:00Z",
|
||||||
|
author: null,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
const service = createGitHubService({
|
||||||
|
runner: runner.runner,
|
||||||
|
resolveGhPath: async () => "/usr/bin/gh",
|
||||||
|
now: () => 100,
|
||||||
|
});
|
||||||
|
|
||||||
|
const timeline = await service.getPullRequestTimeline({
|
||||||
|
cwd: "/repo",
|
||||||
|
prNumber: 42,
|
||||||
|
repoOwner: "parentOwner",
|
||||||
|
repoName: "parentRepo",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(timeline.items[0]).toMatchObject({
|
||||||
|
kind: "comment",
|
||||||
|
id: "IC_badge",
|
||||||
|
body: "",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves GitHub attachment image URLs unchanged when rendered images do not match", async () => {
|
||||||
|
const body = "";
|
||||||
|
const runner = createRunner([
|
||||||
|
pullRequestTimelineJson({
|
||||||
|
reviews: {
|
||||||
|
nodes: [],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
comments: {
|
||||||
|
nodes: [
|
||||||
|
{
|
||||||
|
id: "IC_mismatch",
|
||||||
|
body,
|
||||||
|
bodyHTML: "<p>No rendered image.</p>",
|
||||||
|
url: "https://github.com/parentOwner/parentRepo/pull/42#issuecomment-6",
|
||||||
|
createdAt: "2026-04-02T13:58:00Z",
|
||||||
|
author: null,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
pageInfo: { hasNextPage: false },
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
const service = createGitHubService({
|
||||||
|
runner: runner.runner,
|
||||||
|
resolveGhPath: async () => "/usr/bin/gh",
|
||||||
|
now: () => 100,
|
||||||
|
});
|
||||||
|
|
||||||
|
const timeline = await service.getPullRequestTimeline({
|
||||||
|
cwd: "/repo",
|
||||||
|
prNumber: 42,
|
||||||
|
repoOwner: "parentOwner",
|
||||||
|
repoName: "parentRepo",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(timeline.items[0]).toMatchObject({
|
||||||
|
kind: "comment",
|
||||||
|
id: "IC_mismatch",
|
||||||
|
body,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("maps inline review thread comments as chronological PR timeline comments with location", async () => {
|
it("maps inline review thread comments as chronological PR timeline comments with location", async () => {
|
||||||
const runner = createRunner([
|
const runner = createRunner([
|
||||||
pullRequestTimelineJson({
|
pullRequestTimelineJson({
|
||||||
@@ -937,6 +1135,7 @@ describe("GitHubService", () => {
|
|||||||
{
|
{
|
||||||
id: "PRRC_1",
|
id: "PRRC_1",
|
||||||
body: "This should include line context.",
|
body: "This should include line context.",
|
||||||
|
bodyHTML: "<p>This should include line context.</p>",
|
||||||
url: "https://github.com/parentOwner/parentRepo/pull/42#discussion_r1",
|
url: "https://github.com/parentOwner/parentRepo/pull/42#discussion_r1",
|
||||||
createdAt: "2026-04-02T13:51:00Z",
|
createdAt: "2026-04-02T13:51:00Z",
|
||||||
author: {
|
author: {
|
||||||
|
|||||||
@@ -227,6 +227,7 @@ const PullRequestTimelineReviewNodeSchema = z.object({
|
|||||||
id: z.string().catch(""),
|
id: z.string().catch(""),
|
||||||
state: z.string().catch(""),
|
state: z.string().catch(""),
|
||||||
body: z.string().nullable().catch(null),
|
body: z.string().nullable().catch(null),
|
||||||
|
bodyHTML: z.string().nullable().catch(null),
|
||||||
url: z.string().catch(""),
|
url: z.string().catch(""),
|
||||||
submittedAt: z.string().nullable().catch(null),
|
submittedAt: z.string().nullable().catch(null),
|
||||||
author: TimelineAuthorSchema,
|
author: TimelineAuthorSchema,
|
||||||
@@ -235,6 +236,7 @@ const PullRequestTimelineReviewNodeSchema = z.object({
|
|||||||
const PullRequestTimelineCommentNodeSchema = z.object({
|
const PullRequestTimelineCommentNodeSchema = z.object({
|
||||||
id: z.string().catch(""),
|
id: z.string().catch(""),
|
||||||
body: z.string().nullable().catch(null),
|
body: z.string().nullable().catch(null),
|
||||||
|
bodyHTML: z.string().nullable().catch(null),
|
||||||
url: z.string().catch(""),
|
url: z.string().catch(""),
|
||||||
createdAt: z.string().nullable().catch(null),
|
createdAt: z.string().nullable().catch(null),
|
||||||
author: TimelineAuthorSchema,
|
author: TimelineAuthorSchema,
|
||||||
@@ -413,6 +415,7 @@ query PullRequestTimeline($owner: String!, $name: String!, $number: Int!) {
|
|||||||
id
|
id
|
||||||
state
|
state
|
||||||
body
|
body
|
||||||
|
bodyHTML
|
||||||
url
|
url
|
||||||
submittedAt
|
submittedAt
|
||||||
author {
|
author {
|
||||||
@@ -429,6 +432,7 @@ query PullRequestTimeline($owner: String!, $name: String!, $number: Int!) {
|
|||||||
nodes {
|
nodes {
|
||||||
id
|
id
|
||||||
body
|
body
|
||||||
|
bodyHTML
|
||||||
url
|
url
|
||||||
createdAt
|
createdAt
|
||||||
author {
|
author {
|
||||||
@@ -453,6 +457,7 @@ query PullRequestTimeline($owner: String!, $name: String!, $number: Int!) {
|
|||||||
nodes {
|
nodes {
|
||||||
id
|
id
|
||||||
body
|
body
|
||||||
|
bodyHTML
|
||||||
url
|
url
|
||||||
createdAt
|
createdAt
|
||||||
author {
|
author {
|
||||||
@@ -2265,7 +2270,7 @@ function toPullRequestTimelineReviewItem(
|
|||||||
author: review.author?.login ?? "unknown",
|
author: review.author?.login ?? "unknown",
|
||||||
authorUrl: review.author?.url ?? null,
|
authorUrl: review.author?.url ?? null,
|
||||||
avatarUrl: review.author?.avatarUrl ?? null,
|
avatarUrl: review.author?.avatarUrl ?? null,
|
||||||
body: review.body ?? "",
|
body: normalizeGitHubTimelineBody(review.body ?? "", review.bodyHTML ?? ""),
|
||||||
createdAt: parseOptionalTime(review.submittedAt ?? null),
|
createdAt: parseOptionalTime(review.submittedAt ?? null),
|
||||||
url: review.url,
|
url: review.url,
|
||||||
reviewState,
|
reviewState,
|
||||||
@@ -2282,12 +2287,131 @@ function toPullRequestTimelineCommentItem(
|
|||||||
author: comment.author?.login ?? "unknown",
|
author: comment.author?.login ?? "unknown",
|
||||||
authorUrl: comment.author?.url ?? null,
|
authorUrl: comment.author?.url ?? null,
|
||||||
avatarUrl: comment.author?.avatarUrl ?? null,
|
avatarUrl: comment.author?.avatarUrl ?? null,
|
||||||
body: comment.body ?? "",
|
body: normalizeGitHubTimelineBody(comment.body ?? "", comment.bodyHTML ?? ""),
|
||||||
createdAt: parseOptionalTime(comment.createdAt ?? null),
|
createdAt: parseOptionalTime(comment.createdAt ?? null),
|
||||||
url: comment.url,
|
url: comment.url,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface ImageSourceReference {
|
||||||
|
src: string;
|
||||||
|
start: number;
|
||||||
|
end: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
const RAW_MARKDOWN_IMAGE_RE = /!\[[^\]]*\]\(\s*([^\s)]+)(?:\s+["'][^)]*["'])?\s*\)/g;
|
||||||
|
const HTML_IMAGE_RE = /<img\b[^>]*\bsrc\s*=\s*(["'])(.*?)\1[^>]*>/gi;
|
||||||
|
const GITHUB_RENDERED_IMAGE_HOSTS = new Set([
|
||||||
|
"camo.githubusercontent.com",
|
||||||
|
"private-user-images.githubusercontent.com",
|
||||||
|
]);
|
||||||
|
|
||||||
|
function normalizeGitHubTimelineBody(body: string, bodyHTML: string): string {
|
||||||
|
const rawImages = extractRawImageSourceReferences(body);
|
||||||
|
if (rawImages.length === 0) {
|
||||||
|
return body;
|
||||||
|
}
|
||||||
|
|
||||||
|
const renderedSources = extractRenderedImageSources(bodyHTML);
|
||||||
|
if (renderedSources.length !== rawImages.length) {
|
||||||
|
return body;
|
||||||
|
}
|
||||||
|
|
||||||
|
let cursor = 0;
|
||||||
|
let normalized = "";
|
||||||
|
for (let index = 0; index < rawImages.length; index += 1) {
|
||||||
|
const rawImage = rawImages[index];
|
||||||
|
const renderedSrc = renderedSources[index];
|
||||||
|
if (
|
||||||
|
!rawImage ||
|
||||||
|
!renderedSrc ||
|
||||||
|
!isRawGitHubAttachmentSource(rawImage.src) ||
|
||||||
|
!isGitHubRenderedImageSource(renderedSrc)
|
||||||
|
) {
|
||||||
|
return body;
|
||||||
|
}
|
||||||
|
normalized += body.slice(cursor, rawImage.start);
|
||||||
|
normalized += renderedSrc;
|
||||||
|
cursor = rawImage.end;
|
||||||
|
}
|
||||||
|
normalized += body.slice(cursor);
|
||||||
|
return normalized;
|
||||||
|
}
|
||||||
|
|
||||||
|
function extractRawImageSourceReferences(source: string): ImageSourceReference[] {
|
||||||
|
const references = [
|
||||||
|
...extractHtmlImageSourceReferences(source),
|
||||||
|
...extractMarkdownImageSourceReferences(source),
|
||||||
|
];
|
||||||
|
return references.sort((left, right) => left.start - right.start);
|
||||||
|
}
|
||||||
|
|
||||||
|
function extractRenderedImageSources(source: string): string[] {
|
||||||
|
return extractHtmlImageSourceReferences(source).map((reference) => reference.src);
|
||||||
|
}
|
||||||
|
|
||||||
|
function extractHtmlImageSourceReferences(source: string): ImageSourceReference[] {
|
||||||
|
const references: ImageSourceReference[] = [];
|
||||||
|
HTML_IMAGE_RE.lastIndex = 0;
|
||||||
|
let match: RegExpExecArray | null;
|
||||||
|
while ((match = HTML_IMAGE_RE.exec(source)) !== null) {
|
||||||
|
const src = decodeHtmlAttribute(match[2] ?? "");
|
||||||
|
if (!src) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const rawAttributeSrc = match[2] ?? "";
|
||||||
|
const start = match.index + match[0].indexOf(rawAttributeSrc);
|
||||||
|
references.push({ src, start, end: start + rawAttributeSrc.length });
|
||||||
|
}
|
||||||
|
return references;
|
||||||
|
}
|
||||||
|
|
||||||
|
function extractMarkdownImageSourceReferences(source: string): ImageSourceReference[] {
|
||||||
|
const references: ImageSourceReference[] = [];
|
||||||
|
RAW_MARKDOWN_IMAGE_RE.lastIndex = 0;
|
||||||
|
let match: RegExpExecArray | null;
|
||||||
|
while ((match = RAW_MARKDOWN_IMAGE_RE.exec(source)) !== null) {
|
||||||
|
const src = match[1] ?? "";
|
||||||
|
if (!src) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const start = match.index + match[0].indexOf(src);
|
||||||
|
references.push({ src, start, end: start + src.length });
|
||||||
|
}
|
||||||
|
return references;
|
||||||
|
}
|
||||||
|
|
||||||
|
function isRawGitHubAttachmentSource(src: string): boolean {
|
||||||
|
try {
|
||||||
|
const url = new URL(src);
|
||||||
|
return (
|
||||||
|
url.protocol === "https:" &&
|
||||||
|
url.hostname === "github.com" &&
|
||||||
|
url.pathname.startsWith("/user-attachments/assets/")
|
||||||
|
);
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function isGitHubRenderedImageSource(src: string): boolean {
|
||||||
|
try {
|
||||||
|
const url = new URL(src);
|
||||||
|
return url.protocol === "https:" && GITHUB_RENDERED_IMAGE_HOSTS.has(url.hostname);
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function decodeHtmlAttribute(value: string): string {
|
||||||
|
return value
|
||||||
|
.replaceAll("&", "&")
|
||||||
|
.replaceAll(""", '"')
|
||||||
|
.replaceAll("'", "'")
|
||||||
|
.replaceAll("<", "<")
|
||||||
|
.replaceAll(">", ">");
|
||||||
|
}
|
||||||
|
|
||||||
function toPullRequestTimelineReviewThreadItems(
|
function toPullRequestTimelineReviewThreadItems(
|
||||||
thread: z.infer<typeof PullRequestReviewThreadNodeSchema>,
|
thread: z.infer<typeof PullRequestReviewThreadNodeSchema>,
|
||||||
): PullRequestTimelineItem[] {
|
): PullRequestTimelineItem[] {
|
||||||
|
|||||||
Reference in New Issue
Block a user