diff --git a/mobile/src/components/pr-sidebar/PRCommentCard.tsx b/mobile/src/components/pr-sidebar/PRCommentCard.tsx index 4f5d186c7e4..ce1ef33cb96 100644 --- a/mobile/src/components/pr-sidebar/PRCommentCard.tsx +++ b/mobile/src/components/pr-sidebar/PRCommentCard.tsx @@ -10,7 +10,7 @@ import { PRCommentComposer } from './PRCommentComposer' import { formatPrCommentRelativeTime } from './pr-comment-time' import { prCommentsStyles as styles } from './pr-comments-styles' -export type PRCommentRepoSlug = { owner: string; repo: string } +export type PRCommentRepoSlug = { owner: string; repo: string; host?: string } // Action handlers are passed from the comment actions hook (stable callbacks), so // adding them keeps the memo'd card from re-rendering on unrelated timeline changes. diff --git a/mobile/src/session/github-pr-mutations.ts b/mobile/src/session/github-pr-mutations.ts index dff1775f34d..6d615150ad3 100644 --- a/mobile/src/session/github-pr-mutations.ts +++ b/mobile/src/session/github-pr-mutations.ts @@ -292,11 +292,10 @@ export async function fetchResolveReviewThread( // GitHubProjectMutationResult `{ ok }` envelope sendGithubPrMutation reads. export async function fetchUpdateIssueComment( client: Pick, - args: { owner: string; repo: string; commentId: number; body: string } + args: { owner: string; repo: string; host?: string; commentId: number; body: string } ): Promise { return sendGithubPrMutation(client, 'github.project.updateIssueCommentBySlug', { - owner: args.owner, - repo: args.repo, + ...githubPrRepoSlugParam(args), commentId: args.commentId, body: args.body }) @@ -305,11 +304,10 @@ export async function fetchUpdateIssueComment( // Delete a root conversation (issue) comment. Slug-addressed like the edit wrapper. export async function fetchDeleteIssueComment( client: Pick, - args: { owner: string; repo: string; commentId: number } + args: { owner: string; repo: string; host?: string; commentId: number } ): Promise { return sendGithubPrMutation(client, 'github.project.deleteIssueCommentBySlug', { - owner: args.owner, - repo: args.repo, + ...githubPrRepoSlugParam(args), commentId: args.commentId }) } diff --git a/mobile/src/session/github-pr-rpc.ts b/mobile/src/session/github-pr-rpc.ts index e24c50e8dfc..46d6ec2b6a7 100644 --- a/mobile/src/session/github-pr-rpc.ts +++ b/mobile/src/session/github-pr-rpc.ts @@ -113,7 +113,10 @@ export async function fetchGithubRepoSlug( const record = value as Record const owner = record.owner const repo = record.repo - return typeof owner === 'string' && typeof repo === 'string' ? { owner, repo } : null + const host = record.host + return typeof owner === 'string' && typeof repo === 'string' + ? { owner, repo, ...(typeof host === 'string' && host ? { host } : {}) } + : null } ) } diff --git a/mobile/src/session/pr-comment-actions.ts b/mobile/src/session/pr-comment-actions.ts index 195d2480d87..012500c62da 100644 --- a/mobile/src/session/pr-comment-actions.ts +++ b/mobile/src/session/pr-comment-actions.ts @@ -1,4 +1,4 @@ -import type { PRComment, PRState } from '../../../src/shared/types' +import type { GitHubOwnerRepo, PRComment, PRState } from '../../../src/shared/types' // Pure helpers for the interactive PR comment timeline (reply / resolve / add // root comment). Kept free of React/native imports so they unit-test under the @@ -87,35 +87,46 @@ export function isMutablePRConversationComment( // can hide the affordance rather than firing a doomed request. export function canEditComment( comment: Pick, - prRepo: { owner: string; repo: string } | null | undefined + prRepo: GitHubOwnerRepo | null | undefined ): boolean { return Boolean(prRepo) && isMutablePRConversationComment(comment) } export function canDeleteComment( comment: Pick, - prRepo: { owner: string; repo: string } | null | undefined + prRepo: GitHubOwnerRepo | null | undefined ): boolean { return Boolean(prRepo) && isMutablePRConversationComment(comment) } -export type EditCommentParams = { owner: string; repo: string; commentId: number; body: string } +export type EditCommentParams = GitHubOwnerRepo & { commentId: number; body: string } export function buildEditCommentParams( - prRepo: { owner: string; repo: string }, + prRepo: GitHubOwnerRepo, commentId: number, body: string ): EditCommentParams { - return { owner: prRepo.owner, repo: prRepo.repo, commentId, body } + return { + owner: prRepo.owner, + repo: prRepo.repo, + ...(prRepo.host ? { host: prRepo.host } : {}), + commentId, + body + } } -export type DeleteCommentParams = { owner: string; repo: string; commentId: number } +export type DeleteCommentParams = GitHubOwnerRepo & { commentId: number } export function buildDeleteCommentParams( - prRepo: { owner: string; repo: string }, + prRepo: GitHubOwnerRepo, commentId: number ): DeleteCommentParams { - return { owner: prRepo.owner, repo: prRepo.repo, commentId } + return { + owner: prRepo.owner, + repo: prRepo.repo, + ...(prRepo.host ? { host: prRepo.host } : {}), + commentId + } } // The composer disables submit on empty/whitespace input (host rejects empty body). diff --git a/mobile/src/session/use-mobile-pr-comment-actions.ts b/mobile/src/session/use-mobile-pr-comment-actions.ts index f0909b9e03a..b1eb51ad6ed 100644 --- a/mobile/src/session/use-mobile-pr-comment-actions.ts +++ b/mobile/src/session/use-mobile-pr-comment-actions.ts @@ -39,12 +39,14 @@ export type PrCommentMutations = { editComment: (args: { owner: string repo: string + host?: string commentId: number body: string }) => Promise deleteComment: (args: { owner: string repo: string + host?: string commentId: number }) => Promise } diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 4edf692fa17..860e499d7cf 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -3625,7 +3625,9 @@ describe('getPRForBranch', () => { await expect(getRepoUpstream('/repo-root')).resolves.toBeNull() expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - ['repo', 'view', 'StablyAI/Orca', '--json', 'isFork,parent'], + // Why: positional slugs are explicit about github.com too, so GH_HOST + // cannot redirect them. + ['repo', 'view', 'github.com/StablyAI/Orca', '--json', 'isFork,parent'], { cwd: '/repo-root', host: 'github.com', timeout: 10_000 } ) }) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 1f3d5d10b92..c73230f323f 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -1477,11 +1477,8 @@ async function countWorkItemsForQuery( } function sameOwnerRepo(left: OwnerRepo | null, right: OwnerRepo | null): boolean { - // Why: GitHub owner/repo names are case-insensitive, so differently-cased remotes are the same repo (don't split into two searches). - return ( - left?.owner.toLowerCase() === right?.owner.toLowerCase() && - left?.repo.toLowerCase() === right?.repo.toLowerCase() - ) + // Why: casing does not distinguish GitHub repos, but the same slug on different hosts does. + return Boolean(left && right && githubRepoIdentityKey(left) === githubRepoIdentityKey(right)) } function defaultOpenWorkItemQuery(): ParsedTaskQuery { @@ -2911,6 +2908,11 @@ export async function getPRForBranchOutcome( connectionId, localGitOptions ) + // Why: connection-backed gh runs without a repository cwd. A bare lookup + // here can honor process GH_REPO/GH_HOST and return an unrelated PR. + if (connectionId && candidates.length === 0) { + return { kind: 'no-pr', fetchedAt: Date.now() } + } let data: PullRequestLookupData | null = null let dataRepo: OwnerRepo | null = null let dataHeadRepo: OwnerRepo | null = headRepo diff --git a/src/main/github/github-api-repository.ts b/src/main/github/github-api-repository.ts index 83a1ce0f716..046001168e6 100644 --- a/src/main/github/github-api-repository.ts +++ b/src/main/github/github-api-repository.ts @@ -15,6 +15,7 @@ export type GitHubApiRepository = GitHubOwnerRepo // round trip on connection-backed repos) — hot paths like per-file contents // and viewed-state toggles resolve per call, so cache like ownerRepoCache does. const ORIGIN_REPO_CACHE_TTL_MS = 30_000 +const ORIGIN_REPO_CACHE_MAX_ENTRIES = 512 const originRepoCache = new Map() const originRepoInFlight = new Map>() @@ -33,6 +34,26 @@ export function _resetOriginGitHubApiRepositoryCache(): void { originRepoInFlight.clear() } +/** @internal - exposed for cache-bound tests only */ +export function _getOriginGitHubApiRepositoryCacheSize(): number { + return originRepoCache.size +} + +function pruneOriginRepoCache(now: number): void { + for (const [key, entry] of originRepoCache) { + if (entry.expiresAt <= now) { + originRepoCache.delete(key) + } + } + while (originRepoCache.size > ORIGIN_REPO_CACHE_MAX_ENTRIES) { + const oldestKey = originRepoCache.keys().next().value + if (oldestKey === undefined) { + return + } + originRepoCache.delete(oldestKey) + } +} + /** * Host-qualified repository identity for one remote: github.com remotes come * from the cached slug parser; any other GitHub-shaped host is auth-gated so a @@ -56,8 +77,10 @@ export async function getGitHubApiRepositoryForRemote( return { ...ownerRepo, host: 'github.com' } } const cacheKey = originRepoCacheKey(repoPath, remoteName, connectionId, localGitOptions) + const now = Date.now() + pruneOriginRepoCache(now) const cached = originRepoCache.get(cacheKey) - if (cached && cached.expiresAt > Date.now()) { + if (cached && cached.expiresAt > now) { return cached.value } const inFlight = originRepoInFlight.get(cacheKey) @@ -77,6 +100,7 @@ export async function getGitHubApiRepositoryForRemote( enterpriseOptions ) originRepoCache.set(cacheKey, { value: slug, expiresAt: Date.now() + ORIGIN_REPO_CACHE_TTL_MS }) + pruneOriginRepoCache(Date.now()) return slug })() originRepoInFlight.set(cacheKey, probe) @@ -241,11 +265,11 @@ export function githubRepositoryWebHost(repository: GitHubApiRepository): string /** * Positional `HOST/OWNER/REPO` argv value (e.g. `gh repo view `). * Positional slugs bypass the runner's `--repo` qualifier, so they must be - * qualified here; github.com stays bare for compatibility with gh's default. + * qualified here whenever the host is known. */ export function githubRepositorySlugArg(repository: GitHubApiRepository): string { const slug = `${repository.owner}/${repository.repo}` - return repository.host && !isDefaultGitHubHost(repository.host) - ? `${repository.host}/${slug}` - : slug + // Why: github.com must be explicit too; otherwise process-level GH_HOST can + // redirect positional `gh repo view OWNER/REPO` calls to an Enterprise host. + return repository.host ? `${repository.host}/${slug}` : slug } diff --git a/src/main/github/github-enterprise-repository.ts b/src/main/github/github-enterprise-repository.ts index 1986ce11afb..bd832a229e8 100644 --- a/src/main/github/github-enterprise-repository.ts +++ b/src/main/github/github-enterprise-repository.ts @@ -20,6 +20,7 @@ export type GitHubEnterpriseRepoSlug = GitHubOwnerRepo & { host: string } // mirrors the `glab auth status` signal GitLab self-hosted detection uses, so a // GHES remote is not left to fall through to Gitea (#8312). const HOST_AUTH_TTL_MS = 60_000 +const HOST_AUTH_CACHE_MAX_ENTRIES = 512 type HostAuthCacheEntry = { authenticated: boolean @@ -43,6 +44,26 @@ export function _resetGitHubHostAuthCache(): void { hostAuthInFlight.clear() } +/** @internal - exposed for cache-bound tests only */ +export function _getGitHubHostAuthCacheSize(): number { + return hostAuthCache.size +} + +function pruneHostAuthCache(now: number): void { + for (const [key, entry] of hostAuthCache) { + if (entry.expiresAt <= now) { + hostAuthCache.delete(key) + } + } + while (hostAuthCache.size > HOST_AUTH_CACHE_MAX_ENTRIES) { + const oldestKey = hostAuthCache.keys().next().value + if (oldestKey === undefined) { + return + } + hostAuthCache.delete(oldestKey) + } +} + // Only gh's own stdout/stderr — not the Error.message — counts as an // authoritative answer. A spawn failure (gh missing, ENOENT) carries just a // message and no command output, and must stay indeterminate rather than be @@ -72,6 +93,7 @@ export async function isGitHubHostAuthenticated( const normalizedHost = host.toLowerCase() const cacheKey = `${runtimeCacheKey(connectionId, localGitOptions.wslDistro)}\0${normalizedHost}` const now = Date.now() + pruneHostAuthCache(now) const cached = hostAuthCache.get(cacheKey) if (cached && cached.expiresAt > now) { return cached.authenticated @@ -107,6 +129,7 @@ export async function isGitHubHostAuthenticated( ) } hostAuthCache.set(cacheKey, { authenticated, expiresAt: Date.now() + HOST_AUTH_TTL_MS }) + pruneHostAuthCache(Date.now()) return authenticated })() hostAuthInFlight.set(cacheKey, probe) diff --git a/src/main/github/github-remote-identity-parsing.ts b/src/main/github/github-remote-identity-parsing.ts index e7bd21242f1..e04683dd931 100644 --- a/src/main/github/github-remote-identity-parsing.ts +++ b/src/main/github/github-remote-identity-parsing.ts @@ -5,7 +5,9 @@ export type GitHubRemoteIdentity = GitHubOwnerRepo & { host: string } function normalizeGitHubRemoteHost(host: string): string { const normalizedHost = host.toLowerCase() // Why: GitHub documents ssh.github.com:443 as SSH-over-HTTPS for github.com repos. - return normalizedHost === 'ssh.github.com' ? 'github.com' : normalizedHost + return normalizedHost === 'ssh.github.com' || normalizedHost === 'ssh.github.com:443' + ? 'github.com' + : normalizedHost } function parseGitHubRemotePath(path: string): Pick | null { @@ -34,7 +36,9 @@ export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdenti return null } const path = parseGitHubRemotePath(url.pathname) - return path ? { host: normalizeGitHubRemoteHost(url.hostname), ...path } : null + // Why: GHES may be served on a non-default port. `hostname` silently drops + // it, sending authenticated API calls to a different endpoint on port 443. + return path ? { host: normalizeGitHubRemoteHost(url.host), ...path } : null } catch { return null } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 5479cbd7805..de4901970cf 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1520,6 +1520,7 @@ export type PreloadApi = { repoId?: string owner: string repo: string + host?: string number: number type: 'issue' | 'pr' }) => Promise | null> @@ -1715,8 +1716,8 @@ export type PreloadApi = { * `force: true` to bust after a known-expensive op. */ rateLimit: (args?: { force?: boolean }) => Promise - /** Explains scope_missing ProjectV2 failures — notably a shell `GITHUB_TOKEN` shadowing the keyring credential, where `gh auth refresh` is a no-op. */ - diagnoseAuth: (args?: { host?: string }) => Promise + /** Explains scope_missing ProjectV2 failures — notably a shell `GITHUB_TOKEN` shadowing the keyring credential, where `gh auth refresh` is a no-op. */ + diagnoseAuth: (args?: { host?: string }) => Promise // ── ProjectV2 (GitHub Projects) ───────────────────────────────── listAccessibleProjects: ( args?: ListAccessibleProjectsArgs diff --git a/src/renderer/src/components/new-workspace/SmartWorkspaceNameField.tsx b/src/renderer/src/components/new-workspace/SmartWorkspaceNameField.tsx index f2749139eac..963c32143a2 100644 --- a/src/renderer/src/components/new-workspace/SmartWorkspaceNameField.tsx +++ b/src/renderer/src/components/new-workspace/SmartWorkspaceNameField.tsx @@ -81,6 +81,7 @@ import { type TaskSourceContext } from '../../../../shared/task-source-context' import { parseExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' +import { githubRepoIdentityKey } from '../../../../shared/github-repository-identity-key' type RepoOption = ReturnType['repos'][number] const EMPTY_REPO_SEARCH_REPOS: readonly RepoOption[] = [] @@ -577,6 +578,7 @@ export default function SmartWorkspaceNameField({ sourceContext: target.githubSourceContext, owner: directLink.slug.owner, repo: directLink.slug.repo, + ...(directLink.slug.host ? { host: directLink.slug.host } : {}), number: directLink.number, type: directLink.type }) @@ -599,6 +601,7 @@ export default function SmartWorkspaceNameField({ kind: 'link', owner: directLink.slug.owner, repo: directLink.slug.repo, + ...(directLink.slug.host ? { host: directLink.slug.host } : {}), number: directLink.number, type: directLink.type }, @@ -647,6 +650,7 @@ export default function SmartWorkspaceNameField({ kind: 'link' as const, owner: directLink.slug.owner, repo: directLink.slug.repo, + ...(directLink.slug.host ? { host: directLink.slug.host } : {}), number: directLink.number, type: directLink.type } @@ -1128,6 +1132,7 @@ export default function SmartWorkspaceNameField({ sourceContext, owner: crossRepoPrompt.link.slug.owner, repo: crossRepoPrompt.link.slug.repo, + ...(crossRepoPrompt.link.slug.host ? { host: crossRepoPrompt.link.slug.host } : {}), number: crossRepoPrompt.link.number, type: crossRepoPrompt.link.type }) @@ -1748,10 +1753,7 @@ function RowLabel({ row }: { row: RowEntry }): React.JSX.Element { } function sameSlug(left: RepoSlug, right: RepoSlug): boolean { - return ( - left.owner.toLowerCase() === right.owner.toLowerCase() && - left.repo.toLowerCase() === right.repo.toLowerCase() - ) + return githubRepoIdentityKey(left) === githubRepoIdentityKey(right) } async function getRepoSlugCached( diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index accc351ca50..6e617d6d644 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -1890,6 +1890,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS sourceContext: selectedRepoGitHubSourceContext, owner: normalizedLinkQuery.directLink.slug.owner, repo: normalizedLinkQuery.directLink.slug.repo, + ...(normalizedLinkQuery.directLink.slug.host + ? { host: normalizedLinkQuery.directLink.slug.host } + : {}), number: normalizedLinkQuery.directLink.number, type: normalizedLinkQuery.directLink.type }) diff --git a/src/renderer/src/lib/github-work-item-source-lookup.ts b/src/renderer/src/lib/github-work-item-source-lookup.ts index e12de08b3b0..046c4af0790 100644 --- a/src/renderer/src/lib/github-work-item-source-lookup.ts +++ b/src/renderer/src/lib/github-work-item-source-lookup.ts @@ -18,6 +18,7 @@ type GitHubWorkItemLookupArgs = { type GitHubWorkItemByOwnerRepoLookupArgs = GitHubWorkItemLookupArgs & { owner: string repo: string + host?: string type: 'issue' | 'pr' } @@ -71,6 +72,7 @@ export async function lookupGitHubWorkItemByOwnerRepoForSource( repo: runtimeRepoId(args), owner: args.owner, ownerRepo: args.repo, + ...(args.host ? { host: args.host } : {}), number: args.number, type: args.type }, @@ -81,6 +83,7 @@ export async function lookupGitHubWorkItemByOwnerRepoForSource( repoId: args.repoId, owner: args.owner, repo: args.repo, + ...(args.host ? { host: args.host } : {}), number: args.number, type: args.type }) diff --git a/src/renderer/src/lib/smart-github-submit.ts b/src/renderer/src/lib/smart-github-submit.ts index 1b4e818f26a..748c84161fe 100644 --- a/src/renderer/src/lib/smart-github-submit.ts +++ b/src/renderer/src/lib/smart-github-submit.ts @@ -5,12 +5,14 @@ import { getLinkedWorkItemWorkspaceName } from '../../../shared/workspace-name' import type { LinkedWorkItemSummary } from './new-workspace' import { parseGitHubIssueOrPRLink } from './github-links' import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' +import { githubRepoIdentityKey } from '../../../shared/github-repository-identity-key' export type SmartGitHubSubmitIntent = | { kind: 'link' owner: string repo: string + host?: string number: number type: 'issue' | 'pr' } @@ -48,6 +50,7 @@ export type SmartGitHubSubmitLookup = { sourceContext?: TaskSourceContext | null owner: string repo: string + host?: string number: number type: 'issue' | 'pr' }) => Promise @@ -92,6 +95,7 @@ export function getSmartGitHubSubmitIntent(input: string): SmartGitHubSubmitInte kind: 'link', owner: link.slug.owner, repo: link.slug.repo, + ...(link.slug.host ? { host: link.slug.host } : {}), number: link.number, type: link.type } @@ -132,9 +136,7 @@ function getSmartGitHubSubmitLookupCacheKey({ if (intent.kind === 'hash-number') { return `${repoScope}:hash:${intent.number}` } - return `${repoScope}:link:${intent.owner.toLowerCase()}/${intent.repo.toLowerCase()}:${ - intent.type - }:${intent.number}` + return `${repoScope}:link:${githubRepoIdentityKey(intent)}:${intent.type}:${intent.number}` } export function lookupSmartGitHubSubmitItem({ @@ -161,6 +163,7 @@ export function lookupSmartGitHubSubmitItem({ sourceContext, owner: intent.owner, repo: intent.repo, + ...(intent.host ? { host: intent.host } : {}), number: intent.number, type: intent.type }) diff --git a/src/shared/github-links.ts b/src/shared/github-links.ts index 8f122ba4ac8..ee82d811db6 100644 --- a/src/shared/github-links.ts +++ b/src/shared/github-links.ts @@ -101,7 +101,9 @@ export function parseGitHubIssueOrPRLink(input: string): GitHubIssueOrPRLink | n return { // Why: the URL proves the host — GHES links must keep their server identity // instead of being treated as github.com slugs. - slug: { owner: match[1], repo: match[2], host: url.hostname }, + // Why: GHES installations on non-default ports require the full URL host; + // `hostname` would silently redirect later API calls to port 443. + slug: { owner: match[1], repo: match[2], host: url.host }, type: match[3].toLowerCase() === 'pull' ? 'pr' : 'issue', number } diff --git a/src/shared/terminal-github-pr-link-detector.test.ts b/src/shared/terminal-github-pr-link-detector.test.ts index 23a00fe493d..c2a6cc3c5bf 100644 --- a/src/shared/terminal-github-pr-link-detector.test.ts +++ b/src/shared/terminal-github-pr-link-detector.test.ts @@ -156,8 +156,8 @@ describe('createTerminalGitHubPRLinkDetector', () => { expect(observe('Created https://github.internal:8443/MyOrg/my_repo/pull/397\r\n')).toEqual([ { url: 'https://github.internal:8443/MyOrg/my_repo/pull/397', - // Ports are dropped to match parseGitHubRemoteIdentity's hostname identity. - slug: { owner: 'MyOrg', repo: 'my_repo', host: 'github.internal' }, + // Why: GHES on a non-default port needs the port in its host identity. + slug: { owner: 'MyOrg', repo: 'my_repo', host: 'github.internal:8443' }, number: 397 } ])