diff --git a/src/renderer/src/lib/github-links.test.ts b/src/renderer/src/lib/github-links.test.ts index 320468d6f..f4e5c7705 100644 --- a/src/renderer/src/lib/github-links.test.ts +++ b/src/renderer/src/lib/github-links.test.ts @@ -1,15 +1,98 @@ import { describe, expect, it } from 'vitest' -import { normalizeGitHubLinkQuery, parseGitHubIssueOrPRNumber } from './github-links' +import { + normalizeGitHubLinkQuery, + parseGitHubIssueOrPRLink, + parseGitHubIssueOrPRNumber +} from './github-links' describe('parseGitHubIssueOrPRNumber', () => { it('parses plain issue numbers and GitHub pull request URLs', () => { expect(parseGitHubIssueOrPRNumber('42')).toBe(42) expect(parseGitHubIssueOrPRNumber('#42')).toBe(42) expect(parseGitHubIssueOrPRNumber('https://github.com/stablyai/orca/pull/123')).toBe(123) + expect(parseGitHubIssueOrPRNumber('https://github.com/stablyai/orca/issues/923')).toBe(923) }) - it('rejects non-GitHub URLs', () => { + it('parses GitHub item URLs with trailing page segments', () => { + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/pull/1965/changes')).toBe(1965) + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/pull/1965/files')).toBe(1965) + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/pull/1965/commits')).toBe(1965) + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/issues/923/comments')).toBe(923) + }) + + it('parses trailing segments with query, fragment, and repeated slashes', () => { + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/pull/1965/changes?diff=split')).toBe( + 1965 + ) + expect( + parseGitHubIssueOrPRNumber('https://github.com/o/r/issues/923/comments#issuecomment-1') + ).toBe(923) + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/pull/1965//changes///')).toBe(1965) + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/issues/923///')).toBe(923) + }) + + it('rejects invalid GitHub item URLs', () => { expect(parseGitHubIssueOrPRNumber('https://example.com/stablyai/orca/pull/123')).toBeNull() + expect( + parseGitHubIssueOrPRNumber('https://github.example.com/stablyai/orca/pull/123') + ).toBeNull() + expect( + parseGitHubIssueOrPRNumber('https://github.com/o/r/pull/not-a-number/changes') + ).toBeNull() + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/pull/')).toBeNull() + expect(parseGitHubIssueOrPRNumber('https://github.com/o/r/issues/123abc')).toBeNull() + expect(parseGitHubIssueOrPRNumber('https://github.com/owner/repo/pulls/123')).toBeNull() + }) +}) + +describe('parseGitHubIssueOrPRLink', () => { + it('parses slug, number, and type for direct item URLs', () => { + expect(parseGitHubIssueOrPRLink('https://github.com/stablyai/orca/pull/123')).toEqual({ + slug: { owner: 'stablyai', repo: 'orca' }, + number: 123, + type: 'pr' + }) + expect(parseGitHubIssueOrPRLink('https://github.com/stablyai/orca/issues/923')).toEqual({ + slug: { owner: 'stablyai', repo: 'orca' }, + number: 923, + type: 'issue' + }) + }) + + it('derives item type from the route segment when trailing segments are present', () => { + expect(parseGitHubIssueOrPRLink('https://github.com/o/r/pull/1965/changes')).toEqual({ + slug: { owner: 'o', repo: 'r' }, + number: 1965, + type: 'pr' + }) + expect(parseGitHubIssueOrPRLink('https://github.com/o/r/issues/923/comments')).toEqual({ + slug: { owner: 'o', repo: 'r' }, + number: 923, + type: 'issue' + }) + }) + + it('accepts query, fragment, and repeated trailing slashes', () => { + expect(parseGitHubIssueOrPRLink('https://github.com/o/r/pull/1965/files?plain=1#diff')).toEqual( + { + slug: { owner: 'o', repo: 'r' }, + number: 1965, + type: 'pr' + } + ) + expect(parseGitHubIssueOrPRLink('https://github.com/o/r/issues/923/comments///')).toEqual({ + slug: { owner: 'o', repo: 'r' }, + number: 923, + type: 'issue' + }) + }) + + it('rejects non-GitHub and malformed item URLs', () => { + expect(parseGitHubIssueOrPRLink('https://example.com/o/r/pull/1965/changes')).toBeNull() + expect(parseGitHubIssueOrPRLink('https://github.com/o/r/pull/not-a-number/changes')).toBeNull() + expect(parseGitHubIssueOrPRLink('https://github.com/o/r/pull/')).toBeNull() + expect(parseGitHubIssueOrPRLink('https://github.com/o/r/issues/123abc')).toBeNull() + expect(parseGitHubIssueOrPRLink('https://github.com/owner/repo/pulls/123')).toBeNull() }) }) diff --git a/src/renderer/src/lib/github-links.ts b/src/renderer/src/lib/github-links.ts index d87011718..b61fac8aa 100644 --- a/src/renderer/src/lib/github-links.ts +++ b/src/renderer/src/lib/github-links.ts @@ -1,5 +1,4 @@ -const GH_ITEM_PATH_RE = /^\/[^/]+\/[^/]+\/(?:issues|pull)\/(\d+)(?:\/)?$/i -const GH_ITEM_PATH_FULL_RE = /^\/([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)(?:\/)?$/i +const GH_ITEM_PATH_RE = /^\/([^/]+)\/([^/]+)\/(issues|pull)\/(\d+)(?:\/.*)?$/i export type RepoSlug = { owner: string @@ -11,6 +10,10 @@ export type GitHubLinkQuery = { directNumber: number | null } +function matchGitHubItemPath(url: URL): RegExpExecArray | null { + return GH_ITEM_PATH_RE.exec(url.pathname.replace(/\/+$/, '')) +} + /** * Parses a GitHub issue/PR reference from plain input. * Supports issue/PR numbers (e.g. "42"), "#42", and full GitHub URLs. @@ -37,12 +40,12 @@ export function parseGitHubIssueOrPRNumber(input: string): number | null { return null } - const match = GH_ITEM_PATH_RE.exec(url.pathname) + const match = matchGitHubItemPath(url) if (!match) { return null } - return Number.parseInt(match[1], 10) + return Number.parseInt(match[4], 10) } /** @@ -70,15 +73,15 @@ export function parseGitHubIssueOrPRLink(input: string): { return null } - const match = GH_ITEM_PATH_FULL_RE.exec(url.pathname) + const match = matchGitHubItemPath(url) if (!match) { return null } return { slug: { owner: match[1], repo: match[2] }, - type: url.pathname.toLowerCase().includes('/pull/') ? 'pr' : 'issue', - number: Number.parseInt(match[3], 10) + type: match[3].toLowerCase() === 'pull' ? 'pr' : 'issue', + number: Number.parseInt(match[4], 10) } }