fix(backend): auto-link gitlab mrs for self-managed hosts (#2515)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -284,15 +284,113 @@ func (s *Service) trackPushAndAssociatePR(ctx context.Context, data watcher.GitE
|
||||
// provider's association logic, so the two providers' code paths issue zero
|
||||
// calls into each other's client. GitHub's proven detectPushAndAssociatePR is
|
||||
// called verbatim for every non-GitLab (including unknown/legacy-empty
|
||||
// provider) repository — this wraps it, it does not replace it. Extracted
|
||||
// from trackPushAndAssociatePR to keep that function inside the statement
|
||||
// budget.
|
||||
// provider) repository — this passes the shared repository identity into the
|
||||
// existing provider-specific path. Extracted from trackPushAndAssociatePR to
|
||||
// keep that function inside the statement budget.
|
||||
func (s *Service) dispatchPushDetection(ctx context.Context, sessionID, taskID, repositoryName, branch string) {
|
||||
if s.resolvePushRepositoryProvider(ctx, sessionID, taskID, repositoryName) == gitlabProviderName {
|
||||
s.detectPushAndAssociateMR(ctx, sessionID, taskID, repositoryName, branch)
|
||||
identity := s.resolvePushRepositoryIdentity(ctx, sessionID, taskID, repositoryName)
|
||||
if identity.provider == gitlabProviderName {
|
||||
s.detectPushAndAssociateMRWithIdentity(ctx, sessionID, taskID, repositoryName, branch, identity)
|
||||
return
|
||||
}
|
||||
s.detectPushAndAssociatePR(ctx, sessionID, taskID, repositoryName, branch)
|
||||
if s.githubService == nil {
|
||||
return
|
||||
}
|
||||
s.detectPushAndAssociatePRWithIdentity(ctx, sessionID, taskID, repositoryName, branch, identity)
|
||||
}
|
||||
|
||||
type pushRepositoryIdentity struct {
|
||||
owner string
|
||||
name string
|
||||
repositoryID string
|
||||
provider string
|
||||
projectPath string
|
||||
}
|
||||
|
||||
// resolvePushRepositoryIdentity resolves the repository, provider, and full
|
||||
// project path from one repository snapshot. Local checkout fallbacks are
|
||||
// shared by routing and association so a legacy row cannot be routed from one
|
||||
// identity while its provider-specific lookup uses another.
|
||||
func (s *Service) resolvePushRepositoryIdentity(
|
||||
ctx context.Context, sessionID, taskID, repositoryName string,
|
||||
) pushRepositoryIdentity {
|
||||
owner, name, repositoryID := s.resolvePushRepo(ctx, sessionID, taskID, repositoryName)
|
||||
identity := pushRepositoryIdentity{
|
||||
owner: owner,
|
||||
name: name,
|
||||
repositoryID: repositoryID,
|
||||
}
|
||||
if owner != "" && name != "" {
|
||||
identity.projectPath = owner + "/" + name
|
||||
}
|
||||
if repositoryID == "" {
|
||||
return identity
|
||||
}
|
||||
repoObj := s.getPushRepository(ctx, repositoryID)
|
||||
if repoObj == nil {
|
||||
return identity
|
||||
}
|
||||
remoteURL := s.enrichPushRepositoryIdentity(&identity, repoObj)
|
||||
if identity.projectPath == "" {
|
||||
identity.projectPath = gitLabProjectPathFromRemoteURL(remoteURL)
|
||||
}
|
||||
if identity.provider == "" {
|
||||
identity.provider = s.resolveConfiguredGitLabProvider(ctx, taskID, remoteURL)
|
||||
}
|
||||
return identity
|
||||
}
|
||||
|
||||
func (s *Service) getPushRepository(ctx context.Context, repositoryID string) *models.Repository {
|
||||
store, ok := s.repo.(repoStore)
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
repoObj, err := store.GetRepository(ctx, repositoryID)
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
return repoObj
|
||||
}
|
||||
|
||||
func (s *Service) enrichPushRepositoryIdentity(
|
||||
identity *pushRepositoryIdentity, repoObj *models.Repository,
|
||||
) string {
|
||||
identity.provider = repoObj.Provider
|
||||
if identity.provider == "" && repoObj.LocalPath != "" {
|
||||
if provider, _, localOwner, localName := service.ResolveGitRemoteProviderIdentity(repoObj.LocalPath); provider != "" && localOwner != "" {
|
||||
identity.provider = provider
|
||||
if identity.owner == "" && localName != "" {
|
||||
identity.owner = localOwner
|
||||
identity.name = localName
|
||||
identity.projectPath = localOwner + "/" + localName
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
remoteURL := repoObj.RemoteURL
|
||||
if remoteURL == "" && repoObj.LocalPath != "" {
|
||||
if origin, localOwner, localName := service.ResolveGitRemoteIdentity(repoObj.LocalPath); origin != "" && localOwner != "" && localName != "" {
|
||||
remoteURL = origin + "/" + localOwner + "/" + localName
|
||||
if identity.projectPath == "" {
|
||||
identity.projectPath = localOwner + "/" + localName
|
||||
}
|
||||
}
|
||||
}
|
||||
return remoteURL
|
||||
}
|
||||
|
||||
func (s *Service) resolveConfiguredGitLabProvider(ctx context.Context, taskID, remoteURL string) string {
|
||||
if s.gitlabMRLinkService == nil || remoteURL == "" {
|
||||
return ""
|
||||
}
|
||||
workspaceID := s.taskWorkspaceID(ctx, taskID)
|
||||
if workspaceID == "" {
|
||||
return ""
|
||||
}
|
||||
if s.gitlabMRLinkService.IsConfiguredGitLabHost(ctx, workspaceID, remoteURL) {
|
||||
return gitlabProviderName
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// resolvePushRepositoryProvider looks up the provider ("github", "gitlab", or
|
||||
@@ -300,51 +398,7 @@ func (s *Service) dispatchPushDetection(ctx context.Context, sessionID, taskID,
|
||||
// resolvePushRepo's owner/name matching rather than re-deriving it, so
|
||||
// dispatchPushDetection can route without duplicating that logic.
|
||||
func (s *Service) resolvePushRepositoryProvider(ctx context.Context, sessionID, taskID, repositoryName string) string {
|
||||
_, _, repositoryID := s.resolvePushRepo(ctx, sessionID, taskID, repositoryName)
|
||||
if repositoryID == "" {
|
||||
return ""
|
||||
}
|
||||
store, ok := s.repo.(repoStore)
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
repoObj, err := store.GetRepository(ctx, repositoryID)
|
||||
if err != nil || repoObj == nil {
|
||||
return ""
|
||||
}
|
||||
if repoObj.Provider != "" {
|
||||
return repoObj.Provider
|
||||
}
|
||||
// The row may not yet reflect a provider resolvePushRepo's own call just
|
||||
// derived from the local git remote: matchPushRepo/resolveSessionRepo
|
||||
// compute it in-memory and persist it via a detached backfill goroutine,
|
||||
// so this read can race ahead of that write on the very first push from a
|
||||
// repository with no durable provider yet. Recompute live from the same
|
||||
// local checkout instead of trusting a possibly-stale empty column.
|
||||
// ResolveGitRemoteProviderIdentity recognizes both github.com and
|
||||
// gitlab.com remotes (the same helper resolveRepositoryProviderIdentity
|
||||
// uses to backfill Repository rows in production), so this closes the
|
||||
// race for either provider rather than only GitHub.
|
||||
if repoObj.LocalPath != "" {
|
||||
if provider, _, owner, _ := service.ResolveGitRemoteProviderIdentity(repoObj.LocalPath); provider != "" && owner != "" {
|
||||
return provider
|
||||
}
|
||||
}
|
||||
// Self-managed GitLab instances never get a durable "gitlab" Provider tag
|
||||
// at all — resolveRepositoryProviderIdentity only recognizes github.com
|
||||
// and gitlab.com at discovery time, so the well-known-host fallback above
|
||||
// can never resolve them either; this is a permanent gap for self-managed
|
||||
// repositories, not just a narrow backfill race. remote_url is their only
|
||||
// durable identity signal (still populated by the same production
|
||||
// backfill), so compare it against the workspace's own configured GitLab
|
||||
// connection instead of a hostname allowlist.
|
||||
if s.gitlabMRLinkService != nil && repoObj.RemoteURL != "" {
|
||||
if workspaceID := s.taskWorkspaceID(ctx, taskID); workspaceID != "" &&
|
||||
s.gitlabMRLinkService.IsConfiguredGitLabHost(ctx, workspaceID, repoObj.RemoteURL) {
|
||||
return gitlabProviderName
|
||||
}
|
||||
}
|
||||
return ""
|
||||
return s.resolvePushRepositoryIdentity(ctx, sessionID, taskID, repositoryName).provider
|
||||
}
|
||||
|
||||
// shouldFirePushDetection decides whether to kick off PR association for one
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/gitlab"
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
@@ -49,7 +50,8 @@ func seedUnbackfilledRepoForProviderTest(t *testing.T, remoteURL string) *Servic
|
||||
}
|
||||
if err := repo.CreateTaskRepository(ctx, &models.TaskRepository{
|
||||
ID: "tr1", TaskID: "t1", RepositoryID: "repo1",
|
||||
CreatedAt: now, UpdatedAt: now,
|
||||
CheckoutBranch: "feat/a",
|
||||
CreatedAt: now, UpdatedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("create task repository: %v", err)
|
||||
}
|
||||
@@ -93,3 +95,101 @@ func TestResolvePushRepositoryProvider_LiveFallbackForUnbackfilledGitLabRepo(t *
|
||||
t.Fatalf("provider = %q, want %q (resolved live from the local checkout, not a possibly-stale DB column)", provider, gitlabProviderName)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolvePushRepositoryProvider_LiveFallbackForUnbackfilledSelfManagedGitLabRepo
|
||||
// closes the routing gap CodeRabbit flagged on PR #2515: a self-managed
|
||||
// GitLab repository whose remote_url column hasn't been backfilled yet
|
||||
// (LocalPath is the only signal) previously routed nowhere, because the
|
||||
// self-managed-host check only ever compared repoObj.RemoteURL — never a
|
||||
// live read of LocalPath — against the workspace's configured GitLab host.
|
||||
// dispatchPushDetection therefore sent the push down the GitHub detection
|
||||
// path, and the shared identity resolver's local-checkout fallback
|
||||
// (event_handlers_git.go) never got a chance to run because routing
|
||||
// bailed one step earlier than that fallback.
|
||||
func TestResolvePushRepositoryProvider_LiveFallbackForUnbackfilledSelfManagedGitLabRepo(t *testing.T) {
|
||||
const selfManagedRemote = "https://gitlab.internal.example.com/group/subgroup/widgets"
|
||||
svc := seedUnbackfilledRepoForProviderTest(t, selfManagedRemote)
|
||||
|
||||
fake := &fakeGitLabMRLinkService{taskMRs: make(map[string][]*gitlab.TaskMR)}
|
||||
fake.isConfiguredGitLabHostFunc = func(_ context.Context, workspaceID, remoteURL string) bool {
|
||||
return workspaceID == "ws1" && remoteURL == selfManagedRemote
|
||||
}
|
||||
svc.SetGitLabMRLinkService(fake)
|
||||
|
||||
provider := svc.resolvePushRepositoryProvider(context.Background(), "s1", "t1", "")
|
||||
if provider != gitlabProviderName {
|
||||
t.Fatalf("provider = %q, want %q (resolved live from the local checkout's remote, not requiring a persisted remote_url)", provider, gitlabProviderName)
|
||||
}
|
||||
if fake.isConfiguredGitLabHostCalls != 1 {
|
||||
t.Fatalf("expected IsConfiguredGitLabHost to be consulted exactly once, got %d", fake.isConfiguredGitLabHostCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestDispatchPushDetection_LiveFallbackForUnbackfilledSelfManagedGitLabRepo
|
||||
// covers the complete push path for a legacy row whose only repository
|
||||
// identity is the local checkout's origin. Provider routing and project-path
|
||||
// resolution must agree so the push reaches GitLab rather than falling back to
|
||||
// GitHub or being dropped before the MR lookup.
|
||||
func TestDispatchPushDetection_LiveFallbackForUnbackfilledSelfManagedGitLabRepo(t *testing.T) {
|
||||
const selfManagedRemote = "https://gitlab.internal.example.com/group/subgroup/widgets"
|
||||
svc := seedUnbackfilledRepoForProviderTest(t, selfManagedRemote)
|
||||
|
||||
fake := &fakeGitLabMRLinkService{taskMRs: make(map[string][]*gitlab.TaskMR)}
|
||||
fake.isConfiguredGitLabHostFunc = func(_ context.Context, workspaceID, remoteURL string) bool {
|
||||
return workspaceID == "ws1" && remoteURL == selfManagedRemote
|
||||
}
|
||||
var gotProjectPath string
|
||||
fake.autoLinkFunc = func(_ context.Context, _, _, _, repositoryID, projectPath, branch string) (*gitlab.TaskMR, error) {
|
||||
gotProjectPath = projectPath
|
||||
return &gitlab.TaskMR{TaskID: "t1", RepositoryID: repositoryID, ProjectPath: projectPath, MRIID: 5, HeadBranch: branch}, nil
|
||||
}
|
||||
svc.SetGitLabMRLinkService(fake)
|
||||
ghSvc := &mockGitHubService{}
|
||||
svc.SetGitHubService(ghSvc)
|
||||
|
||||
svc.dispatchPushDetection(context.Background(), "s1", "t1", "", "feat/a")
|
||||
|
||||
if fake.autoLinkCalls != 1 {
|
||||
t.Fatalf("expected one GitLab auto-link call, got %d", fake.autoLinkCalls)
|
||||
}
|
||||
if gotProjectPath != "group/subgroup/widgets" {
|
||||
t.Fatalf("projectPath = %q, want %q", gotProjectPath, "group/subgroup/widgets")
|
||||
}
|
||||
if fake.lastAutoLinkRepositoryID != "repo1" {
|
||||
t.Fatalf("repositoryID = %q, want repo1", fake.lastAutoLinkRepositoryID)
|
||||
}
|
||||
if ghSvc.getPRWatchBySessionRepoAndBranchCalls != 0 {
|
||||
t.Fatalf("expected zero GitHub lookups for a self-managed GitLab repository, got %d", ghSvc.getPRWatchBySessionRepoAndBranchCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckSessionMR_LiveFallbackForUnbackfilledSelfManagedGitLabRepo covers
|
||||
// the on-demand path for the same local-only repository shape. It must apply
|
||||
// the provider guard and pass the local checkout's full nested project path to
|
||||
// GitLab before creating the association.
|
||||
func TestCheckSessionMR_LiveFallbackForUnbackfilledSelfManagedGitLabRepo(t *testing.T) {
|
||||
const selfManagedRemote = "https://gitlab.internal.example.com/group/subgroup/widgets"
|
||||
svc := seedUnbackfilledRepoForProviderTest(t, selfManagedRemote)
|
||||
|
||||
fake := &fakeGitLabMRLinkService{taskMRs: make(map[string][]*gitlab.TaskMR)}
|
||||
fake.isConfiguredGitLabHostFunc = func(_ context.Context, workspaceID, remoteURL string) bool {
|
||||
return workspaceID == "ws1" && remoteURL == selfManagedRemote
|
||||
}
|
||||
var gotProjectPath string
|
||||
fake.autoLinkFunc = func(_ context.Context, _, _, _, repositoryID, projectPath, branch string) (*gitlab.TaskMR, error) {
|
||||
gotProjectPath = projectPath
|
||||
return &gitlab.TaskMR{TaskID: "t1", RepositoryID: repositoryID, ProjectPath: projectPath, MRIID: 5, HeadBranch: branch}, nil
|
||||
}
|
||||
svc.SetGitLabMRLinkService(fake)
|
||||
|
||||
found, err := svc.CheckSessionMR(context.Background(), "t1", "s1")
|
||||
if err != nil {
|
||||
t.Fatalf("CheckSessionMR: %v", err)
|
||||
}
|
||||
if !found {
|
||||
t.Fatal("expected found=true for a self-managed GitLab repository with only a local origin")
|
||||
}
|
||||
if gotProjectPath != "group/subgroup/widgets" {
|
||||
t.Fatalf("projectPath = %q, want %q", gotProjectPath, "group/subgroup/widgets")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -600,13 +600,19 @@ func (s *Service) detectPushAndAssociatePR(
|
||||
if s.githubService == nil {
|
||||
return
|
||||
}
|
||||
identity := s.resolvePushRepositoryIdentity(ctx, sessionID, taskID, repositoryName)
|
||||
s.detectPushAndAssociatePRWithIdentity(ctx, sessionID, taskID, repositoryName, branch, identity)
|
||||
}
|
||||
|
||||
func (s *Service) detectPushAndAssociatePRWithIdentity(
|
||||
ctx context.Context, sessionID, taskID, repositoryName, branch string, identity pushRepositoryIdentity,
|
||||
) {
|
||||
workspaceID := s.taskWorkspaceID(ctx, taskID)
|
||||
if workspaceID == "" {
|
||||
return
|
||||
}
|
||||
|
||||
owner, repoName, repositoryID := s.resolvePushRepo(ctx, sessionID, taskID, repositoryName)
|
||||
if owner == "" || repoName == "" {
|
||||
if identity.owner == "" || identity.name == "" {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -617,7 +623,7 @@ func (s *Service) detectPushAndAssociatePR(
|
||||
// If the watch already has a PR number, the PR was found — nothing to do.
|
||||
// If the watch has pr_number=0, it's still searching — do an immediate
|
||||
// search (faster than waiting for the 1-minute poller).
|
||||
existing, err := s.githubService.GetPRWatchBySessionRepoAndBranch(ctx, sessionID, repositoryID, branch)
|
||||
existing, err := s.githubService.GetPRWatchBySessionRepoAndBranch(ctx, sessionID, identity.repositoryID, branch)
|
||||
if err == nil && existing != nil {
|
||||
if existing.PRNumber > 0 {
|
||||
return // PR already found and being monitored
|
||||
@@ -636,12 +642,12 @@ func (s *Service) detectPushAndAssociatePR(
|
||||
case <-time.After(delay):
|
||||
}
|
||||
// Re-check if a watch was created in the meantime (e.g. by CreatePR callback)
|
||||
if ex, err := s.githubService.GetPRWatchBySessionRepoAndBranch(ctx, sessionID, repositoryID, branch); err == nil && ex != nil {
|
||||
if ex, err := s.githubService.GetPRWatchBySessionRepoAndBranch(ctx, sessionID, identity.repositoryID, branch); err == nil && ex != nil {
|
||||
return
|
||||
}
|
||||
}
|
||||
foundPR, findErr := s.githubService.FindPRByBranchForWorkspace(
|
||||
ctx, workspaceID, owner, repoName, branch,
|
||||
ctx, workspaceID, identity.owner, identity.name, branch,
|
||||
)
|
||||
if findErr != nil || foundPR == nil {
|
||||
s.logger.Debug("no PR found for branch (will retry)",
|
||||
@@ -657,7 +663,7 @@ func (s *Service) detectPushAndAssociatePR(
|
||||
zap.String("repository_name", repositoryName),
|
||||
zap.Int("pr_number", foundPR.Number),
|
||||
zap.String("branch", branch))
|
||||
s.associatePRFromPushScoped(ctx, workspaceID, sessionID, taskID, owner, repoName, repositoryID, branch, foundPR)
|
||||
s.associatePRFromPushScoped(ctx, workspaceID, sessionID, taskID, identity.owner, identity.name, identity.repositoryID, branch, foundPR)
|
||||
return
|
||||
}
|
||||
s.logger.Warn("exhausted all retries, no PR found after push",
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"go.uber.org/zap"
|
||||
|
||||
"github.com/kandev/kandev/internal/gitlab"
|
||||
"github.com/kandev/kandev/internal/task/service"
|
||||
)
|
||||
|
||||
// detectPushAndAssociateMR is the GitLab twin of detectPushAndAssociatePR: on
|
||||
@@ -17,6 +18,16 @@ import (
|
||||
// push (a common `glab mr create` race) still gets picked up.
|
||||
func (s *Service) detectPushAndAssociateMR(
|
||||
ctx context.Context, sessionID, taskID, repositoryName, branch string,
|
||||
) {
|
||||
if s.gitlabMRLinkService == nil {
|
||||
return
|
||||
}
|
||||
identity := s.resolvePushRepositoryIdentity(ctx, sessionID, taskID, repositoryName)
|
||||
s.detectPushAndAssociateMRWithIdentity(ctx, sessionID, taskID, repositoryName, branch, identity)
|
||||
}
|
||||
|
||||
func (s *Service) detectPushAndAssociateMRWithIdentity(
|
||||
ctx context.Context, sessionID, taskID, repositoryName, branch string, identity pushRepositoryIdentity,
|
||||
) {
|
||||
if s.gitlabMRLinkService == nil {
|
||||
return
|
||||
@@ -36,11 +47,11 @@ func (s *Service) detectPushAndAssociateMR(
|
||||
if workspaceID == "" {
|
||||
return
|
||||
}
|
||||
owner, repoName, repositoryID := s.resolvePushRepo(ctx, sessionID, taskID, repositoryName)
|
||||
if owner == "" || repoName == "" || repositoryID == "" {
|
||||
if identity.projectPath == "" || identity.repositoryID == "" {
|
||||
return
|
||||
}
|
||||
projectPath := owner + "/" + repoName
|
||||
projectPath := identity.projectPath
|
||||
repositoryID := identity.repositoryID
|
||||
|
||||
// Already linked for this (task, repository, branch) — don't re-link, but
|
||||
// still make sure the refresh watch exists. AssociateExistingMRByURL (the
|
||||
@@ -203,19 +214,19 @@ func (s *Service) CheckSessionMR(ctx context.Context, taskID, sessionID string)
|
||||
// gitlab.check_session_mr for a GitHub-backed session would install a
|
||||
// bogus GitLab watch keyed off the GitHub repository's owner/name before
|
||||
// AutoLinkMRForBranch's own identity check ever runs.
|
||||
if s.resolvePushRepositoryProvider(ctx, sessionID, taskID, "") != gitlabProviderName {
|
||||
identity := s.resolvePushRepositoryIdentity(ctx, sessionID, taskID, "")
|
||||
if identity.provider != gitlabProviderName {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
owner, repoName, repositoryID := s.resolvePushRepo(ctx, sessionID, taskID, "")
|
||||
if owner == "" || repoName == "" || repositoryID == "" {
|
||||
projectPath, repositoryID := identity.projectPath, identity.repositoryID
|
||||
if projectPath == "" || repositoryID == "" {
|
||||
return false, nil
|
||||
}
|
||||
branch := strings.TrimSpace(s.resolvePRWatchBranch(ctx, taskID, sessionID, ""))
|
||||
if branch == "" {
|
||||
return false, nil
|
||||
}
|
||||
projectPath := owner + "/" + repoName
|
||||
|
||||
// Already associated for this exact (repository, branch) — ensure its
|
||||
// watch exists (Create-MR action / manual URL linking writes
|
||||
@@ -258,3 +269,14 @@ func (s *Service) CheckSessionMR(ctx context.Context, taskID, sessionID string)
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// gitLabProjectPathFromRemoteURL derives a GitLab project path
|
||||
// ("group/subgroup/project") from a raw git remote URL, reusing
|
||||
// service.ParseGitRemoteIdentity so nested subgroups and every ssh/https/scp
|
||||
// remote shape stay in sync with the rest of the codebase's identity
|
||||
// parsing. Returns "" when the URL is empty, malformed, or has no project
|
||||
// segment.
|
||||
func gitLabProjectPathFromRemoteURL(remoteURL string) string {
|
||||
_, projectPath := service.ParseGitRemoteIdentity(remoteURL)
|
||||
return projectPath
|
||||
}
|
||||
|
||||
@@ -480,3 +480,168 @@ func TestCheckSessionMRDeniesOwnSessionPairedWithForeignTask(t *testing.T) {
|
||||
t.Error("found = true; an owned session must not unlock another user's task")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGitLabProjectPathFromRemoteURL covers the remote-URL shapes a
|
||||
// self-managed GitLab (or a gitlab.com repo added as source_type: local)
|
||||
// must resolve a project path from, including nested subgroups, since
|
||||
// detectPushAndAssociateMR reassembles owner+"/"+repoName into the project
|
||||
// path it sends to the GitLab API.
|
||||
func TestGitLabProjectPathFromRemoteURL(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
url string
|
||||
want string
|
||||
}{
|
||||
{"self-managed ssh nested subgroup", "ssh://gitlab.example.com/group/subgroup/project", "group/subgroup/project"},
|
||||
{"self-managed https with .git", "https://gitlab.example.com/group/project.git", "group/project"},
|
||||
{"self-managed scp-style", "git@gitlab.example.com:group/subgroup/project.git", "group/subgroup/project"},
|
||||
{"gitlab.com", "https://gitlab.com/owner/repo", "owner/repo"},
|
||||
{"empty", "", ""},
|
||||
{"malformed", "not-a-url", ""},
|
||||
{"no project segment", "https://gitlab.example.com/onlyowner", ""},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := gitLabProjectPathFromRemoteURL(tt.url)
|
||||
if got != tt.want {
|
||||
t.Errorf("gitLabProjectPathFromRemoteURL(%q) = %q, want %q", tt.url, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// seedSelfManagedGitLabSessionWithRepo mirrors seedGitLabSessionWithRepo but
|
||||
// reproduces the live-instance row shape for a self-managed GitLab
|
||||
// repository: all four provider columns (provider, provider_host,
|
||||
// provider_owner, provider_name) are empty, and remote_url is the only
|
||||
// identity signal — source_type: local repos, and any repo added before
|
||||
// self-managed hosts got a discovery-time provider tag, look exactly like
|
||||
// this.
|
||||
func seedSelfManagedGitLabSessionWithRepo(t *testing.T) (*Service, *fakeGitLabMRLinkService) {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
|
||||
repoObj := &models.Repository{
|
||||
ID: "repo1", WorkspaceID: "ws1", Name: "forge",
|
||||
SourceType: "local",
|
||||
RemoteURL: "ssh://gitlab.example.com/group/subgroup/project",
|
||||
CreatedAt: now, UpdatedAt: now,
|
||||
}
|
||||
if err := repo.CreateRepository(ctx, repoObj); err != nil {
|
||||
t.Fatalf("create repository: %v", err)
|
||||
}
|
||||
if err := repo.CreateTaskRepository(ctx, &models.TaskRepository{
|
||||
ID: "tr1", TaskID: "t1", RepositoryID: "repo1", CheckoutBranch: "feat/a",
|
||||
CreatedAt: now, UpdatedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("create task repository: %v", err)
|
||||
}
|
||||
session, _ := repo.GetTaskSession(ctx, "s1")
|
||||
session.RepositoryID = "repo1"
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("update session: %v", err)
|
||||
}
|
||||
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
fake := &fakeGitLabMRLinkService{taskMRs: make(map[string][]*gitlab.TaskMR)}
|
||||
fake.isConfiguredGitLabHostFunc = func(_ context.Context, _, _ string) bool { return true }
|
||||
svc.SetGitLabMRLinkService(fake)
|
||||
return svc, fake
|
||||
}
|
||||
|
||||
// TestDetectPushAndAssociateMR_SelfManagedGitLabResolvesProjectFromRemoteURL
|
||||
// is the regression test for the self-managed-GitLab auto-link gap: a
|
||||
// repository row with every provider column empty (source_type: local, or
|
||||
// any repo added before self-managed hosts got a discovery-time tag) must
|
||||
// still reach AutoLinkMRForBranch, deriving the project path from remote_url
|
||||
// instead of the never-populated provider_owner/provider_name. Before the
|
||||
// shared identity resolver fallback this failed with 0 AutoLinkMRForBranch
|
||||
// calls, because resolvePushRepo alone returns empty owner/name for any
|
||||
// non-github.com, non-provider-tagged repository.
|
||||
func TestDetectPushAndAssociateMR_SelfManagedGitLabResolvesProjectFromRemoteURL(t *testing.T) {
|
||||
svc, fake := seedSelfManagedGitLabSessionWithRepo(t)
|
||||
var gotProjectPath string
|
||||
fake.autoLinkFunc = func(_ context.Context, _, _, _, repositoryID, projectPath, branch string) (*gitlab.TaskMR, error) {
|
||||
gotProjectPath = projectPath
|
||||
return &gitlab.TaskMR{TaskID: "t1", RepositoryID: repositoryID, ProjectPath: projectPath, MRIID: 5, HeadBranch: branch}, nil
|
||||
}
|
||||
|
||||
svc.detectPushAndAssociateMR(context.Background(), "s1", "t1", "", "feat/a")
|
||||
|
||||
if fake.autoLinkCalls != 1 {
|
||||
t.Fatalf("expected 1 AutoLinkMRForBranch call for a self-managed GitLab repo with empty provider columns, got %d", fake.autoLinkCalls)
|
||||
}
|
||||
if gotProjectPath != "group/subgroup/project" {
|
||||
t.Fatalf("projectPath = %q, want %q (nested subgroup must survive)", gotProjectPath, "group/subgroup/project")
|
||||
}
|
||||
if fake.lastAutoLinkRepositoryID != "repo1" {
|
||||
t.Fatalf("repositoryID = %q, want repo1", fake.lastAutoLinkRepositoryID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckSessionMR_SelfManagedGitLabResolvesProjectFromRemoteURL is the
|
||||
// on-demand-check twin of the push-detection regression test above.
|
||||
func TestCheckSessionMR_SelfManagedGitLabResolvesProjectFromRemoteURL(t *testing.T) {
|
||||
svc, fake := seedSelfManagedGitLabSessionWithRepo(t)
|
||||
var gotProjectPath string
|
||||
fake.autoLinkFunc = func(_ context.Context, _, _, _, repositoryID, projectPath, branch string) (*gitlab.TaskMR, error) {
|
||||
gotProjectPath = projectPath
|
||||
return &gitlab.TaskMR{TaskID: "t1", RepositoryID: repositoryID, ProjectPath: projectPath, MRIID: 5, HeadBranch: branch}, nil
|
||||
}
|
||||
|
||||
found, err := svc.CheckSessionMR(context.Background(), "t1", "s1")
|
||||
|
||||
if err != nil {
|
||||
t.Fatalf("CheckSessionMR: %v", err)
|
||||
}
|
||||
if !found {
|
||||
t.Fatal("expected found=true for a self-managed GitLab repo with empty provider columns")
|
||||
}
|
||||
if gotProjectPath != "group/subgroup/project" {
|
||||
t.Fatalf("projectPath = %q, want %q (nested subgroup must survive)", gotProjectPath, "group/subgroup/project")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDetectPushAndAssociatePR_SelfManagedGitLabRemoteStillBails pins the
|
||||
// "do not widen the shared helper" decision behind the self-managed-GitLab
|
||||
// auto-link fix: resolvePushRepo (shared with the GitHub path) must keep
|
||||
// returning empty owner/name for a non-github.com remote even though
|
||||
// shared identity resolver now has its own remote_url fallback on the
|
||||
// GitLab-only path. If a future refactor widened resolvePushRepo instead,
|
||||
// this repository would start reaching the GitHub API with a GitLab
|
||||
// owner/name whenever routing ever misfired (e.g. no GitLab connection
|
||||
// configured for the workspace).
|
||||
func TestDetectPushAndAssociatePR_SelfManagedGitLabRemoteStillBails(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
repoObj := &models.Repository{
|
||||
ID: "repo1", WorkspaceID: "ws1", Name: "forge",
|
||||
SourceType: "local",
|
||||
RemoteURL: "ssh://gitlab.example.com/group/subgroup/project",
|
||||
CreatedAt: now, UpdatedAt: now,
|
||||
}
|
||||
if err := repo.CreateRepository(ctx, repoObj); err != nil {
|
||||
t.Fatalf("create repository: %v", err)
|
||||
}
|
||||
session, _ := repo.GetTaskSession(ctx, "s1")
|
||||
session.RepositoryID = "repo1"
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("update session: %v", err)
|
||||
}
|
||||
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
ghSvc := &mockGitHubService{}
|
||||
svc.SetGitHubService(ghSvc)
|
||||
|
||||
svc.detectPushAndAssociatePR(ctx, "s1", "t1", "", "feat/a")
|
||||
|
||||
if ghSvc.getPRWatchBySessionRepoAndBranchCalls != 0 {
|
||||
t.Fatalf("expected detectPushAndAssociatePR to bail before any GitHub lookup for a GitLab remote (resolvePushRepo must still return empty owner/name), got %d watch lookups",
|
||||
ghSvc.getPRWatchBySessionRepoAndBranchCalls)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user