refactor(server): consolidate duplicate URL parsing into splitRepoPath

internal/server/handlers.go

@@ -10,30 +10,14 @@ import (
 )
 
 // parseRepoPath parses a /repos/... URL path into the full repo name and the
-// endpoint string that follows the "/-/" separator.
+// endpoint string that follows the "/-/" separator. Delegates to splitRepoPath.
 //
 //	/repos/acme/myrepo/-/tree        → ("acme/myrepo", "tree", true)
 //	/repos/acme/team/sub/-/commit    → ("acme/team/sub", "commit", true)
 //	/repos/acme/myrepo               → ("acme/myrepo", "", true)  // bare repo
 //	/something/else                  → ("", "", false)
-//
-// NOTE: repoAndSubPath in middleware.go parses the same "/-/" URL format for
-// RBAC purposes. Both functions must be kept in sync if the URL structure changes.
 func parseRepoPath(path string) (repoName, endpoint string, ok bool) {
-	const prefix = "/repos/"
-	if !strings.HasPrefix(path, prefix) {
-		return "", "", false
-	}
-	rest := path[len(prefix):]
-	if rest == "" {
-		return "", "", false
-	}
-	repoName, endpoint, found := strings.Cut(rest, "/-/")
-	if !found {
-		// Bare /repos/:repopath — no endpoint
-		return rest, "", true
-	}
-	return repoName, endpoint, true
+	return splitRepoPath(path)
 }
 
 // handleReposPrefix is the catch-all handler for all /repos/... paths (except

internal/server/middleware.go

@@ -192,24 +192,16 @@ func RBACMiddleware(roles RoleStore, bootstrapAdmin string) func(http.Handler) h
 
 // repoAndSubPath extracts the repo name and sub-path from a /repos/.../-/subpath URL.
 // Returns ("", "") for non-repo paths or bare /repos/:repopath paths with no /-/ separator.
+// Delegates to splitRepoPath for the core parsing logic.
 //
 // Examples:
 //
 //	/repos/acme/myrepo/-/commit  →  ("acme/myrepo", "commit")
 //	/repos/acme/team/sub/-/tree  →  ("acme/team/sub", "tree")
 //	/repos/acme/myrepo            →  ("", "")  — bare repo, no RBAC check needed
-//
-// NOTE: parseRepoPath in handlers.go parses the same "/-/" URL format for routing.
-// Both functions must be kept in sync if the URL structure changes.
 func repoAndSubPath(path string) (repo, subPath string) {
-	const prefix = "/repos/"
-	if !strings.HasPrefix(path, prefix) {
-		return "", ""
-	}
-	rest := path[len(prefix):]
-	repo, subPath, found := strings.Cut(rest, "/-/")
-	if !found {
-		// /repos/:repopath with no /-/ — no sub-path, no RBAC check needed.
+	repo, subPath, ok := splitRepoPath(path)
+	if !ok || subPath == "" {
 		return "", ""
 	}
 	return repo, subPath

internal/server/middleware_test.go

@@ -1066,9 +1066,51 @@ func TestJobTokenMiddleware_WrongIssuer(t *testing.T) {
 }
 
 // ---------------------------------------------------------------------------
-// repoAndSubPath tests
+// URL parsing tests
 // ---------------------------------------------------------------------------
 
+// TestSplitRepoPathConsistency verifies that parseRepoPath and repoAndSubPath
+// agree on the repo name and endpoint for every path that contains a "/-/"
+// separator. Both functions delegate to splitRepoPath, so any divergence would
+// indicate a bug in the wrapper logic.
+func TestSplitRepoPathConsistency(t *testing.T) {
+	paths := []string{
+		"/repos/acme/myrepo/-/tree",
+		"/repos/acme/myrepo/-/commit",
+		"/repos/acme/myrepo/-/branch/feature/with/slashes",
+		"/repos/acme/team/sub/-/tree",
+		"/repos/acme/team/sub/-/roles/bob@example.com",
+		"/repos/org/myrepo/-/branches",
+		// Paths without /-/ — both functions return empty strings for repo/endpoint.
+		"/repos/myrepo",
+		"/repos/org/myrepo",
+		"/healthz",
+		"",
+	}
+	for _, path := range paths {
+		parsedRepo, parsedEndpoint, parsedOK := parseRepoPath(path)
+		rbacRepo, rbacEndpoint := repoAndSubPath(path)
+
+		// For paths with a /-/ separator, both functions must agree on repo and endpoint.
+		if parsedOK && parsedEndpoint != "" {
+			if parsedRepo != rbacRepo {
+				t.Errorf("path=%q: repo mismatch: parseRepoPath=%q repoAndSubPath=%q",
+					path, parsedRepo, rbacRepo)
+			}
+			if parsedEndpoint != rbacEndpoint {
+				t.Errorf("path=%q: endpoint mismatch: parseRepoPath=%q repoAndSubPath=%q",
+					path, parsedEndpoint, rbacEndpoint)
+			}
+		} else {
+			// No /-/ separator: repoAndSubPath must return ("", "").
+			if rbacRepo != "" || rbacEndpoint != "" {
+				t.Errorf("path=%q: repoAndSubPath should return (\"\", \"\") for bare/non-repo path, got (%q, %q)",
+					path, rbacRepo, rbacEndpoint)
+			}
+		}
+	}
+}
+
 func TestRepoAndSubPath(t *testing.T) {
 	tests := []struct {
 		path        string

internal/server/url.go

@@ -0,0 +1,28 @@
+package server
+
+import "strings"
+
+// splitRepoPath parses a /repos/... URL path into the full repo name and the
+// endpoint string that follows the "/-/" separator. It is the single source of
+// truth for the "/-/" URL format; both parseRepoPath (handlers.go) and
+// repoAndSubPath (middleware.go) delegate here.
+//
+//	/repos/acme/myrepo/-/tree        → ("acme/myrepo", "tree", true)
+//	/repos/acme/team/sub/-/commit    → ("acme/team/sub", "commit", true)
+//	/repos/acme/myrepo               → ("acme/myrepo", "", true)  // bare repo, no endpoint
+//	/something/else                  → ("", "", false)
+func splitRepoPath(path string) (repoName, endpoint string, ok bool) {
+	const prefix = "/repos/"
+	if !strings.HasPrefix(path, prefix) {
+		return "", "", false
+	}
+	rest := path[len(prefix):]
+	if rest == "" {
+		return "", "", false
+	}
+	repoName, endpoint, found := strings.Cut(rest, "/-/")
+	if !found {
+		return rest, "", true
+	}
+	return repoName, endpoint, true
+}