Bound GHES identity caches and preserve non-default ports in host identity

Cap the origin-repo and host-auth caches like ownerRepoCache; keep ports
from remote/link URLs so GHES on a non-default port is a distinct
identity; make positional github.com slugs explicit against GH_HOST;
compare work-item sources by host-aware identity key; bail cwd-less
branch lookups when no repository candidate resolved; thread host
through the renderer work-item slug lookup.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing
2026-07-20 12:35:25 -07:00
co-authored by Orca
parent bb6998097a
commit f8078448f2
17 changed files with 125 additions and 42 deletions
@@ -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.
+4 -6
View File
@@ -292,11 +292,10 @@ export async function fetchResolveReviewThread(
// GitHubProjectMutationResult `{ ok }` envelope sendGithubPrMutation reads.
export async function fetchUpdateIssueComment(
client: Pick<RpcClient, 'sendRequest'>,
args: { owner: string; repo: string; commentId: number; body: string }
args: { owner: string; repo: string; host?: string; commentId: number; body: string }
): Promise<GitHubPrMutationOutcome> {
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<RpcClient, 'sendRequest'>,
args: { owner: string; repo: string; commentId: number }
args: { owner: string; repo: string; host?: string; commentId: number }
): Promise<GitHubPrMutationOutcome> {
return sendGithubPrMutation(client, 'github.project.deleteIssueCommentBySlug', {
owner: args.owner,
repo: args.repo,
...githubPrRepoSlugParam(args),
commentId: args.commentId
})
}
+4 -1
View File
@@ -113,7 +113,10 @@ export async function fetchGithubRepoSlug(
const record = value as Record<string, unknown>
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
}
)
}
+20 -9
View File
@@ -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<PRComment, 'id' | 'threadId' | 'path' | 'url'>,
prRepo: { owner: string; repo: string } | null | undefined
prRepo: GitHubOwnerRepo | null | undefined
): boolean {
return Boolean(prRepo) && isMutablePRConversationComment(comment)
}
export function canDeleteComment(
comment: Pick<PRComment, 'id' | 'threadId' | 'path' | 'url'>,
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).
@@ -39,12 +39,14 @@ export type PrCommentMutations = {
editComment: (args: {
owner: string
repo: string
host?: string
commentId: number
body: string
}) => Promise<GitHubPrMutationOutcome>
deleteComment: (args: {
owner: string
repo: string
host?: string
commentId: number
}) => Promise<GitHubPrMutationOutcome>
}
+3 -1
View File
@@ -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 }
)
})
+7 -5
View File
@@ -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
+29 -5
View File
@@ -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<string, { value: GitHubApiRepository | null; expiresAt: number }>()
const originRepoInFlight = new Map<string, Promise<GitHubApiRepository | null>>()
@@ -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 <slug>`).
* 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
}
@@ -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)
@@ -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<GitHubRemoteIdentity, 'owner' | 'repo'> | 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
}
+3 -2
View File
@@ -1520,6 +1520,7 @@ export type PreloadApi = {
repoId?: string
owner: string
repo: string
host?: string
number: number
type: 'issue' | 'pr'
}) => Promise<Omit<GitHubWorkItem, 'repoId'> | null>
@@ -1715,8 +1716,8 @@ export type PreloadApi = {
* `force: true` to bust after a known-expensive op.
*/
rateLimit: (args?: { force?: boolean }) => Promise<GetRateLimitResult>
/** 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<GhAuthDiagnostic>
/** 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<GhAuthDiagnostic>
// ── ProjectV2 (GitHub Projects) ─────────────────────────────────
listAccessibleProjects: (
args?: ListAccessibleProjectsArgs
@@ -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<typeof useAppStore.getState>['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(
@@ -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
})
@@ -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
})
+6 -3
View File
@@ -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<GitHubWorkItem | null>
@@ -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
})
+3 -1
View File
@@ -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
}
@@ -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
}
])