diff --git a/src/main/git/gh-rate-limit-breaker.test.ts b/src/main/git/gh-rate-limit-breaker.test.ts index 3e136954522..787ad3d1777 100644 --- a/src/main/git/gh-rate-limit-breaker.test.ts +++ b/src/main/git/gh-rate-limit-breaker.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { + _getGhRateLimitBlockCount, _resetGhRateLimitBreaker, classifyGhRateLimitBucket, createGhRateLimitBlockedError, @@ -117,6 +118,19 @@ describe('breaker state', () => { expect(getGhRateLimitBlockedUntilMs('core', now, 'wsl:ubuntu:github.com')).toBeNull() }) + it('bounds retained blocks across user-supplied Enterprise host scopes', () => { + const now = Date.now() + for (let i = 0; i < 1_200; i += 1) { + recordGhPrimaryRateLimit('core', now + 60_000, `native:ghe-${i}.example.test`) + } + + expect(_getGhRateLimitBlockCount()).toBe(1024) + expect(getGhRateLimitBlockedUntilMs('core', now, 'native:ghe-1199.example.test')).toBe( + now + 60_000 + ) + expect(getGhRateLimitBlockedUntilMs('core', now, 'native:ghe-0.example.test')).toBeNull() + }) + it('notifyGhPrimaryRateLimit applies a fallback block and fires the reset probe', () => { const probe = vi.fn() registerGhRateLimitResetProbe(probe) diff --git a/src/main/git/gh-rate-limit-breaker.ts b/src/main/git/gh-rate-limit-breaker.ts index 623555d8133..6da81bab879 100644 --- a/src/main/git/gh-rate-limit-breaker.ts +++ b/src/main/git/gh-rate-limit-breaker.ts @@ -57,6 +57,7 @@ export function parseGhRateLimitScopeKey(scope: string): GhRateLimitScopeParts | } const DEFAULT_SCOPE = ghRateLimitScopeKey('native', 'github.com') +const GH_RATE_LIMIT_BLOCK_MAX_ENTRIES = 1024 const blockedUntilMsByScopeAndBucket = new Map() let resetProbe: ((bucket: GhRateLimitBucket, scope: string) => void) | null = null @@ -146,7 +147,18 @@ export function recordGhPrimaryRateLimit( ): void { const key = breakerKey(bucket, scope) const existing = blockedUntilMsByScopeAndBucket.get(key) ?? 0 + // Why: host scopes can originate from pasted GHES URLs. Refresh insertion + // order and cap retained blocks so a stream of failing hosts cannot grow the + // main-process map for the lifetime of the app. + blockedUntilMsByScopeAndBucket.delete(key) blockedUntilMsByScopeAndBucket.set(key, Math.max(existing, blockedUntilMs)) + while (blockedUntilMsByScopeAndBucket.size > GH_RATE_LIMIT_BLOCK_MAX_ENTRIES) { + const oldestKey = blockedUntilMsByScopeAndBucket.keys().next().value + if (oldestKey === undefined) { + break + } + blockedUntilMsByScopeAndBucket.delete(oldestKey) + } } export function clearGhRateLimitBlock(bucket: GhRateLimitBucket, scope = DEFAULT_SCOPE): void { @@ -158,6 +170,11 @@ export function getGhRateLimitBlockedUntilMs( nowMs: number = Date.now(), scope = DEFAULT_SCOPE ): number | null { + for (const [key, blockedUntil] of blockedUntilMsByScopeAndBucket) { + if (blockedUntil <= nowMs) { + blockedUntilMsByScopeAndBucket.delete(key) + } + } const key = breakerKey(bucket, scope) const blockedUntil = blockedUntilMsByScopeAndBucket.get(key) if (blockedUntil === undefined) { @@ -170,6 +187,11 @@ export function getGhRateLimitBlockedUntilMs( return blockedUntil } +/** @internal — cache-bound tests only. */ +export function _getGhRateLimitBlockCount(): number { + return blockedUntilMsByScopeAndBucket.size +} + /** Register the (single) callback that refreshes precise reset times after a * breaker trip. Registered by the github rate-limit prober at module load. */ export function registerGhRateLimitResetProbe( diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 50617955945..ed643c17ecd 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -2486,10 +2486,6 @@ function parseTrackedUpstreamBranch(upstreamRef: string): TrackedUpstreamBranch return parsed } -function prOwnerRepoKey(ownerRepo: OwnerRepo): string { - return `${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}` -} - function shouldRetryTrackedUpstreamBranch( upstreamBranch: TrackedUpstreamBranch, branchName: string, @@ -2502,7 +2498,7 @@ function shouldRetryTrackedUpstreamBranch( if (!headRepo) { return true } - return prOwnerRepoKey(upstreamHeadRepo) !== prOwnerRepoKey(headRepo) + return githubRepoIdentityKey(upstreamHeadRepo) !== githubRepoIdentityKey(headRepo) } async function getTrackedUpstreamBranch( diff --git a/src/main/github/github-repository-identity.ts b/src/main/github/github-repository-identity.ts index 28b7b0676c7..1996d7b2b54 100644 --- a/src/main/github/github-repository-identity.ts +++ b/src/main/github/github-repository-identity.ts @@ -8,6 +8,7 @@ import { type GitHubRemoteIdentity } from './github-remote-identity-parsing' import { isStableMissingGitRemoteError } from './stable-missing-git-remote-error' +import { githubRepoIdentityKey } from '../../shared/github-repository-identity-key' export type OwnerRepo = GitHubOwnerRepo @@ -234,7 +235,7 @@ export type PRRepositoryCandidates = { } function ownerRepoKey(ownerRepo: OwnerRepo): string { - return `${ownerRepo.owner.toLowerCase()}/${ownerRepo.repo.toLowerCase()}` + return githubRepoIdentityKey(ownerRepo) } export async function resolvePRRepositoryCandidates( diff --git a/src/main/github/rate-limit.ts b/src/main/github/rate-limit.ts index 6691488f90b..2ca3f5cad43 100644 --- a/src/main/github/rate-limit.ts +++ b/src/main/github/rate-limit.ts @@ -195,6 +195,24 @@ const DEFAULT_BREAKER_SCOPE = ghRateLimitScopeKey('native', 'github.com') // versa). const scopeRefinementInFlight = new Map>() const scopeProbeFailureAtMs = new Map() +const SCOPE_PROBE_FAILURE_MAX_ENTRIES = 512 + +function rememberScopeProbeFailure(scope: string, failedAt: number): void { + for (const [key, at] of scopeProbeFailureAtMs) { + if (failedAt - at >= RATE_LIMIT_CACHE_TTL_MS) { + scopeProbeFailureAtMs.delete(key) + } + } + scopeProbeFailureAtMs.delete(scope) + scopeProbeFailureAtMs.set(scope, failedAt) + while (scopeProbeFailureAtMs.size > SCOPE_PROBE_FAILURE_MAX_ENTRIES) { + const oldestKey = scopeProbeFailureAtMs.keys().next().value + if (oldestKey === undefined) { + break + } + scopeProbeFailureAtMs.delete(oldestKey) + } +} function refineBreakerForScope(scope: string): void { // Default scope keeps the existing shared-snapshot refinement path. @@ -241,7 +259,9 @@ function refineBreakerForScope(scope: string): void { release() } } catch { - scopeProbeFailureAtMs.set(scope, Date.now()) + // Why: GHES hosts can be supplied through pasted project URLs. Bound the + // negative cache so repeated failing hosts cannot accumulate forever. + rememberScopeProbeFailure(scope, Date.now()) } finally { scopeRefinementInFlight.delete(scope) } diff --git a/src/renderer/src/components/PullRequestPage.tsx b/src/renderer/src/components/PullRequestPage.tsx index c5a9786cf10..d4c7b5a3b32 100644 --- a/src/renderer/src/components/PullRequestPage.tsx +++ b/src/renderer/src/components/PullRequestPage.tsx @@ -223,20 +223,18 @@ import { import { translate } from '@/i18n/i18n' import { getSettingsForRepoRuntimeOwner } from '@/lib/repo-runtime-owner' -// Why: the item URL is the only owner/repo source present on every work item across the IPC boundary; non-GitHub hosts return null (matching the indicator's suppression). +// Why: the item URL is the only host-aware repository identity present on every work item across IPC. function parseOwnerRepoFromItemUrl(url: string): GitHubOwnerRepo | null { try { const parsed = new URL(url) - if (parsed.hostname !== 'github.com') { + if ((parsed.protocol !== 'https:' && parsed.protocol !== 'http:') || !parsed.host) { return null } const segments = parsed.pathname.split('/').filter(Boolean) if (segments.length < 2) { return null } - // Why: the URL proves the host. Leaving it off lets the main process stamp - // a GHES origin's host onto what is provably a github.com repo. - return { owner: segments[0], repo: segments[1], host: 'github.com' } + return { owner: segments[0], repo: segments[1], host: parsed.host } } catch { return null } @@ -287,6 +285,7 @@ function normalizeItemDialogTab( export type PullRequestPageProjectOrigin = { owner: string repo: string + host?: string number: number type: 'issue' | 'pr' projectId: string @@ -563,7 +562,8 @@ function PRAssigneesPanel({ slugOwner, slugRepo, assigneeLogins, - sourceSettings + sourceSettings, + projectOrigin?.host ?? assigneeSlug?.host ) const repoAssigneesByPath = useRepoAssignees(repoPath, item.repoId, sourceSettings) const repoAssignees = slugOwner && slugRepo ? repoAssigneesBySlug : repoAssigneesByPath @@ -841,7 +841,8 @@ function PRReviewersPanel({ open && reviewSlug ? reviewSlug.owner : null, open && reviewSlug ? reviewSlug.repo : null, reviewerSeedUsers.map((user) => user.login), - sourceSettings + sourceSettings, + reviewSlug?.host ) const reviewerMetadataByPath = useRepoAssignees( open && !reviewSlug ? repoPath : null, @@ -3249,7 +3250,10 @@ function ConversationTab({ const bodySlug = useMemo(() => parseOwnerRepoFromItemUrl(item.url), [item.url]) const markdownGitHubRepo = useMemo( - () => (projectOrigin ? { owner: projectOrigin.owner, repo: projectOrigin.repo } : bodySlug), + () => + projectOrigin + ? { owner: projectOrigin.owner, repo: projectOrigin.repo, host: projectOrigin.host } + : bodySlug, [bodySlug, projectOrigin] ) const canEditBody = @@ -5491,6 +5495,7 @@ async function runIssueUpdate(args: { const updateArgs = { owner: args.projectOrigin.owner, repo: args.projectOrigin.repo, + ...(args.projectOrigin.host ? { host: args.projectOrigin.host } : {}), number: args.number, updates: args.updates } @@ -5571,7 +5576,11 @@ async function runWorkItemBodyUpdate(args: { }): Promise { if (args.item.type === 'pr') { const targetSlug = args.projectOrigin - ? { owner: args.projectOrigin.owner, repo: args.projectOrigin.repo } + ? { + owner: args.projectOrigin.owner, + repo: args.projectOrigin.repo, + host: args.projectOrigin.host + } : args.parsedSlug if (!targetSlug) { throw new Error('No GitHub repository context available for this pull request.') @@ -5584,6 +5593,7 @@ async function runWorkItemBodyUpdate(args: { const updateArgs = { owner: targetSlug.owner, repo: targetSlug.repo, + ...(targetSlug.host ? { host: targetSlug.host } : {}), number: args.item.number, updates: { body: args.body } } @@ -5643,6 +5653,7 @@ async function runPullRequestStateUpdate(args: { const updateArgs = { owner: args.projectOrigin.owner, repo: args.projectOrigin.repo, + ...(args.projectOrigin.host ? { host: args.projectOrigin.host } : {}), number: args.number, updates: args.updates } @@ -5785,14 +5796,25 @@ function GHEditSection({ projectOrigin ? null : repoId, sourceSettings ) - const repoLabelsBySlug = useRepoLabelsBySlug(slugOwner, slugRepo, sourceSettings) + const repoLabelsBySlug = useRepoLabelsBySlug( + slugOwner, + slugRepo, + sourceSettings, + projectOrigin?.host + ) const repoLabels = projectOrigin ? repoLabelsBySlug : repoLabelsByPath const repoAssigneesByPath = useRepoAssignees( projectOrigin ? null : repoPath, projectOrigin ? null : repoId, sourceSettings ) - const repoAssigneesBySlug = useRepoAssigneesBySlug(slugOwner, slugRepo, assignees, sourceSettings) + const repoAssigneesBySlug = useRepoAssigneesBySlug( + slugOwner, + slugRepo, + assignees, + sourceSettings, + projectOrigin?.host + ) const repoAssignees = projectOrigin ? repoAssigneesBySlug : repoAssigneesByPath // Why: sync local assignees on item change / detail resolve, but skip if the user made an optimistic edit so we don't clobber in-flight changes. diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index d0186a1b1a4..5ed37ea6912 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -1180,6 +1180,7 @@ function GHStatusCell({ { owner: parsedOwnerRepo.owner, repo: parsedOwnerRepo.repo, + ...(parsedOwnerRepo.host ? { host: parsedOwnerRepo.host } : {}), number: item.number, updates }, @@ -1188,6 +1189,7 @@ function GHStatusCell({ : window.api.gh.updateIssueBySlug({ owner: parsedOwnerRepo.owner, repo: parsedOwnerRepo.repo, + ...(parsedOwnerRepo.host ? { host: parsedOwnerRepo.host } : {}), number: item.number, updates }) @@ -1795,7 +1797,8 @@ function GHAssigneesCell({ open ? owner : null, open ? repoName : null, seedLogins, - sourceSettings + sourceSettings, + parsed?.slug.host ) const toggleAssignee = useCallback( @@ -1819,6 +1822,7 @@ function GHAssigneesCell({ const args = { owner, repo: repoName, + ...(parsed?.slug.host ? { host: parsed.slug.host } : {}), number: item.number, updates } @@ -2188,7 +2192,8 @@ function PRReviewCell({ open && reviewSlug ? reviewSlug.owner : null, open && reviewSlug ? reviewSlug.repo : null, reviewerSeedUsers.map((user) => user.login), - sourceSettings + sourceSettings, + reviewSlug?.host ) const authorLogin = item.author?.toLowerCase() ?? null diff --git a/src/renderer/src/components/github-enterprise-slug-routing-boundary.test.ts b/src/renderer/src/components/github-enterprise-slug-routing-boundary.test.ts new file mode 100644 index 00000000000..4247fd80d0d --- /dev/null +++ b/src/renderer/src/components/github-enterprise-slug-routing-boundary.test.ts @@ -0,0 +1,54 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +function componentSource(relativePath: string): string { + return readFileSync(join(__dirname, relativePath), 'utf8') +} + +function sourceBetween(source: string, startPattern: string, endPattern: string): string { + const start = source.indexOf(startPattern) + expect(start).toBeGreaterThanOrEqual(0) + const end = source.indexOf(endPattern, start + startPattern.length) + expect(end).toBeGreaterThan(start) + return source.slice(start, end) +} + +describe('GitHub Enterprise slug routing boundaries', () => { + it('keeps work-item URL hosts on TaskPage metadata and issue mutations', () => { + const source = componentSource('TaskPage.tsx') + const statusSection = sourceBetween(source, 'function GHStatusCell', 'function formatPRDelta') + const assigneeSection = sourceBetween( + source, + 'function GHAssigneesCell', + 'function getChecksLabel' + ) + + expect(statusSection).toContain( + '...(parsedOwnerRepo.host ? { host: parsedOwnerRepo.host } : {})' + ) + expect(assigneeSection).toContain('parsed?.slug.host') + expect(assigneeSection).toContain('...(parsed?.slug.host ? { host: parsed.slug.host } : {})') + }) + + it('keeps PR base-repository hosts on checks-sidebar comment writes', () => { + const source = componentSource('right-sidebar/ChecksPanel.tsx') + const conversationSection = sourceBetween( + source, + 'const handleEditComment = useCallback', + 'const handleReplyToComment = useCallback' + ) + + expect(conversationSection).toContain('...(pr.prRepo.host ? { host: pr.prRepo.host } : {})') + expect(conversationSection).toContain('updateIssueCommentBySlug({') + expect(conversationSection).toContain('deleteIssueCommentBySlug({') + }) + + it('keeps the primary repository host on task filter metadata reads', () => { + const source = componentSource('github/PRFilterDropdowns.tsx') + + expect(source).toContain('useRepoLabelsBySlug(') + expect(source).toContain('useRepoAssigneesBySlug(') + expect(source.match(/primarySlug\?\.host/g)).toHaveLength(2) + }) +}) diff --git a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts index 951e22d6651..6851fdb3a2a 100644 --- a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts +++ b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts @@ -57,7 +57,8 @@ describe('GitHubItemDialog source host boundaries', () => { expect(section).toContain('getTaskSourceRuntimeSettings(sourceContext)') expect(section).toContain('useRepoLabels(') - expect(section).toContain('useRepoLabelsBySlug(slugOwner, slugRepo, sourceSettings)') + expect(section).toContain('useRepoLabelsBySlug(') + expect(section).toContain('projectOrigin?.host') expect(section).toContain('useRepoAssignees(') expect(section).toContain('useRepoAssigneesBySlug(') expect(section).toContain('sourceSettings') diff --git a/src/renderer/src/components/github-project/GhAuthErrorHelp.test.ts b/src/renderer/src/components/github-project/GhAuthErrorHelp.test.ts new file mode 100644 index 00000000000..da666d54523 --- /dev/null +++ b/src/renderer/src/components/github-project/GhAuthErrorHelp.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it, vi } from 'vitest' +import type { GhAuthDiagnostic } from '../../../../shared/github-auth-types' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +import { buildRemediation } from './GhAuthErrorHelp' + +function diagnostic(overrides: Partial = {}): GhAuthDiagnostic { + return { + ghAvailable: true, + activeAccount: { + host: 'ghe.acme.test:8443', + user: 'alice', + active: true, + envToken: null, + source: 'keyring', + scopes: ['project', 'read:org', 'repo'] + }, + accounts: [], + envTokenInProcess: null, + missingScopes: [], + requiredScopes: ['project', 'read:org', 'repo'], + hasKeyringFallback: false, + requiredHost: 'ghe.acme.test:8443', + requiredHostAuthenticated: true, + ...overrides + } +} + +describe('GitHub Project auth remediation host routing', () => { + it('uses the requested GHES host while the diagnostic is still loading', () => { + expect( + buildRemediation('Sign in required.', 'auth_required', null, 'ghe.acme.test:8443').commands + ).toEqual([ + { + label: 'Copy command', + command: 'gh auth login --hostname ghe.acme.test:8443' + } + ]) + expect( + buildRemediation('Missing scope.', 'scope_missing', null, 'ghe.acme.test:8443').commands + ).toEqual([ + { + label: 'Copy command', + command: 'gh auth refresh --hostname ghe.acme.test:8443 -s project -s read:org -s repo' + } + ]) + }) + + it('keeps the GHES host on best-effort refresh advice after diagnostics complete', () => { + expect(buildRemediation('Access denied.', 'scope_missing', diagnostic()).commands).toEqual([ + { + label: 'Copy refresh command', + command: 'gh auth refresh --hostname ghe.acme.test:8443 -s project -s read:org -s repo' + } + ]) + }) +}) diff --git a/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx b/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx index 84e0d16876c..30d1ea61b16 100644 --- a/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx +++ b/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx @@ -122,10 +122,11 @@ type Remediation = { docsUrl?: string } -function buildRemediation( +export function buildRemediation( errorMessage: string, kind: AuthErrorKind, - diag: GhAuthDiagnostic | null + diag: GhAuthDiagnostic | null, + requestedHost?: string ): Remediation { // Diagnostic still loading or unavailable — fall back to the canned advice // so the UI never gets worse than the pre-diagnosis behavior. @@ -138,7 +139,10 @@ function buildRemediation( 'auto.components.github.project.GhAuthErrorHelp.b436c586d1', 'Copy command' ), - command: kind === 'auth_required' ? LOGIN_CMD : REFRESH_CMD + command: + kind === 'auth_required' + ? loginCommandForHost(requestedHost) + : refreshCommandForHost(requestedHost) } ] } @@ -173,7 +177,7 @@ function buildRemediation( 'auto.components.github.project.GhAuthErrorHelp.9c2da6353b', 'Copy login command' ), - command: LOGIN_CMD + command: loginCommandForHost(diag.requiredHost ?? requestedHost) } ], docsUrl: 'https://cli.github.com/' @@ -264,7 +268,7 @@ function buildRemediation( 'auto.components.github.project.GhAuthErrorHelp.3fefeebde4', 'Copy refresh command' ), - command: REFRESH_CMD + command: refreshCommandForHost(diag.requiredHost ?? requestedHost) } ], docsUrl: @@ -299,7 +303,7 @@ export function GhAuthErrorHelp({ } }, [host]) - const remedy = buildRemediation(error.message, error.type, diag) + const remedy = buildRemediation(error.message, error.type, diag, host) const docsUrl = remedy.docsUrl if (variant === 'banner') { diff --git a/src/renderer/src/components/github-project/ProjectPicker.tsx b/src/renderer/src/components/github-project/ProjectPicker.tsx index 2bd7d516eca..a4d54b3a71e 100644 --- a/src/renderer/src/components/github-project/ProjectPicker.tsx +++ b/src/renderer/src/components/github-project/ProjectPicker.tsx @@ -814,6 +814,7 @@ function AuthErrorBanner({ error }: { error: GitHubProjectViewError }): React.JS ) } diff --git a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx index 7d7af9bcd95..4e5c6b48a87 100644 --- a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx +++ b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx @@ -906,6 +906,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS { if (selectedViewUrl) { void window.api.shell.openUrl(selectedViewUrl) @@ -1262,10 +1263,12 @@ function ViewTabStrip({ function ErrorState({ error, totalCount, + host, onOpenInGitHub }: { error: GitHubProjectViewError totalCount?: number + host?: string onOpenInGitHub: () => void }): React.JSX.Element { // Auth/scope errors get a richer `gh auth status` remediation UI; bail early before the generic block. @@ -1274,6 +1277,7 @@ function ErrorState({