From 6539f2162219d0eec576da158ff49bf29b1e07ef Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 7 May 2026 12:25:07 -0700 Subject: [PATCH 1/4] Resolve issues and PRs by full GitHub URL --- apps/code/src/main/services/git/service.ts | 23 ++++- packages/git/src/utils.test.ts | 102 ++++++++++++++++++++- packages/git/src/utils.ts | 34 +++++++ 3 files changed, 157 insertions(+), 2 deletions(-) diff --git a/apps/code/src/main/services/git/service.ts b/apps/code/src/main/services/git/service.ts index 7d20939a3..4517f653e 100644 --- a/apps/code/src/main/services/git/service.ts +++ b/apps/code/src/main/services/git/service.ts @@ -35,7 +35,11 @@ import { CommitSaga } from "@posthog/git/sagas/commit"; import { DiscardFileChangesSaga } from "@posthog/git/sagas/discard"; import { PullSaga } from "@posthog/git/sagas/pull"; import { PushSaga } from "@posthog/git/sagas/push"; -import { parseGitHubUrl, parsePrUrl } from "@posthog/git/utils"; +import { + parseGitHubUrl, + parseGithubRefUrl, + parsePrUrl, +} from "@posthog/git/utils"; import { inject, injectable } from "inversify"; import { MAIN_TOKENS } from "../../di/tokens"; import { logger } from "../../utils/logger"; @@ -1544,6 +1548,23 @@ ${truncatedDiff || "(no diff available)"}${contextSection}`; const repoInfo = await this.getGitRepoInfo(directoryPath); if (!repoInfo) return []; + // Full GitHub URL: look up directly. May target a different repo than the local one. + const urlRef = parseGithubRefUrl(query); + if (urlRef && kinds.includes(urlRef.kind)) { + const repoSlug = `${urlRef.owner}/${urlRef.repo}`; + return this.fetchGhRefs( + [ + urlRef.kind === "pr" ? "pr" : "issue", + "view", + String(urlRef.number), + "--repo", + repoSlug, + ], + repoSlug, + urlRef.kind, + ); + } + const repo = await this.resolveCanonicalRepo( `${repoInfo.organization}/${repoInfo.repository}`, ); diff --git a/packages/git/src/utils.test.ts b/packages/git/src/utils.test.ts index 441cd887a..9465e8dc9 100644 --- a/packages/git/src/utils.test.ts +++ b/packages/git/src/utils.test.ts @@ -2,7 +2,12 @@ import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { forceRemove, parseGitHubUrl, parsePrUrl } from "./utils"; +import { + forceRemove, + parseGitHubUrl, + parseGithubRefUrl, + parsePrUrl, +} from "./utils"; async function fileExists(p: string): Promise { try { @@ -245,3 +250,98 @@ describe("parsePrUrl", () => { expect(parsePrUrl(url)).toBeNull(); }); }); + +describe("parseGithubRefUrl", () => { + it.each([ + // Issue URLs + [ + "https://github.com/PostHog/posthog/issues/57021", + "PostHog", + "posthog", + 57021, + "issue" as const, + ], + [ + "http://github.com/PostHog/code/issues/1", + "PostHog", + "code", + 1, + "issue" as const, + ], + // Whitespace + [ + " https://github.com/PostHog/code/issues/7\n", + "PostHog", + "code", + 7, + "issue" as const, + ], + // Query strings + fragments + [ + "https://github.com/PostHog/code/issues/42?foo=bar", + "PostHog", + "code", + 42, + "issue" as const, + ], + [ + "https://github.com/PostHog/code/issues/42#issuecomment-1", + "PostHog", + "code", + 42, + "issue" as const, + ], + // Mixed-case host + [ + "https://GITHUB.COM/PostHog/code/issues/42", + "PostHog", + "code", + 42, + "issue" as const, + ], + // PR URLs + [ + "https://github.com/PostHog/code/pull/42", + "PostHog", + "code", + 42, + "pr" as const, + ], + [ + "https://github.com/PostHog/code/pull/42/files", + "PostHog", + "code", + 42, + "pr" as const, + ], + ])("parses %s", (url, owner, repo, number, kind) => { + expect(parseGithubRefUrl(url)).toEqual({ owner, repo, number, kind }); + }); + + it.each([ + "", + " ", + null, + undefined, + "not-a-url", + // Repo-only + "https://github.com/PostHog/code", + "https://github.com/PostHog/code/", + // Wrong path keyword + "https://github.com/PostHog/code/discussions/42", + "https://github.com/PostHog/code/pulls/42", + "https://github.com/PostHog/code/issue/42", + // Bad number + "https://github.com/PostHog/code/issues/abc", + "https://github.com/PostHog/code/issues/0", + "https://github.com/PostHog/code/issues/-1", + "https://github.com/PostHog/code/issues/42.5", + "https://github.com/PostHog/code/issues/", + "https://github.com/PostHog/code/issues", + // Wrong host + "https://gitlab.com/PostHog/code/issues/42", + "https://api.github.com/repos/PostHog/code/issues/42", + ])("returns null for %s", (url) => { + expect(parseGithubRefUrl(url)).toBeNull(); + }); +}); diff --git a/packages/git/src/utils.ts b/packages/git/src/utils.ts index 34f43abd6..fc84509eb 100644 --- a/packages/git/src/utils.ts +++ b/packages/git/src/utils.ts @@ -178,6 +178,40 @@ export function parsePrUrl(prUrl: string | null | undefined): GitHubPr | null { return { owner, repo, number }; } +export interface GitHubRefLink { + owner: string; + repo: string; + number: number; + kind: "issue" | "pr"; +} + +export function parseGithubRefUrl( + url: string | null | undefined, +): GitHubRefLink | null { + if (!url) return null; + let parsed: gitUrlParse.GitUrl; + try { + parsed = gitUrlParse(url.trim()); + } catch { + return null; + } + if (parsed.source.toLowerCase() !== "github.com") return null; + // git-url-parse keeps the issue suffix in pathname but strips it from + // full_name, so read pathname directly. + const parts = parsed.pathname.split("/"); + if (parts.length < 5 || parts[0] !== "") return null; + const [, owner, repoRaw, segment, num] = parts; + if (!owner || !repoRaw || !segment || !num) return null; + let kind: "issue" | "pr"; + if (segment === "pull") kind = "pr"; + else if (segment === "issues") kind = "issue"; + else return null; + const number = Number(num); + if (!Number.isInteger(number) || number <= 0) return null; + const repo = repoRaw.replace(/\.git$/, ""); + return { owner, repo, number, kind }; +} + export function parseGitHubUrl( url: string | null | undefined, ): GitHubRepo | null { From 66153f1a735b1f668dc2e2a45d1112cab9fdb490 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 7 May 2026 12:49:31 -0700 Subject: [PATCH 2/4] unify github url parsing into one function --- .../code/src/main/services/folders/service.ts | 13 +- apps/code/src/main/services/git/service.ts | 52 +- packages/git/src/utils.test.ts | 448 +++++++----------- packages/git/src/utils.ts | 101 ++-- 4 files changed, 240 insertions(+), 374 deletions(-) diff --git a/apps/code/src/main/services/folders/service.ts b/apps/code/src/main/services/folders/service.ts index 925070108..ccf338e98 100644 --- a/apps/code/src/main/services/folders/service.ts +++ b/apps/code/src/main/services/folders/service.ts @@ -2,7 +2,7 @@ import fs from "node:fs"; import path from "node:path"; import { getRemoteUrl, isGitRepository } from "@posthog/git/queries"; import { InitRepositorySaga } from "@posthog/git/sagas/init"; -import { parseGitHubUrl } from "@posthog/git/utils"; +import { parseGithubUrl } from "@posthog/git/utils"; import { WorktreeManager } from "@posthog/git/worktree"; import type { IDialog } from "@posthog/platform/dialog"; import { normalizeRepoKey } from "@shared/utils/repo"; @@ -237,14 +237,15 @@ export class FoldersService { folderPath: string, overrideRemoteUrl: string | undefined, ): Promise { + const slug = (url: string | null | undefined) => { + const parsed = parseGithubUrl(url); + return parsed ? `${parsed.owner}/${parsed.repo}` : null; + }; if (overrideRemoteUrl) { - return ( - parseGitHubUrl(overrideRemoteUrl)?.path ?? - normalizeRepoKey(overrideRemoteUrl) - ); + return slug(overrideRemoteUrl) ?? normalizeRepoKey(overrideRemoteUrl); } const localRemoteUrl = await getRemoteUrl(folderPath); - return parseGitHubUrl(localRemoteUrl)?.path ?? null; + return slug(localRemoteUrl); } getRepositoryByRemoteUrl( diff --git a/apps/code/src/main/services/git/service.ts b/apps/code/src/main/services/git/service.ts index 4517f653e..357002d6c 100644 --- a/apps/code/src/main/services/git/service.ts +++ b/apps/code/src/main/services/git/service.ts @@ -35,11 +35,7 @@ import { CommitSaga } from "@posthog/git/sagas/commit"; import { DiscardFileChangesSaga } from "@posthog/git/sagas/discard"; import { PullSaga } from "@posthog/git/sagas/pull"; import { PushSaga } from "@posthog/git/sagas/push"; -import { - parseGitHubUrl, - parseGithubRefUrl, - parsePrUrl, -} from "@posthog/git/utils"; +import { parseGithubUrl } from "@posthog/git/utils"; import { inject, injectable } from "inversify"; import { MAIN_TOKENS } from "../../di/tokens"; import { logger } from "../../utils/logger"; @@ -222,15 +218,15 @@ export class GitService extends TypedEventEmitter { const remoteUrl = await getRemoteUrl(directoryPath); if (!remoteUrl) return null; - const repo = parseGitHubUrl(remoteUrl); - if (!repo) return null; + const parsed = parseGithubUrl(remoteUrl); + if (!parsed) return null; const branch = await getCurrentBranch(directoryPath); if (!branch) return null; return { - organization: repo.organization, - repository: repo.repository, + organization: parsed.owner, + repository: parsed.repo, remote: remoteUrl, branch, }; @@ -430,7 +426,7 @@ export class GitService extends TypedEventEmitter { const remoteUrl = await getRemoteUrl(directoryPath); if (!remoteUrl) return null; - const parsed = parseGitHubUrl(remoteUrl); + const parsed = parseGithubUrl(remoteUrl); if (!parsed) return null; const currentBranch = await getCurrentBranch(directoryPath); @@ -438,12 +434,12 @@ export class GitService extends TypedEventEmitter { let compareUrl: string | null = null; if (currentBranch && currentBranch !== defaultBranch) { - compareUrl = `https://github.com/${parsed.path}/compare/${defaultBranch}...${currentBranch}?expand=1`; + compareUrl = `https://github.com/${parsed.owner}/${parsed.repo}/compare/${defaultBranch}...${currentBranch}?expand=1`; } return { - organization: parsed.organization, - repository: parsed.repository, + organization: parsed.owner, + repository: parsed.repo, currentBranch: currentBranch ?? null, defaultBranch, compareUrl, @@ -823,7 +819,7 @@ export class GitService extends TypedEventEmitter { try { const remoteUrl = await getRemoteUrl(directoryPath); - const isGitHubRepo = !!(remoteUrl && parseGitHubUrl(remoteUrl)); + const isGitHubRepo = !!(remoteUrl && parseGithubUrl(remoteUrl)); const currentBranch = await getCurrentBranch(directoryPath); const defaultBranch = await getDefaultBranch(directoryPath).catch( () => null, @@ -894,7 +890,7 @@ export class GitService extends TypedEventEmitter { const remoteUrl = await getRemoteUrl(directoryPath); if (!remoteUrl) return null; - const parsed = parseGitHubUrl(remoteUrl); + const parsed = parseGithubUrl(remoteUrl); if (!parsed) return null; const result = await execGh([ @@ -909,7 +905,7 @@ export class GitService extends TypedEventEmitter { "--limit", "1", "--repo", - parsed.path, + `${parsed.owner}/${parsed.repo}`, ]); if (result.exitCode !== 0) { @@ -984,8 +980,8 @@ export class GitService extends TypedEventEmitter { } public async getPrChangedFiles(prUrl: string): Promise { - const pr = parsePrUrl(prUrl); - if (!pr) return []; + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") return []; const { owner, repo, number } = pr; @@ -1057,8 +1053,8 @@ export class GitService extends TypedEventEmitter { public async getPrDetailsByUrl( prUrl: string, ): Promise { - const pr = parsePrUrl(prUrl); - if (!pr) return null; + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") return null; try { const result = await execGh([ @@ -1093,8 +1089,8 @@ export class GitService extends TypedEventEmitter { prUrl: string, action: PrActionType, ): Promise { - const pr = parsePrUrl(prUrl); - if (!pr) { + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") { return { success: false, message: "Invalid PR URL" }; } @@ -1127,8 +1123,8 @@ export class GitService extends TypedEventEmitter { } public async getPrReviewComments(prUrl: string): Promise { - const pr = parsePrUrl(prUrl); - if (!pr) return []; + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") return []; const { owner, repo, number } = pr; @@ -1159,8 +1155,8 @@ export class GitService extends TypedEventEmitter { commentId: number, body: string, ): Promise { - const pr = parsePrUrl(prUrl); - if (!pr) { + const pr = parseGithubUrl(prUrl); + if (pr?.kind !== "pr") { return { success: false, comment: null }; } @@ -1549,8 +1545,8 @@ ${truncatedDiff || "(no diff available)"}${contextSection}`; if (!repoInfo) return []; // Full GitHub URL: look up directly. May target a different repo than the local one. - const urlRef = parseGithubRefUrl(query); - if (urlRef && kinds.includes(urlRef.kind)) { + const urlRef = parseGithubUrl(query); + if (urlRef && urlRef.kind !== "repo" && kinds.includes(urlRef.kind)) { const repoSlug = `${urlRef.owner}/${urlRef.repo}`; return this.fetchGhRefs( [ diff --git a/packages/git/src/utils.test.ts b/packages/git/src/utils.test.ts index 9465e8dc9..ddcd61c0f 100644 --- a/packages/git/src/utils.test.ts +++ b/packages/git/src/utils.test.ts @@ -2,12 +2,7 @@ import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { - forceRemove, - parseGitHubUrl, - parseGithubRefUrl, - parsePrUrl, -} from "./utils"; +import { forceRemove, parseGithubUrl } from "./utils"; async function fileExists(p: string): Promise { try { @@ -72,276 +67,191 @@ describe("forceRemove", () => { }); }); -describe("parseGitHubUrl", () => { - it.each([ - // HTTPS canonical forms - ["https://github.com/PostHog/code.git", "PostHog", "code"], - ["https://github.com/PostHog/code", "PostHog", "code"], - ["https://github.com/PostHog/code/", "PostHog", "code"], - ["https://github.com/PostHog/code.git/", "PostHog", "code"], - ["http://github.com/PostHog/code.git", "PostHog", "code"], - ["https://user:token@github.com/PostHog/code.git", "PostHog", "code"], - // SCP-style SSH - ["git@github.com:PostHog/code.git", "PostHog", "code"], - ["git@github.com:PostHog/code", "PostHog", "code"], - // ssh:// SSH variants - ["ssh://git@github.com/PostHog/code.git", "PostHog", "code"], - ["ssh://github.com/PostHog/code.git", "PostHog", "code"], - ["ssh://git@ssh.github.com:443/PostHog/code.git", "PostHog", "code"], - [ - "ssh://git@ssh.github.com:443/buildingapplications/bilt-landing.git", - "buildingapplications", - "bilt-landing", - ], - ["ssh://git@github.com:22/PostHog/code.git", "PostHog", "code"], - // Other protocols - ["git://github.com/PostHog/code.git", "PostHog", "code"], - ["git+https://github.com/PostHog/code.git", "PostHog", "code"], - ["git+ssh://git@github.com/PostHog/code.git", "PostHog", "code"], - // Whitespace + shorthand - [" https://github.com/PostHog/code.git\n", "PostHog", "code"], - ["\thttps://github.com/PostHog/code.git", "PostHog", "code"], - ["PostHog/code", "PostHog", "code"], - // Web URLs (path markers git-url-parse recognises) - ["https://github.com/PostHog/code/blob/main/README.md", "PostHog", "code"], - ["https://github.com/PostHog/code/tree/main", "PostHog", "code"], - ["https://github.com/PostHog/code/issues/12", "PostHog", "code"], - ["https://github.com/PostHog/code/commit/abc123", "PostHog", "code"], - // Mixed-case host (case in path is preserved) - ["git@GitHub.com:PostHog/Code.git", "PostHog", "Code"], - ["https://GITHUB.COM/PostHog/code.git", "PostHog", "code"], - ["HTTPS://github.com/PostHog/code.git", "PostHog", "code"], - // Query strings + fragments - ["https://github.com/PostHog/code.git?ref=main", "PostHog", "code"], - ["https://github.com/PostHog/code#readme", "PostHog", "code"], - // Special characters - ["https://github.com/post-hog/my-cool-repo", "post-hog", "my-cool-repo"], - ["https://github.com/PostHog/dotted.repo", "PostHog", "dotted.repo"], - ["https://github.com/Post_Hog/repo_name", "Post_Hog", "repo_name"], - ["https://github.com/123/456", "123", "456"], - ])("parses %s", (url, organization, repository) => { - expect(parseGitHubUrl(url)).toEqual({ - organization, - repository, - path: `${organization}/${repository}`, +describe("parseGithubUrl", () => { + describe("repo", () => { + it.each([ + // HTTPS canonical forms + ["https://github.com/PostHog/code.git", "PostHog", "code"], + ["https://github.com/PostHog/code", "PostHog", "code"], + ["https://github.com/PostHog/code/", "PostHog", "code"], + ["https://github.com/PostHog/code.git/", "PostHog", "code"], + ["http://github.com/PostHog/code.git", "PostHog", "code"], + ["https://user:token@github.com/PostHog/code.git", "PostHog", "code"], + // SCP-style SSH + ["git@github.com:PostHog/code.git", "PostHog", "code"], + ["git@github.com:PostHog/code", "PostHog", "code"], + // ssh:// SSH variants + ["ssh://git@github.com/PostHog/code.git", "PostHog", "code"], + ["ssh://github.com/PostHog/code.git", "PostHog", "code"], + ["ssh://git@ssh.github.com:443/PostHog/code.git", "PostHog", "code"], + [ + "ssh://git@ssh.github.com:443/buildingapplications/bilt-landing.git", + "buildingapplications", + "bilt-landing", + ], + ["ssh://git@github.com:22/PostHog/code.git", "PostHog", "code"], + // Other protocols + ["git://github.com/PostHog/code.git", "PostHog", "code"], + ["git+https://github.com/PostHog/code.git", "PostHog", "code"], + ["git+ssh://git@github.com/PostHog/code.git", "PostHog", "code"], + // Whitespace + shorthand + [" https://github.com/PostHog/code.git\n", "PostHog", "code"], + ["\thttps://github.com/PostHog/code.git", "PostHog", "code"], + ["PostHog/code", "PostHog", "code"], + // Deep links resolve to the underlying repo + [ + "https://github.com/PostHog/code/blob/main/README.md", + "PostHog", + "code", + ], + ["https://github.com/PostHog/code/tree/main", "PostHog", "code"], + ["https://github.com/PostHog/code/commit/abc123", "PostHog", "code"], + ["https://github.com/PostHog/code/wiki", "PostHog", "code"], + ["https://github.com/PostHog/code/actions", "PostHog", "code"], + [ + "https://github.com/PostHog/code/releases/tag/v1.0.0", + "PostHog", + "code", + ], + // Mixed-case host (case in path is preserved) + ["git@GitHub.com:PostHog/Code.git", "PostHog", "Code"], + ["https://GITHUB.COM/PostHog/code.git", "PostHog", "code"], + ["HTTPS://github.com/PostHog/code.git", "PostHog", "code"], + // Query strings + fragments + ["https://github.com/PostHog/code.git?ref=main", "PostHog", "code"], + ["https://github.com/PostHog/code#readme", "PostHog", "code"], + // Special characters + ["https://github.com/post-hog/my-cool-repo", "post-hog", "my-cool-repo"], + ["https://github.com/PostHog/dotted.repo", "PostHog", "dotted.repo"], + ["https://github.com/Post_Hog/repo_name", "Post_Hog", "repo_name"], + ["https://github.com/123/456", "123", "456"], + ])("parses %s", (url, owner, repo) => { + expect(parseGithubUrl(url)).toEqual({ kind: "repo", owner, repo }); }); }); - it.each([ - // Empty / nullish - "", - " ", - "\t\n", - null, - undefined, - // Non-URL strings - "not-a-url", - "PostHog", - "github.com/PostHog/code", - "//github.com/PostHog/code", - // Wrong host - "https://gitlab.com/PostHog/code.git", - "https://example.com/PostHog/code.git", - "git@gitlab.com:PostHog/code.git", - // SSH host alias (e.g. ~/.ssh/config Host github-personal). The remote may - // resolve to GitHub at connect time, but we can't know that statically. - "git@my-alias:PostHog/code.git", - "https://raw.githubusercontent.com/PostHog/code/main/README.md", - "file:///path/to/repo", - // Missing repo - "https://github.com/PostHog", - // Multiple / leading slashes - "https://github.com//PostHog/code.git", - "https://github.com/PostHog//code.git", - // Subdomains we don't trust - "https://api.github.com/repos/PostHog/code", - // GitHub web tabs git-url-parse can't isolate the repo from - "https://github.com/PostHog/code/wiki", - "https://github.com/PostHog/code/actions", - "https://github.com/PostHog/code/releases/tag/v1.0.0", - "https://github.com/PostHog/code/pull/42", - ])("returns null for %s", (url) => { - expect(parseGitHubUrl(url)).toBeNull(); - }); -}); - -describe("parsePrUrl", () => { - it.each([ - // Canonical PR URLs - ["https://github.com/PostHog/code/pull/42", "PostHog", "code", 42], - ["http://github.com/PostHog/code/pull/1", "PostHog", "code", 1], - [ - "https://github.com/buildingapplications/bilt-landing/pull/123", - "buildingapplications", - "bilt-landing", - 123, - ], - // Whitespace - [" https://github.com/PostHog/code/pull/7\n", "PostHog", "code", 7], - // PR sub-pages and tabs - ["https://github.com/PostHog/code/pull/42/files", "PostHog", "code", 42], - ["https://github.com/PostHog/code/pull/42/commits", "PostHog", "code", 42], - [ - "https://github.com/PostHog/code/pull/42/commits/abc123", - "PostHog", - "code", - 42, - ], - ["https://github.com/PostHog/code/pull/42/checks", "PostHog", "code", 42], - // Query strings + fragments - [ - "https://github.com/PostHog/code/pull/42?diff=split", - "PostHog", - "code", - 42, - ], - [ - "https://github.com/PostHog/code/pull/42#discussion_r123", - "PostHog", - "code", - 42, - ], - [ - "https://github.com/PostHog/code/pull/42/files#diff-abc", - "PostHog", - "code", - 42, - ], - // Mixed-case host - ["https://GITHUB.COM/PostHog/code/pull/42", "PostHog", "code", 42], - // Special characters in owner/repo - ["https://github.com/post-hog/my-repo/pull/42", "post-hog", "my-repo", 42], - // Large numbers (still valid integers) - ["https://github.com/PostHog/code/pull/999999", "PostHog", "code", 999999], - ])("parses %s", (url, owner, repo, number) => { - expect(parsePrUrl(url)).toEqual({ owner, repo, number }); + describe("issue", () => { + it.each([ + [ + "https://github.com/PostHog/posthog/issues/57021", + "PostHog", + "posthog", + 57021, + ], + ["http://github.com/PostHog/code/issues/1", "PostHog", "code", 1], + [" https://github.com/PostHog/code/issues/7\n", "PostHog", "code", 7], + [ + "https://github.com/PostHog/code/issues/42?foo=bar", + "PostHog", + "code", + 42, + ], + [ + "https://github.com/PostHog/code/issues/42#issuecomment-1", + "PostHog", + "code", + 42, + ], + ["https://GITHUB.COM/PostHog/code/issues/42", "PostHog", "code", 42], + [ + "https://github.com/PostHog/code/issues/999999", + "PostHog", + "code", + 999999, + ], + ])("parses %s", (url, owner, repo, number) => { + expect(parseGithubUrl(url)).toEqual({ + kind: "issue", + owner, + repo, + number, + }); + }); }); - it.each([ - // Empty / nullish - "", - " ", - null, - undefined, - // Not a URL - "not-a-url", - // Missing /pull - "https://github.com/PostHog/code", - "git@github.com:PostHog/code.git", - // Wrong path keyword - "https://github.com/PostHog/code/issues/42", - "https://github.com/PostHog/code/pulls/42", - "https://github.com/PostHog/code/discussions/42", - // Bad number - "https://github.com/PostHog/code/pull/abc", - "https://github.com/PostHog/code/pull/0", - "https://github.com/PostHog/code/pull/-1", - "https://github.com/PostHog/code/pull/42.5", - "https://github.com/PostHog/code/pull/", - "https://github.com/PostHog/code/pull", - // Double / leading slashes in the path - "https://github.com/PostHog/code//pull/42", - "https://github.com/PostHog/code/pull//42", - "https://github.com//PostHog/code/pull/42", - // Wrong host - "https://gitlab.com/PostHog/code/pull/42", - "https://api.github.com/repos/PostHog/code/pulls/42", - ])("returns null for %s", (url) => { - expect(parsePrUrl(url)).toBeNull(); + describe("pr", () => { + it.each([ + ["https://github.com/PostHog/code/pull/42", "PostHog", "code", 42], + ["http://github.com/PostHog/code/pull/1", "PostHog", "code", 1], + [" https://github.com/PostHog/code/pull/7\n", "PostHog", "code", 7], + // Sub-pages and tabs + ["https://github.com/PostHog/code/pull/42/files", "PostHog", "code", 42], + [ + "https://github.com/PostHog/code/pull/42/commits/abc123", + "PostHog", + "code", + 42, + ], + ["https://github.com/PostHog/code/pull/42/checks", "PostHog", "code", 42], + // Query strings + fragments + [ + "https://github.com/PostHog/code/pull/42?diff=split", + "PostHog", + "code", + 42, + ], + [ + "https://github.com/PostHog/code/pull/42#discussion_r123", + "PostHog", + "code", + 42, + ], + ["https://GITHUB.COM/PostHog/code/pull/42", "PostHog", "code", 42], + [ + "https://github.com/post-hog/my-repo/pull/42", + "post-hog", + "my-repo", + 42, + ], + [ + "https://github.com/PostHog/code/pull/999999", + "PostHog", + "code", + 999999, + ], + ])("parses %s", (url, owner, repo, number) => { + expect(parseGithubUrl(url)).toEqual({ kind: "pr", owner, repo, number }); + }); }); -}); -describe("parseGithubRefUrl", () => { - it.each([ - // Issue URLs - [ - "https://github.com/PostHog/posthog/issues/57021", - "PostHog", - "posthog", - 57021, - "issue" as const, - ], - [ - "http://github.com/PostHog/code/issues/1", - "PostHog", - "code", - 1, - "issue" as const, - ], - // Whitespace - [ - " https://github.com/PostHog/code/issues/7\n", - "PostHog", - "code", - 7, - "issue" as const, - ], - // Query strings + fragments - [ - "https://github.com/PostHog/code/issues/42?foo=bar", - "PostHog", - "code", - 42, - "issue" as const, - ], - [ - "https://github.com/PostHog/code/issues/42#issuecomment-1", - "PostHog", - "code", - 42, - "issue" as const, - ], - // Mixed-case host - [ - "https://GITHUB.COM/PostHog/code/issues/42", + describe("rejects", () => { + it.each([ + // Empty / nullish + "", + " ", + "\t\n", + null, + undefined, + // Non-URL strings + "not-a-url", "PostHog", - "code", - 42, - "issue" as const, - ], - // PR URLs - [ - "https://github.com/PostHog/code/pull/42", - "PostHog", - "code", - 42, - "pr" as const, - ], - [ - "https://github.com/PostHog/code/pull/42/files", - "PostHog", - "code", - 42, - "pr" as const, - ], - ])("parses %s", (url, owner, repo, number, kind) => { - expect(parseGithubRefUrl(url)).toEqual({ owner, repo, number, kind }); - }); - - it.each([ - "", - " ", - null, - undefined, - "not-a-url", - // Repo-only - "https://github.com/PostHog/code", - "https://github.com/PostHog/code/", - // Wrong path keyword - "https://github.com/PostHog/code/discussions/42", - "https://github.com/PostHog/code/pulls/42", - "https://github.com/PostHog/code/issue/42", - // Bad number - "https://github.com/PostHog/code/issues/abc", - "https://github.com/PostHog/code/issues/0", - "https://github.com/PostHog/code/issues/-1", - "https://github.com/PostHog/code/issues/42.5", - "https://github.com/PostHog/code/issues/", - "https://github.com/PostHog/code/issues", - // Wrong host - "https://gitlab.com/PostHog/code/issues/42", - "https://api.github.com/repos/PostHog/code/issues/42", - ])("returns null for %s", (url) => { - expect(parseGithubRefUrl(url)).toBeNull(); + // Wrong host + "https://gitlab.com/PostHog/code.git", + "https://example.com/PostHog/code.git", + "git@gitlab.com:PostHog/code.git", + "git@my-alias:PostHog/code.git", + "https://raw.githubusercontent.com/PostHog/code/main/README.md", + "https://api.github.com/repos/PostHog/code", + "file:///path/to/repo", + // Missing repo + "https://github.com/PostHog", + // Multiple / leading slashes + "https://github.com//PostHog/code.git", + "https://github.com/PostHog//code.git", + "https://github.com//PostHog/code/pull/42", + "https://github.com/PostHog/code//pull/42", + "https://github.com/PostHog/code/pull//42", + // Bad ref number + "https://github.com/PostHog/code/issues/abc", + "https://github.com/PostHog/code/issues/0", + "https://github.com/PostHog/code/issues/-1", + "https://github.com/PostHog/code/issues/42.5", + "https://github.com/PostHog/code/issues/", + "https://github.com/PostHog/code/pull/abc", + "https://github.com/PostHog/code/pull/0", + "https://github.com/PostHog/code/pull/", + ])("returns null for %s", (url) => { + expect(parseGithubUrl(url)).toBeNull(); + }); }); }); diff --git a/packages/git/src/utils.ts b/packages/git/src/utils.ts index fc84509eb..78a772bd0 100644 --- a/packages/git/src/utils.ts +++ b/packages/git/src/utils.ts @@ -4,11 +4,10 @@ import * as os from "node:os"; import * as path from "node:path"; import gitUrlParse from "git-url-parse"; -export interface GitHubRepo { - organization: string; - repository: string; - path: string; -} +export type GitHubUrl = + | { kind: "repo"; owner: string; repo: string } + | { kind: "issue"; owner: string; repo: string; number: number } + | { kind: "pr"; owner: string; repo: string; number: number }; export async function safeSymlink( source: string, @@ -154,40 +153,9 @@ export async function forceRemove(target: string): Promise { await fs.rm(target, { recursive: true, force: true, maxRetries: 3 }); } -export interface GitHubPr { - owner: string; - repo: string; - number: number; -} - -export function parsePrUrl(prUrl: string | null | undefined): GitHubPr | null { - if (!prUrl) return null; - let parsed: gitUrlParse.GitUrl; - try { - parsed = gitUrlParse(prUrl.trim()); - } catch { - return null; - } - if (parsed.source.toLowerCase() !== "github.com" || !parsed.full_name) { - return null; - } - const [owner, repo, kind, num] = parsed.full_name.split("/"); - if (!owner || !repo || kind !== "pull") return null; - const number = Number(num); - if (!Number.isInteger(number) || number <= 0) return null; - return { owner, repo, number }; -} - -export interface GitHubRefLink { - owner: string; - repo: string; - number: number; - kind: "issue" | "pr"; -} - -export function parseGithubRefUrl( +export function parseGithubUrl( url: string | null | undefined, -): GitHubRefLink | null { +): GitHubUrl | null { if (!url) return null; let parsed: gitUrlParse.GitUrl; try { @@ -195,40 +163,31 @@ export function parseGithubRefUrl( } catch { return null; } - if (parsed.source.toLowerCase() !== "github.com") return null; - // git-url-parse keeps the issue suffix in pathname but strips it from - // full_name, so read pathname directly. - const parts = parsed.pathname.split("/"); - if (parts.length < 5 || parts[0] !== "") return null; - const [, owner, repoRaw, segment, num] = parts; - if (!owner || !repoRaw || !segment || !num) return null; - let kind: "issue" | "pr"; - if (segment === "pull") kind = "pr"; - else if (segment === "issues") kind = "issue"; - else return null; - const number = Number(num); - if (!Number.isInteger(number) || number <= 0) return null; + // git-url-parse normalizes source to github.com for any *.github.com host, + // so check resource to reject api.github.com etc. SSH uses ssh.github.com. + const resource = parsed.resource.toLowerCase(); + if (resource !== "github.com" && resource !== "ssh.github.com") return null; + + // Read pathname directly: git-url-parse keeps /pull/N in full_name but + // strips /issues/N, and stuffs unknown path segments into owner. Pathname + // is consistent across HTTPS, SSH, and shorthand inputs. + const raw = parsed.pathname.split("/"); + if (raw[0] !== "") return null; + const parts = raw[raw.length - 1] === "" ? raw.slice(1, -1) : raw.slice(1); + if (parts.length < 2 || parts.some((p) => p === "")) return null; + const [owner, repoRaw, segment, num] = parts; const repo = repoRaw.replace(/\.git$/, ""); - return { owner, repo, number, kind }; -} -export function parseGitHubUrl( - url: string | null | undefined, -): GitHubRepo | null { - if (!url) return null; - let parsed: gitUrlParse.GitUrl; - try { - parsed = gitUrlParse(url.trim()); - } catch { - return null; + if (segment === "issues" || segment === "pull") { + const number = Number(num); + if (!Number.isInteger(number) || number <= 0) return null; + return { + kind: segment === "pull" ? "pr" : "issue", + owner, + repo, + number, + }; } - if (parsed.source.toLowerCase() !== "github.com") return null; - // git-url-parse stuffs unhandled path segments into owner (e.g. wiki, actions, - // releases pages), so reject anything that didn't cleanly split into org/repo. - if (!parsed.owner || !parsed.name || parsed.owner.includes("/")) return null; - return { - organization: parsed.owner, - repository: parsed.name, - path: `${parsed.owner}/${parsed.name}`, - }; + + return { kind: "repo", owner, repo }; } From 8e4137d5377e3b640df53ac1c19e4c34fcf78f88 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 7 May 2026 14:23:42 -0700 Subject: [PATCH 3/4] drop redundant ternary on urlRef.kind --- apps/code/src/main/services/git/service.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/apps/code/src/main/services/git/service.ts b/apps/code/src/main/services/git/service.ts index 357002d6c..90d789d0b 100644 --- a/apps/code/src/main/services/git/service.ts +++ b/apps/code/src/main/services/git/service.ts @@ -1549,13 +1549,7 @@ ${truncatedDiff || "(no diff available)"}${contextSection}`; if (urlRef && urlRef.kind !== "repo" && kinds.includes(urlRef.kind)) { const repoSlug = `${urlRef.owner}/${urlRef.repo}`; return this.fetchGhRefs( - [ - urlRef.kind === "pr" ? "pr" : "issue", - "view", - String(urlRef.number), - "--repo", - repoSlug, - ], + [urlRef.kind, "view", String(urlRef.number), "--repo", repoSlug], repoSlug, urlRef.kind, ); From ccb88114f29d2192393ba88b8a7a96f4030345a9 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 7 May 2026 14:38:22 -0700 Subject: [PATCH 4/4] cover pull/-1 and pull/42.5 in url tests --- packages/git/src/utils.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/git/src/utils.test.ts b/packages/git/src/utils.test.ts index ddcd61c0f..f25c355f2 100644 --- a/packages/git/src/utils.test.ts +++ b/packages/git/src/utils.test.ts @@ -249,6 +249,8 @@ describe("parseGithubUrl", () => { "https://github.com/PostHog/code/issues/", "https://github.com/PostHog/code/pull/abc", "https://github.com/PostHog/code/pull/0", + "https://github.com/PostHog/code/pull/-1", + "https://github.com/PostHog/code/pull/42.5", "https://github.com/PostHog/code/pull/", ])("returns null for %s", (url) => { expect(parseGithubUrl(url)).toBeNull();