Skip to content

fix(control-plane): use full namespace path as GitLab repo owner #734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
duboff wants to merge 1 commit into from
Closed

fix(control-plane): use full namespace path as GitLab repo owner #734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 68 additions & 5 deletions packages/control-plane/src/source-control/providers/gitlab-provider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe("GitLabSourceControlProvider", () => {
name: "My Web App", // display name — should NOT be used
path: "web", // URL slug — should be used as name
path_with_namespace: "acme/web",
namespace: { path: "acme" },
namespace: { path: "acme", full_path: "acme" },
default_branch: "main",
visibility: "private",
})
Expand All @@ -59,14 +59,37 @@ describe("GitLabSourceControlProvider", () => {
});
});

it("returns the full namespace path as owner for nested-group projects", async () => {
mockFetch.mockResolvedValueOnce(
makeResponse({
id: 43,
name: "Web App",
path: "web",
path_with_namespace: "acme/backend/web",
namespace: { path: "backend", full_path: "acme/backend" },
default_branch: "main",
visibility: "private",
})
);

const provider = new GitLabSourceControlProvider(fakeConfig);
const repo = await provider.getRepository(
{ authType: "pat", token: "user-token" },
{ owner: "acme/backend", name: "web" }
);

expect(repo.owner).toBe("acme/backend");
expect(repo.fullName).toBe("acme/backend/web");
});

it("marks public repos as not private", async () => {
mockFetch.mockResolvedValueOnce(
makeResponse({
id: 7,
name: "OSS Project",
path: "oss",
path_with_namespace: "acme/oss",
namespace: { path: "acme" },
namespace: { path: "acme", full_path: "acme" },
default_branch: "main",
visibility: "public",
})
Expand Down Expand Up @@ -342,7 +365,7 @@ describe("GitLabSourceControlProvider", () => {
mockFetch.mockResolvedValueOnce(
makeResponse({
id: 99,
namespace: { path: "acme" },
namespace: { path: "acme", full_path: "acme" },
path: "web",
default_branch: "main",
})
Expand All @@ -359,6 +382,23 @@ describe("GitLabSourceControlProvider", () => {
});
});

it("returns the full namespace path as repoOwner for nested-group projects", async () => {
mockFetch.mockResolvedValueOnce(
makeResponse({
id: 100,
namespace: { path: "backend", full_path: "acme/backend" },
path: "web",
default_branch: "main",
})
);

const provider = new GitLabSourceControlProvider(fakeConfig);
const result = await provider.checkRepositoryAccess({ owner: "acme/backend", name: "web" });

expect(result?.repoOwner).toBe("acme/backend");
expect(result?.repoName).toBe("web");
});

it("returns null for 404", async () => {
mockFetch.mockResolvedValueOnce(makeResponse("not found", 404));

Expand All @@ -384,7 +424,7 @@ describe("GitLabSourceControlProvider", () => {
mockFetch.mockResolvedValueOnce(
makeResponse({
id: 1,
namespace: { path: "ACME" },
namespace: { path: "ACME", full_path: "ACME" },
path: "WEB",
default_branch: "main",
})
Expand All @@ -407,7 +447,7 @@ describe("GitLabSourceControlProvider", () => {
name: "My Web App", // display name — should NOT be used
path: "web", // URL slug — should be used as name
path_with_namespace: "acme/web",
namespace: { path: "acme" },
namespace: { path: "acme", full_path: "acme" },
description: "The web app",
visibility: "private",
default_branch: "main",
Expand All @@ -434,6 +474,29 @@ describe("GitLabSourceControlProvider", () => {
});
});

it("returns the full namespace path as owner for nested-group projects", async () => {
mockFetch.mockResolvedValueOnce(
makeResponse([
{
id: 2,
name: "Web App",
path: "web",
path_with_namespace: "acme/backend/web",
namespace: { path: "backend", full_path: "acme/backend" },
description: null,
visibility: "private",
default_branch: "main",
},
])
);

const provider = new GitLabSourceControlProvider(fakeConfig);
const repos = await provider.listRepositories();

expect(repos[0].owner).toBe("acme/backend");
expect(repos[0].fullName).toBe("acme/backend/web");
});

it("fetches from group endpoint when namespace is configured", async () => {
mockFetch.mockResolvedValueOnce(makeResponse([]));

Expand Down
14 changes: 8 additions & 6 deletions packages/control-plane/src/source-control/providers/gitlab-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,15 @@ export class GitLabSourceControlProvider implements SourceControlProvider {
name: string;
path: string;
path_with_namespace: string;
namespace: { path: string };
namespace: { full_path: string };
default_branch: string;
visibility: string;
};

// full_path, not path: nested groups ("group/subgroup") need the
// entire namespace so owner/name lookups reconstruct the project path.
return {
owner: data.namespace.path,
owner: data.namespace.full_path,
name: data.path,
Comment on lines +112 to 116

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Nested-group owner now breaks manual MR URL path composition.

Switching owner to namespace.full_path is correct, but this makes owner contain / for nested groups. buildManualPullRequestUrl currently URL-encodes owner as a single segment, which produces an invalid GitLab project path for nested owners.

💡 Proposed fix 
   buildManualPullRequestUrl(config: BuildManualPullRequestUrlConfig): string {
-    const encodedOwner = encodeURIComponent(config.owner);
+    const encodedOwner = config.owner
+      .split("/")
+      .map((segment) => encodeURIComponent(segment))
+      .join("/");
     const encodedName = encodeURIComponent(config.name);
     const encodedSource = encodeURIComponent(config.sourceBranch);
     const encodedTarget = encodeURIComponent(config.targetBranch);
     return (
       `https://gitlab.com/${encodedOwner}/${encodedName}/-/merge_requests/new` +
       `?merge_request[source_branch]=${encodedSource}` +
       `&merge_request[target_branch]=${encodedTarget}`
     );
   }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/control-plane/src/source-control/providers/gitlab-provider.ts`
around lines 112 - 116, The change setting owner to data.namespace.full_path
causes owner to include '/' for nested groups, which breaks
buildManualPullRequestUrl because it URL-encodes the whole owner as one segment;
update the URL construction to preserve path separators by splitting owner on
'/' and encode each segment separately (e.g.,
owner.split('/').map(encodeURIComponent).join('/')) and encode the project name
with encodeURIComponent when building the manual MR URL; locate the owner
assignment in gitlab-provider.ts (owner: data.namespace.full_path) and update
buildManualPullRequestUrl to apply per-segment encoding so nested-group paths
produce valid GitLab project URLs.

fullName: data.path_with_namespace,
defaultBranch: data.default_branch,
Expand Down Expand Up @@ -229,14 +231,14 @@ export class GitLabSourceControlProvider implements SourceControlProvider {

const data = (await response.json()) as {
id: number;
namespace: { path: string };
namespace: { full_path: string };
path: string;
default_branch: string;
};

return {
repoId: data.id,
repoOwner: data.namespace.path.toLowerCase(),
repoOwner: data.namespace.full_path.toLowerCase(),
repoName: data.path.toLowerCase(),
defaultBranch: data.default_branch,
};
Expand Down Expand Up @@ -281,15 +283,15 @@ export class GitLabSourceControlProvider implements SourceControlProvider {
name: string;
path: string;
path_with_namespace: string;
namespace: { path: string };
namespace: { full_path: string };
description: string | null;
visibility: string;
default_branch: string;
}>;

return data.map((project) => ({
id: project.id,
owner: project.namespace.path,
owner: project.namespace.full_path,
name: project.path,
fullName: project.path_with_namespace,
description: project.description,
Expand Down
Loading