From f75eae3dc65de8e56a0557b86d368eabc3ea1cd4 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Wed, 6 May 2026 08:44:55 +0200 Subject: [PATCH] Fix backend forge detection for GitHub Enterprise users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `forge_provider` backend command was missing from the Tauri ACL permissions list (fixed in an earlier commit), which meant the query silently failed for all desktop users since 0.19.4. Regular github.com users were unaffected because the frontend had redundant domain-matching logic that masked the failure. GitHub Enterprise Server users (e.g. github.ourinternaldomain.net) had no such fallback and lost their forge integration (PR button, reviews, etc.). ACLs are going away soon (tauri-apps/tauri#15266), so rather than papering over backend failures with frontend fallbacks, this commit: - Removes the redundant frontend domain-matching in forgeFactory that was masking the broken backend query - Simplifies forge type resolution to: manual override → backend detection → default - Adds Rust tests confirming GHES subdomain detection works for both SSH and HTTPS URL forms --- .../src/lib/forge/forgeFactory.svelte.ts | 53 +++--------- .../src/lib/forge/forgeFactory.test.ts | 81 ++++++++++--------- crates/but-forge/src/lib.rs | 22 ++++- 3 files changed, 74 insertions(+), 82 deletions(-) diff --git a/apps/desktop/src/lib/forge/forgeFactory.svelte.ts b/apps/desktop/src/lib/forge/forgeFactory.svelte.ts index 22bd95c982a..7a9717015ef 100644 --- a/apps/desktop/src/lib/forge/forgeFactory.svelte.ts +++ b/apps/desktop/src/lib/forge/forgeFactory.svelte.ts @@ -1,9 +1,9 @@ -import { AZURE_DOMAIN, AzureDevOps } from "$lib/forge/azure/azure"; -import { BitBucket, BITBUCKET_DOMAIN } from "$lib/forge/bitbucket/bitbucket"; +import { AzureDevOps } from "$lib/forge/azure/azure"; +import { BitBucket } from "$lib/forge/bitbucket/bitbucket"; import { DefaultForge } from "$lib/forge/default/default"; -import { GitHub, GITHUB_DOMAIN } from "$lib/forge/github/github"; +import { GitHub } from "$lib/forge/github/github"; import { GitHubClient } from "$lib/forge/github/githubClient"; -import { GitLab, GITLAB_DOMAIN, GITLAB_SUB_DOMAIN } from "$lib/forge/gitlab/gitlab"; +import { GitLab } from "$lib/forge/gitlab/gitlab"; import { InjectionToken } from "@gitbutler/core/context"; import { deepCompare } from "@gitbutler/shared/compare"; import type { ForgeProvider } from "$lib/baseBranch/baseBranch"; @@ -113,16 +113,16 @@ export class DefaultForgeFactory implements Reactive { } = config; this._githubError = githubError; if (repo && baseBranch) { - this._determinedForgeType = this.determineForgeType(repo, detectedForgeProvider); + const forgeType = forgeOverride ?? detectedForgeProvider ?? "default"; + this._determinedForgeType = forgeType; this._forge = this.build({ repo, pushRepo, baseBranch, + forgeType, githubAuthenticated, forgeIsLoading, gitlabAuthenticated, - detectedForgeProvider, - forgeOverride, }); } else { this._determinedForgeType = "default"; @@ -134,25 +134,19 @@ export class DefaultForgeFactory implements Reactive { repo, pushRepo, baseBranch, + forgeType, githubAuthenticated, forgeIsLoading, gitlabAuthenticated, - detectedForgeProvider, - forgeOverride, }: { repo: RepoInfo; pushRepo?: RepoInfo; baseBranch: string; + forgeType: ForgeName; githubAuthenticated?: boolean; forgeIsLoading?: boolean; gitlabAuthenticated?: boolean; - detectedForgeProvider: ForgeProvider | undefined; - forgeOverride: ForgeName | undefined; }): Forge { - let forgeType = this.determineForgeType(repo, detectedForgeProvider); - if (forgeType === "default" && forgeOverride) { - forgeType = forgeOverride; - } const forkStr = pushRepo && pushRepo.hash !== repo.hash ? `${pushRepo.owner}:${pushRepo.name}` : undefined; @@ -198,35 +192,6 @@ export class DefaultForgeFactory implements Reactive { return this.default; } - private determineForgeType( - repo: RepoInfo, - detectedForgeProvider: ForgeProvider | undefined, - ): ForgeName { - if (detectedForgeProvider) { - return detectedForgeProvider; - } - const domain = repo.domain; - - if (domain.includes(GITHUB_DOMAIN)) { - return "github"; - } - if ( - domain === GITLAB_DOMAIN || - domain.startsWith(GITLAB_SUB_DOMAIN + ".") || - domain.startsWith("xy" + GITLAB_SUB_DOMAIN + ".") // Temporary workaround until we have foerge overrides implemented - ) { - return "gitlab"; - } - if (domain.includes(BITBUCKET_DOMAIN)) { - return "bitbucket"; - } - if (domain.includes(AZURE_DOMAIN)) { - return "azure"; - } - - return "default"; - } - invalidate(tags: TagDescription[]) { const action = this.current.invalidate(tags); const { dispatch } = this.params; diff --git a/apps/desktop/src/lib/forge/forgeFactory.test.ts b/apps/desktop/src/lib/forge/forgeFactory.test.ts index 55473fb05bc..29dfdb77efb 100644 --- a/apps/desktop/src/lib/forge/forgeFactory.test.ts +++ b/apps/desktop/src/lib/forge/forgeFactory.test.ts @@ -52,13 +52,12 @@ describe.concurrent("DefaultforgeFactory", () => { owner: "test-owner", }, baseBranch: "some-base", - detectedForgeProvider: undefined, - forgeOverride: undefined, + forgeType: "github", }), ).instanceOf(GitHub); }); - test("Create self hosted Gitlab service", async () => { + test("Create GitLab service", async () => { const factory = new DefaultForgeFactory({ gitHubClient, gitHubApi, @@ -71,18 +70,17 @@ describe.concurrent("DefaultforgeFactory", () => { expect( factory.build({ repo: { - domain: "gitlab.domain.com", + domain: "gitlab.com", name: "test-repo", owner: "test-owner", }, baseBranch: "some-base", - detectedForgeProvider: undefined, - forgeOverride: undefined, + forgeType: "gitlab", }), ).instanceOf(GitLab); }); - test("Create Gitlab service", async () => { + test("setConfig uses detectedForgeProvider when present", async () => { const factory = new DefaultForgeFactory({ gitHubClient, gitHubApi, @@ -92,21 +90,17 @@ describe.concurrent("DefaultforgeFactory", () => { posthog, dispatch, }); - expect( - factory.build({ - repo: { - domain: "gitlab.com", - name: "test-repo", - owner: "test-owner", - }, - baseBranch: "some-base", - detectedForgeProvider: undefined, - forgeOverride: undefined, - }), - ).instanceOf(GitLab); + factory.setConfig({ + repo: { domain: "github.example.net", name: "test-repo", owner: "test-owner" }, + baseBranch: "main", + detectedForgeProvider: "github", + forgeOverride: undefined, + }); + expect(factory.current).instanceOf(GitHub); + expect(factory.determinedForgeType).toBe("github"); }); - test("Respects detectedForgeProvider: GitHub", async () => { + test("setConfig falls back to detectedForgeProvider when forgeOverride is absent", async () => { const factory = new DefaultForgeFactory({ gitHubClient, gitHubApi, @@ -116,39 +110,52 @@ describe.concurrent("DefaultforgeFactory", () => { posthog, dispatch, }); - const result = factory.build({ - repo: { - domain: "gitlab.com", - name: "test-repo", - owner: "test-owner", - }, + factory.setConfig({ + repo: { domain: "github.example.net", name: "test-repo", owner: "test-owner" }, baseBranch: "main", detectedForgeProvider: "github", forgeOverride: undefined, }); - expect(result).instanceOf(GitHub); + expect(factory.current).instanceOf(GitHub); + expect(factory.determinedForgeType).toBe("github"); }); - test("Respects detectedForgeProvider: GitLab", async () => { + test("setConfig resolves to default when both detectedForgeProvider and forgeOverride are absent", async () => { const factory = new DefaultForgeFactory({ gitHubClient, gitHubApi, - gitLabClient, backendApi, + gitLabClient, gitLabApi, posthog, dispatch, }); - const result = factory.build({ - repo: { - domain: "github.com", - name: "test-repo", - owner: "test-owner", - }, + factory.setConfig({ + repo: { domain: "custom.example.com", name: "test-repo", owner: "test-owner" }, baseBranch: "main", - detectedForgeProvider: "gitlab", + detectedForgeProvider: undefined, forgeOverride: undefined, }); - expect(result).instanceOf(GitLab); + expect(factory.determinedForgeType).toBe("default"); + }); + + test("forgeOverride takes precedence over detectedForgeProvider", async () => { + const factory = new DefaultForgeFactory({ + gitHubClient, + gitHubApi, + backendApi, + gitLabClient, + gitLabApi, + posthog, + dispatch, + }); + factory.setConfig({ + repo: { domain: "github.com", name: "test-repo", owner: "test-owner" }, + baseBranch: "main", + detectedForgeProvider: "github", + forgeOverride: "gitlab", + }); + expect(factory.current).instanceOf(GitLab); + expect(factory.determinedForgeType).toBe("gitlab"); }); }); diff --git a/crates/but-forge/src/lib.rs b/crates/but-forge/src/lib.rs index 77fc8e6f0de..9911958c1d2 100644 --- a/crates/but-forge/src/lib.rs +++ b/crates/but-forge/src/lib.rs @@ -148,7 +148,8 @@ pub fn get_all_forge_accounts() -> anyhow::Result> { #[cfg(test)] mod tests { use super::{ - ForgeName, ForgeUser, match_host_to_accounts_custom_host, normalize_host_for_comparison, + ForgeName, ForgeUser, derive_forge_repo_info, match_host_to_accounts_custom_host, + normalize_host_for_comparison, }; #[test] @@ -280,4 +281,23 @@ mod tests { "repository.com" ); } + + #[test] + fn derive_forge_detects_github_enterprise_subdomain_ssh() { + let info = derive_forge_repo_info("git@github.ourinternaldomain.net:our-org/my-repo.git") + .expect("should detect GHES from github.* subdomain"); + assert_eq!(info.forge, ForgeName::GitHub); + assert_eq!(info.owner, "our-org"); + assert_eq!(info.repo, "my-repo"); + } + + #[test] + fn derive_forge_detects_github_enterprise_subdomain_https() { + let info = + derive_forge_repo_info("https://github.ourinternaldomain.net/our-org/my-repo.git") + .expect("should detect GHES from github.* subdomain"); + assert_eq!(info.forge, ForgeName::GitHub); + assert_eq!(info.owner, "our-org"); + assert_eq!(info.repo, "my-repo"); + } }