Skip to content

Commit 64540de

Browse files
239573049239573049
authored
Merge pull request #397 from jirisacha/fix/mcp-workspace-branch-path
fix(repositories): read workspaces from the branch-scoped layout
2 parents ee332f0 + c7d40a5 commit 64540de

7 files changed

Lines changed: 235 additions & 33 deletions

File tree

‎src/OpenDeepWiki/MCP/McpRepositoryTools.cs‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public static async Task<string> SearchDoc(
7070
return JsonSerializer.Serialize(new { error = true, message = $"No documentation in language '{language}'" });
7171

7272
var tools = new List<AITool>();
73-
var repoPath = BuildRepositoryPath(repoOptions.Value, resolvedOwner!, resolvedName!);
73+
var repoPath = RepositoryWorkspacePath.Resolve(
74+
repoOptions.Value, resolvedOwner!, resolvedName!, branch.BranchName);
7475
if (Directory.Exists(repoPath))
7576
{
7677
try
@@ -184,7 +185,13 @@ public static async Task<string> GetRepoStructure(
184185
if (repository == null)
185186
return JsonSerializer.Serialize(new { error = true, message = $"Repository {resolvedOwner}/{resolvedName} not found" });
186187

187-
var repoPath = BuildRepositoryPath(repoOptions.Value, resolvedOwner!, resolvedName!);
188+
// The MCP scope carries owner/repo only, so the branch owning the workspace has to
189+
// come from the database.
190+
var branch = await context.RepositoryBranches
191+
.FirstOrDefaultAsync(b => b.RepositoryId == repository.Id && !b.IsDeleted, cancellationToken);
192+
193+
var repoPath = RepositoryWorkspacePath.Resolve(
194+
repoOptions.Value, resolvedOwner!, resolvedName!, branch?.BranchName);
188195
if (!Directory.Exists(repoPath))
189196
return JsonSerializer.Serialize(new { error = true, message = "Repository workspace not found on server" });
190197

@@ -236,7 +243,13 @@ public static async Task<string> ReadFile(
236243
if (repository == null)
237244
return JsonSerializer.Serialize(new { error = true, message = $"Repository {resolvedOwner}/{resolvedName} not found" });
238245

239-
var repoPath = BuildRepositoryPath(repoOptions.Value, resolvedOwner!, resolvedName!);
246+
// The MCP scope carries owner/repo only, so the branch owning the workspace has to
247+
// come from the database.
248+
var branch = await context.RepositoryBranches
249+
.FirstOrDefaultAsync(b => b.RepositoryId == repository.Id && !b.IsDeleted, cancellationToken);
250+
251+
var repoPath = RepositoryWorkspacePath.Resolve(
252+
repoOptions.Value, resolvedOwner!, resolvedName!, branch?.BranchName);
240253
if (!Directory.Exists(repoPath))
241254
return JsonSerializer.Serialize(new { error = true, message = "Repository workspace not found on server" });
242255

@@ -421,24 +434,6 @@ private static AiRequestType ParseRequestType(string? provider)
421434
};
422435
}
423436

424-
private static string BuildRepositoryPath(RepositoryAnalyzerOptions options, string owner, string repo)
425-
{
426-
var safeOwner = SanitizePathComponent(owner);
427-
var safeRepo = SanitizePathComponent(repo);
428-
return Path.Combine(options.RepositoriesDirectory, safeOwner, safeRepo, "tree");
429-
}
430-
431-
private static string SanitizePathComponent(string component)
432-
{
433-
var sanitized = component
434-
.Replace('/', '_')
435-
.Replace('\\', '_')
436-
.Replace("..", "_")
437-
.Trim();
438-
439-
return string.IsNullOrWhiteSpace(sanitized) ? "_" : sanitized;
440-
}
441-
442437
private static string NormalizeRelativePath(string? path)
443438
{
444439
if (string.IsNullOrWhiteSpace(path)) return string.Empty;

‎src/OpenDeepWiki/Services/Chat/ChatAssistantService.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ private async IAsyncEnumerable<SSEEvent> InternalStreamChatAsync(
465465
var tools = new List<AITool>();
466466

467467
// Calculate repository path from Owner/Repo
468-
var repositoryPath = GetRepositoryPath(request.Context.Owner, request.Context.Repo);
468+
var repositoryPath = GetRepositoryPath(request.Context.Owner, request.Context.Repo, request.Context.Branch);
469469

470470
// Initialize GitTool with calculated repository path
471471
GitTool? gitTool = null;
@@ -1252,12 +1252,12 @@ private static AiRequestType ParseRequestType(string provider)
12521252
}
12531253

12541254
/// <summary>
1255-
/// Gets the repository working directory path based on owner and repo name.
1256-
/// The repository is cloned to {RepositoriesDirectory}/{org}/{repo}/tree/
1255+
/// Gets the repository working directory path based on owner, repo name and branch.
1256+
/// The repository is cloned to {RepositoriesDirectory}/{org}/{repo}/branches/{branch}/tree/
12571257
/// </summary>
1258-
private string GetRepositoryPath(string owner, string repo)
1258+
private string GetRepositoryPath(string owner, string repo, string? branch)
12591259
{
1260-
return Path.Combine(_repoOptions.RepositoriesDirectory, owner, repo, "tree");
1260+
return RepositoryWorkspacePath.Resolve(_repoOptions, owner, repo, branch);
12611261
}
12621262

12631263
/// <summary>

‎src/OpenDeepWiki/Services/Chat/EmbedService.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ public async IAsyncEnumerable<SSEEvent> StreamEmbedChatAsync(
340340

341341
if (!string.IsNullOrWhiteSpace(request.Owner) && !string.IsNullOrWhiteSpace(request.Repo))
342342
{
343-
var repositoryPath = GetRepositoryPath(request.Owner, request.Repo);
343+
var repositoryPath = GetRepositoryPath(request.Owner, request.Repo, request.Branch);
344344
if (Directory.Exists(repositoryPath))
345345
{
346346
try
@@ -822,11 +822,12 @@ private static string BuildEnhancedSystemPrompt(
822822
}
823823

824824
/// <summary>
825-
/// Gets the repository working directory path based on owner and repo name.
825+
/// Gets the repository working directory path based on owner, repo name and branch.
826+
/// The branch is optional here; the workspace is then resolved without it.
826827
/// </summary>
827-
private string GetRepositoryPath(string owner, string repo)
828+
private string GetRepositoryPath(string owner, string repo, string? branch)
828829
{
829-
return Path.Combine(_repoOptions.RepositoriesDirectory, owner, repo, "tree");
830+
return RepositoryWorkspacePath.Resolve(_repoOptions, owner, repo, branch);
830831
}
831832

832833
/// <summary>

‎src/OpenDeepWiki/Services/Repositories/IRepositoryAnalyzer.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public class RepositoryWorkspace
7474
{
7575
/// <summary>
7676
/// The absolute path to the working directory containing the repository files.
77-
/// Format: /data/{organization}/{name}/tree/
77+
/// Format: /data/{organization}/{name}/branches/{branch}/tree/
7878
/// </summary>
7979
public string WorkingDirectory { get; set; } = string.Empty;
8080

‎src/OpenDeepWiki/Services/Repositories/RepositoryAnalyzer.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,12 +351,14 @@ public Task<string[]> GetChangedFilesAsync(
351351
/// </summary>
352352
private string GetWorkingDirectory(string organization, string repositoryName, string branchName)
353353
{
354-
// Sanitize organization and repository names to prevent path traversal
354+
// Sanitize organization and repository names to prevent path traversal. Unlike the
355+
// readers, the analyzer creates the workspace, so an unusable component must throw
356+
// instead of falling back to a placeholder.
355357
var safeOrg = SanitizePathComponent(organization);
356358
var safeRepo = SanitizePathComponent(repositoryName);
357359
var safeBranch = SanitizePathComponent(branchName);
358360

359-
return Path.Combine(_options.RepositoriesDirectory, safeOrg, safeRepo, "branches", safeBranch, "tree");
361+
return RepositoryWorkspacePath.ForBranch(_options, safeOrg, safeRepo, safeBranch);
360362
}
361363

362364
private async Task PrepareArchiveWorkspaceAsync(

‎src/OpenDeepWiki/Services/Repositories/RepositoryWorkspacePath.cs‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
namespace OpenDeepWiki.Services.Repositories;
2+
3+
/// <summary>
4+
/// Resolves the on-disk workspace (source checkout) of a repository.
5+
/// Layout written by <see cref="RepositoryAnalyzer"/>:
6+
/// {RepositoriesDirectory}/{organization}/{name}/branches/{branch}/tree/
7+
/// </summary>
8+
public static class RepositoryWorkspacePath
9+
{
10+
private const string BranchesDirectoryName = "branches";
11+
private const string TreeDirectoryName = "tree";
12+
13+
/// <summary>
14+
/// Gets the workspace path of a specific branch, whether or not it exists on disk.
15+
/// </summary>
16+
public static string ForBranch(
17+
RepositoryAnalyzerOptions options,
18+
string organization,
19+
string repositoryName,
20+
string branchName)
21+
{
22+
return Path.Combine(
23+
RepositoryRoot(options, organization, repositoryName),
24+
BranchesDirectoryName,
25+
Sanitize(branchName),
26+
TreeDirectoryName);
27+
}
28+
29+
/// <summary>
30+
/// Resolves the best workspace available to a reader (MCP tools, chat services).
31+
/// Prefers the requested branch, then any other branch workspace, and finally the
32+
/// flat layout used before branch workspaces were introduced. The returned path is
33+
/// not guaranteed to exist -- callers still have to check.
34+
/// </summary>
35+
public static string Resolve(
36+
RepositoryAnalyzerOptions options,
37+
string organization,
38+
string repositoryName,
39+
string? branchName)
40+
{
41+
var repositoryRoot = RepositoryRoot(options, organization, repositoryName);
42+
var hasBranch = !string.IsNullOrWhiteSpace(branchName);
43+
44+
var branchWorkspace = hasBranch
45+
? Path.Combine(repositoryRoot, BranchesDirectoryName, Sanitize(branchName!), TreeDirectoryName)
46+
: null;
47+
48+
if (branchWorkspace != null && Directory.Exists(branchWorkspace))
49+
{
50+
return branchWorkspace;
51+
}
52+
53+
// Callers without branch context (e.g. the MCP scope carries owner/repo only)
54+
// fall back to the most recently written branch workspace.
55+
if (!hasBranch && NewestBranchWorkspace(repositoryRoot) is { } newestWorkspace)
56+
{
57+
return newestWorkspace;
58+
}
59+
60+
var legacyWorkspace = Path.Combine(repositoryRoot, TreeDirectoryName);
61+
if (Directory.Exists(legacyWorkspace))
62+
{
63+
return legacyWorkspace;
64+
}
65+
66+
return branchWorkspace ?? legacyWorkspace;
67+
}
68+
69+
private static string RepositoryRoot(
70+
RepositoryAnalyzerOptions options,
71+
string organization,
72+
string repositoryName)
73+
{
74+
return Path.Combine(options.RepositoriesDirectory, Sanitize(organization), Sanitize(repositoryName));
75+
}
76+
77+
private static string? NewestBranchWorkspace(string repositoryRoot)
78+
{
79+
var branchesRoot = Path.Combine(repositoryRoot, BranchesDirectoryName);
80+
if (!Directory.Exists(branchesRoot))
81+
{
82+
return null;
83+
}
84+
85+
return Directory.EnumerateDirectories(branchesRoot)
86+
.Select(branchDirectory => Path.Combine(branchDirectory, TreeDirectoryName))
87+
.Where(Directory.Exists)
88+
.OrderByDescending(Directory.GetLastWriteTimeUtc)
89+
.FirstOrDefault();
90+
}
91+
92+
/// <summary>
93+
/// Strips path separators and traversal sequences from a path component. Readers use a
94+
/// placeholder for empty input instead of throwing, because they resolve untrusted
95+
/// owner/repo values coming straight from a request.
96+
/// </summary>
97+
private static string Sanitize(string? component)
98+
{
99+
var sanitized = (component ?? string.Empty)
100+
.Replace('/', '_')
101+
.Replace('\\', '_')
102+
.Replace("..", "_")
103+
.Trim();
104+
105+
return string.IsNullOrWhiteSpace(sanitized) ? "_" : sanitized;
106+
}
107+
}

‎tests/OpenDeepWiki.Tests/Services/Repositories/RepositoryWorkspacePathTests.cs‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
using OpenDeepWiki.Services.Repositories;
2+
using Xunit;
3+
4+
namespace OpenDeepWiki.Tests.Services.Repositories;
5+
6+
public class RepositoryWorkspacePathTests
7+
{
8+
[Fact]
9+
public void ForBranch_ShouldUseBranchScopedLayout()
10+
{
11+
var options = CreateOptions(CreateTempDirectory());
12+
13+
var path = RepositoryWorkspacePath.ForBranch(options, "acme", "widgets", "main");
14+
15+
Assert.Equal(
16+
Path.Combine(options.RepositoriesDirectory, "acme", "widgets", "branches", "main", "tree"),
17+
path);
18+
}
19+
20+
[Fact]
21+
public void ForBranch_ShouldStripPathTraversalFromComponents()
22+
{
23+
var options = CreateOptions(CreateTempDirectory());
24+
25+
var path = RepositoryWorkspacePath.ForBranch(options, "../etc", "wid/gets", @"feature\x");
26+
27+
// Separators become underscores first, so "../etc" ends up as "__etc".
28+
Assert.Equal(
29+
Path.Combine(options.RepositoriesDirectory, "__etc", "wid_gets", "branches", "feature_x", "tree"),
30+
path);
31+
}
32+
33+
[Fact]
34+
public void Resolve_ShouldReturnRequestedBranchWorkspace()
35+
{
36+
var repositoriesRoot = CreateTempDirectory();
37+
var options = CreateOptions(repositoriesRoot);
38+
var expected = CreateWorkspace(repositoriesRoot, "acme", "widgets", "main");
39+
CreateWorkspace(repositoriesRoot, "acme", "widgets", "develop");
40+
41+
Assert.Equal(expected, RepositoryWorkspacePath.Resolve(options, "acme", "widgets", "main"));
42+
}
43+
44+
[Fact]
45+
public void Resolve_WithoutBranch_ShouldReturnMostRecentlyWrittenBranchWorkspace()
46+
{
47+
var repositoriesRoot = CreateTempDirectory();
48+
var options = CreateOptions(repositoriesRoot);
49+
var stale = CreateWorkspace(repositoriesRoot, "acme", "widgets", "develop");
50+
var recent = CreateWorkspace(repositoriesRoot, "acme", "widgets", "main");
51+
Directory.SetLastWriteTimeUtc(stale, DateTime.UtcNow.AddDays(-1));
52+
Directory.SetLastWriteTimeUtc(recent, DateTime.UtcNow);
53+
54+
Assert.Equal(recent, RepositoryWorkspacePath.Resolve(options, "acme", "widgets", branchName: null));
55+
}
56+
57+
[Fact]
58+
public void Resolve_ShouldFallBackToLegacyFlatWorkspace()
59+
{
60+
var repositoriesRoot = CreateTempDirectory();
61+
var options = CreateOptions(repositoriesRoot);
62+
var legacy = Path.Combine(repositoriesRoot, "acme", "widgets", "tree");
63+
Directory.CreateDirectory(legacy);
64+
65+
Assert.Equal(legacy, RepositoryWorkspacePath.Resolve(options, "acme", "widgets", "main"));
66+
}
67+
68+
[Fact]
69+
public void Resolve_WhenNothingExists_ShouldReturnBranchWorkspacePath()
70+
{
71+
var options = CreateOptions(CreateTempDirectory());
72+
73+
var path = RepositoryWorkspacePath.Resolve(options, "acme", "widgets", "main");
74+
75+
Assert.Equal(RepositoryWorkspacePath.ForBranch(options, "acme", "widgets", "main"), path);
76+
Assert.False(Directory.Exists(path));
77+
}
78+
79+
private static RepositoryAnalyzerOptions CreateOptions(string repositoriesRoot)
80+
{
81+
return new RepositoryAnalyzerOptions { RepositoriesDirectory = repositoriesRoot };
82+
}
83+
84+
private static string CreateWorkspace(string repositoriesRoot, string organization, string repository, string branch)
85+
{
86+
var path = Path.Combine(repositoriesRoot, organization, repository, "branches", branch, "tree");
87+
Directory.CreateDirectory(path);
88+
return path;
89+
}
90+
91+
private static string CreateTempDirectory()
92+
{
93+
var path = Path.Combine(Path.GetTempPath(), "OpenDeepWiki.Tests", Guid.NewGuid().ToString("N"));
94+
Directory.CreateDirectory(path);
95+
return path;
96+
}
97+
}

0 commit comments

Comments
 (0)