feat: support GitHub enterprise remotes#2210
Conversation
…motes # Conflicts: # src/renderer/features/projects/stores/repository-store.test.ts
Greptile SummaryThis PR adds GitHub Enterprise Server (GHES) support across the entire GitHub integration stack — remote parsing, host probing, authentication, Octokit creation, PR sync, issue listing, and renderer capability gating — while also replacing the github.com-only
Confidence Score: 5/5Safe to merge — all changed paths degrade gracefully on auth or network failure, and the cross-host PR guard is correctly enforced server-side. The auth caching, host probing, Result propagation, and cross-host guard are all implemented correctly. The two observations are limited to minor message-string duplication and a spurious-but-harmless subprocess call in an infrequently-hit fallback path; neither affects correctness or data integrity. No files require special attention beyond the noted observations in issue-service.ts and the DB-fallback path in pr-sync-scheduler.ts.
|
| Filename | Overview |
|---|---|
| src/shared/repository-ref.ts | New file providing provider-neutral URL parsing. Handles HTTPS, SSH, and SCP formats; preserves HTTPS ports for GHES canonical URLs while stripping SSH transport ports. Tests cover the asymmetry. |
| src/main/core/github/services/github-host-service.ts | New service that probes non-github.com hosts for GHES API compatibility via the /api/v3/meta endpoint. Uses 15-minute positive / 2-minute negative TTL caches and in-flight deduplication. 401/403 responses are correctly treated as GitHub compatible. |
| src/main/core/github/services/ghes-auth-source.ts | New auth source that shells out to gh auth token --hostname for GHES. Caches both successful and null (missing) tokens with separate TTLs (2 min / 30 s), plus in-flight deduplication — the previously reported negative-caching gap is addressed in this PR. |
| src/main/core/github/services/octokit-provider.ts | Now host-aware: derives the correct baseUrl (api.github.com vs /api/v3) and caches Octokit instances keyed by normalized host. Token invalidation flushes the relevant Octokit instance. |
| src/main/core/github/services/issue-service.ts | Migrated from raw Octokit to host-aware getOctokit and returns Result types. mapApiError has a minor inconsistency with githubApiAuthRequired (inline message construction diverges for github.com). |
| src/main/core/pull-requests/pr-sync-engine.ts | Now host-aware throughout: full/incremental/single/checks syncs all call getOctokit(host) and propagate auth errors as typed Results. Create/merge/markReady/getComments/getFiles all use the same pattern consistently. |
| src/main/core/pull-requests/controller.ts | Switched to providerRepositoryService for capability checks; introduces mapPrSyncEngineError for typed error propagation; adds cross-host PR rejection; telemetry capture correctly placed on both result-error and exception paths. |
| src/main/core/pull-requests/pr-sync-scheduler.ts | Primary path correctly uses githubRepositoryResolver.resolve() (with host probe) for new remote discovery. Fallback path uses parseRepositoryRef, which now accepts non-GitHub parseable URLs — could trigger spurious gh auth token calls for Bitbucket/GitLab remotes. |
| src/main/core/repository/provider-repository-service.ts | New service wiring project selected remote through host probe to a typed ProviderRepository. Error mapping covers all GitHubResolveError variants correctly. |
| src/renderer/features/projects/stores/repository-store.ts | Adds providerRepositoryInfo Resource backed by the new RPC call. Exposes providerRepository, pullRequestRepositoryUrl, and issueRepositoryUrl computed getters; properly invalidated on remote config changes and disposed on store cleanup. |
| src/main/core/pull-requests/pr-sync-errors.ts | New error module unifying PrSyncEngineError variants. toPrApiError correctly delegates to githubApiAuthRequired for auth status codes and preserves GitHub response.data.errors message extraction. |
Sequence Diagram
sequenceDiagram
participant Renderer
participant Controller
participant ProviderRepoSvc
participant Resolver
participant HostService
participant AuthService
participant GhesAuthSrc
participant Octokit
Renderer->>Controller: resolveProviderRepository(projectId)
Controller->>ProviderRepoSvc: resolveProject(projectId)
ProviderRepoSvc->>Resolver: resolve(remoteUrl)
Resolver->>HostService: probe(host)
HostService-->>Resolver: ok(host) cached 15min
Resolver-->>ProviderRepoSvc: ok(RepositoryRef)
ProviderRepoSvc-->>Controller: ok(ProviderRepository)
Controller-->>Renderer: ProviderRepository
Renderer->>Controller: createPullRequest(params)
Controller->>Controller: cross-host check
Controller->>PrSyncEngine: createPullRequest(params)
PrSyncEngine->>AuthService: getToken(host)
AuthService->>GhesAuthSrc: getToken(host) if GHES
GhesAuthSrc-->>AuthService: token cached 2min
AuthService-->>PrSyncEngine: ok(token)
PrSyncEngine->>Octokit: rest.pulls.create
Octokit-->>PrSyncEngine: url, number
PrSyncEngine-->>Controller: ok(url, number)
Controller-->>Renderer: ok(url, number)
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/main/core/github/services/issue-service.ts:174-195
**Third inline reconstruction of GitHub auth error messages**
`mapApiError`'s 401/403 branch re-builds the auth error string from scratch instead of delegating to `githubApiAuthRequired`. This is now the third location carrying that logic (the previous thread flagged the same pattern in `pr-sync-errors.ts`, which was fixed to delegate). As a side-effect, the messages diverge: for `github.com`, `mapApiError` produces `"GitHub authentication required. Connect GitHub from account settings."` (hint embedded) while `githubApiAuthRequired` produces `"GitHub authentication required."` (no hint). This makes the user-visible message dependent on whether the failure originates at Octokit-creation time vs. during the REST call, which is surprising. Replacing the inline branch with `return this.mapAuthError(githubApiAuthRequired(host))` would unify the messages and remove the duplication.
### Issue 2 of 2
src/main/core/pull-requests/pr-sync-scheduler.ts:168-171
**Fallback now returns all parseable remote URLs, not only GitHub ones**
The old fallback used `parseGitHubRepository` which was github.com-only. The new code uses `parseRepositoryRef`, which successfully parses any git remote (Bitbucket, GitLab, etc.). When `_projectRemoteUrls` is not yet populated (e.g. a task is provisioned before `onProjectMounted` completes), this fallback is used, and every parseable non-GitHub remote in the DB is returned. Each of those URLs is then passed to `prSyncEngine.syncSingle`, which calls `githubApiAuthService.getToken(bitbucket.org)` → `gh auth token --hostname bitbucket.org` — a subprocess call that fails and returns null. The sync short-circuits gracefully, but the spurious subprocess is new. The primary path in `_syncAndGetGitHubRemotes` correctly filters via the host probe; the fallback could apply the same filter with a simple host-equality check or a lightweight `isGitHubDotComHost` guard for known non-GHES hosts.
Reviews (2): Last reviewed commit: "refactor(github): share api auth error m..." | Re-trigger Greptile
- accepts github.com directly
- probes non github.com hosts for github compatible API support
- caches probe results in memory only