From c7d40a5c70d58463e995d3ead273282a6998074d Mon Sep 17 00:00:00 2001 From: Jiri Sacha Date: Wed, 2 Sep 2026 09:30:01 +0200 Subject: [PATCH] fix(repositories): read workspaces from the branch-scoped layout Since 0254c3c ("fix: isolate branch generation workspaces") repositories are cloned to {RepositoriesDirectory}/{org}/{repo}/branches/{branch}/tree, but every reader still resolved the pre-existing flat path {RepositoriesDirectory}/{org}/ {repo}/tree, which nothing writes any more: - McpRepositoryTools.ReadFile and GetRepoStructure always answer "Repository workspace not found on server". - McpRepositoryTools.SearchDoc silently attaches no GitTool, so its summaries never see the source. - ChatAssistantService and EmbedService fall back to a documentation-only chat for the same reason. Add RepositoryWorkspacePath, a single resolver shared by the analyzer and the readers, and use it everywhere. It prefers the requested branch workspace, falls back to the most recently written one when the caller has no branch context, and finally to the legacy flat layout so workspaces created before 0254c3c keep working. --- src/OpenDeepWiki/MCP/McpRepositoryTools.cs | 37 +++--- .../Services/Chat/ChatAssistantService.cs | 10 +- .../Services/Chat/EmbedService.cs | 9 +- .../Repositories/IRepositoryAnalyzer.cs | 2 +- .../Repositories/RepositoryAnalyzer.cs | 6 +- .../Repositories/RepositoryWorkspacePath.cs | 107 ++++++++++++++++++ .../RepositoryWorkspacePathTests.cs | 97 ++++++++++++++++ 7 files changed, 235 insertions(+), 33 deletions(-) create mode 100644 src/OpenDeepWiki/Services/Repositories/RepositoryWorkspacePath.cs create mode 100644 tests/OpenDeepWiki.Tests/Services/Repositories/RepositoryWorkspacePathTests.cs diff --git a/src/OpenDeepWiki/MCP/McpRepositoryTools.cs b/src/OpenDeepWiki/MCP/McpRepositoryTools.cs index 4fa48957..9a3c1ae5 100644 --- a/src/OpenDeepWiki/MCP/McpRepositoryTools.cs +++ b/src/OpenDeepWiki/MCP/McpRepositoryTools.cs @@ -70,7 +70,8 @@ public static async Task SearchDoc( return JsonSerializer.Serialize(new { error = true, message = $"No documentation in language '{language}'" }); var tools = new List(); - var repoPath = BuildRepositoryPath(repoOptions.Value, resolvedOwner!, resolvedName!); + var repoPath = RepositoryWorkspacePath.Resolve( + repoOptions.Value, resolvedOwner!, resolvedName!, branch.BranchName); if (Directory.Exists(repoPath)) { try @@ -184,7 +185,13 @@ public static async Task GetRepoStructure( if (repository == null) return JsonSerializer.Serialize(new { error = true, message = $"Repository {resolvedOwner}/{resolvedName} not found" }); - var repoPath = BuildRepositoryPath(repoOptions.Value, resolvedOwner!, resolvedName!); + // The MCP scope carries owner/repo only, so the branch owning the workspace has to + // come from the database. + var branch = await context.RepositoryBranches + .FirstOrDefaultAsync(b => b.RepositoryId == repository.Id && !b.IsDeleted, cancellationToken); + + var repoPath = RepositoryWorkspacePath.Resolve( + repoOptions.Value, resolvedOwner!, resolvedName!, branch?.BranchName); if (!Directory.Exists(repoPath)) return JsonSerializer.Serialize(new { error = true, message = "Repository workspace not found on server" }); @@ -236,7 +243,13 @@ public static async Task ReadFile( if (repository == null) return JsonSerializer.Serialize(new { error = true, message = $"Repository {resolvedOwner}/{resolvedName} not found" }); - var repoPath = BuildRepositoryPath(repoOptions.Value, resolvedOwner!, resolvedName!); + // The MCP scope carries owner/repo only, so the branch owning the workspace has to + // come from the database. + var branch = await context.RepositoryBranches + .FirstOrDefaultAsync(b => b.RepositoryId == repository.Id && !b.IsDeleted, cancellationToken); + + var repoPath = RepositoryWorkspacePath.Resolve( + repoOptions.Value, resolvedOwner!, resolvedName!, branch?.BranchName); if (!Directory.Exists(repoPath)) return JsonSerializer.Serialize(new { error = true, message = "Repository workspace not found on server" }); @@ -421,24 +434,6 @@ private static AiRequestType ParseRequestType(string? provider) }; } - private static string BuildRepositoryPath(RepositoryAnalyzerOptions options, string owner, string repo) - { - var safeOwner = SanitizePathComponent(owner); - var safeRepo = SanitizePathComponent(repo); - return Path.Combine(options.RepositoriesDirectory, safeOwner, safeRepo, "tree"); - } - - private static string SanitizePathComponent(string component) - { - var sanitized = component - .Replace('/', '_') - .Replace('\\', '_') - .Replace("..", "_") - .Trim(); - - return string.IsNullOrWhiteSpace(sanitized) ? "_" : sanitized; - } - private static string NormalizeRelativePath(string? path) { if (string.IsNullOrWhiteSpace(path)) return string.Empty; diff --git a/src/OpenDeepWiki/Services/Chat/ChatAssistantService.cs b/src/OpenDeepWiki/Services/Chat/ChatAssistantService.cs index 9611cec1..192db0aa 100644 --- a/src/OpenDeepWiki/Services/Chat/ChatAssistantService.cs +++ b/src/OpenDeepWiki/Services/Chat/ChatAssistantService.cs @@ -465,7 +465,7 @@ private async IAsyncEnumerable InternalStreamChatAsync( var tools = new List(); // Calculate repository path from Owner/Repo - var repositoryPath = GetRepositoryPath(request.Context.Owner, request.Context.Repo); + var repositoryPath = GetRepositoryPath(request.Context.Owner, request.Context.Repo, request.Context.Branch); // Initialize GitTool with calculated repository path GitTool? gitTool = null; @@ -1252,12 +1252,12 @@ private static AiRequestType ParseRequestType(string provider) } /// - /// Gets the repository working directory path based on owner and repo name. - /// The repository is cloned to {RepositoriesDirectory}/{org}/{repo}/tree/ + /// Gets the repository working directory path based on owner, repo name and branch. + /// The repository is cloned to {RepositoriesDirectory}/{org}/{repo}/branches/{branch}/tree/ /// - private string GetRepositoryPath(string owner, string repo) + private string GetRepositoryPath(string owner, string repo, string? branch) { - return Path.Combine(_repoOptions.RepositoriesDirectory, owner, repo, "tree"); + return RepositoryWorkspacePath.Resolve(_repoOptions, owner, repo, branch); } /// diff --git a/src/OpenDeepWiki/Services/Chat/EmbedService.cs b/src/OpenDeepWiki/Services/Chat/EmbedService.cs index 7f29664c..a47ceb50 100644 --- a/src/OpenDeepWiki/Services/Chat/EmbedService.cs +++ b/src/OpenDeepWiki/Services/Chat/EmbedService.cs @@ -340,7 +340,7 @@ public async IAsyncEnumerable StreamEmbedChatAsync( if (!string.IsNullOrWhiteSpace(request.Owner) && !string.IsNullOrWhiteSpace(request.Repo)) { - var repositoryPath = GetRepositoryPath(request.Owner, request.Repo); + var repositoryPath = GetRepositoryPath(request.Owner, request.Repo, request.Branch); if (Directory.Exists(repositoryPath)) { try @@ -822,11 +822,12 @@ private static string BuildEnhancedSystemPrompt( } /// - /// Gets the repository working directory path based on owner and repo name. + /// Gets the repository working directory path based on owner, repo name and branch. + /// The branch is optional here; the workspace is then resolved without it. /// - private string GetRepositoryPath(string owner, string repo) + private string GetRepositoryPath(string owner, string repo, string? branch) { - return Path.Combine(_repoOptions.RepositoriesDirectory, owner, repo, "tree"); + return RepositoryWorkspacePath.Resolve(_repoOptions, owner, repo, branch); } /// diff --git a/src/OpenDeepWiki/Services/Repositories/IRepositoryAnalyzer.cs b/src/OpenDeepWiki/Services/Repositories/IRepositoryAnalyzer.cs index 19d7f56b..80c9d141 100644 --- a/src/OpenDeepWiki/Services/Repositories/IRepositoryAnalyzer.cs +++ b/src/OpenDeepWiki/Services/Repositories/IRepositoryAnalyzer.cs @@ -74,7 +74,7 @@ public class RepositoryWorkspace { /// /// The absolute path to the working directory containing the repository files. - /// Format: /data/{organization}/{name}/tree/ + /// Format: /data/{organization}/{name}/branches/{branch}/tree/ /// public string WorkingDirectory { get; set; } = string.Empty; diff --git a/src/OpenDeepWiki/Services/Repositories/RepositoryAnalyzer.cs b/src/OpenDeepWiki/Services/Repositories/RepositoryAnalyzer.cs index 466012af..e6ec2de1 100644 --- a/src/OpenDeepWiki/Services/Repositories/RepositoryAnalyzer.cs +++ b/src/OpenDeepWiki/Services/Repositories/RepositoryAnalyzer.cs @@ -351,12 +351,14 @@ public Task GetChangedFilesAsync( /// private string GetWorkingDirectory(string organization, string repositoryName, string branchName) { - // Sanitize organization and repository names to prevent path traversal + // Sanitize organization and repository names to prevent path traversal. Unlike the + // readers, the analyzer creates the workspace, so an unusable component must throw + // instead of falling back to a placeholder. var safeOrg = SanitizePathComponent(organization); var safeRepo = SanitizePathComponent(repositoryName); var safeBranch = SanitizePathComponent(branchName); - return Path.Combine(_options.RepositoriesDirectory, safeOrg, safeRepo, "branches", safeBranch, "tree"); + return RepositoryWorkspacePath.ForBranch(_options, safeOrg, safeRepo, safeBranch); } private async Task PrepareArchiveWorkspaceAsync( diff --git a/src/OpenDeepWiki/Services/Repositories/RepositoryWorkspacePath.cs b/src/OpenDeepWiki/Services/Repositories/RepositoryWorkspacePath.cs new file mode 100644 index 00000000..6fe5f4e3 --- /dev/null +++ b/src/OpenDeepWiki/Services/Repositories/RepositoryWorkspacePath.cs @@ -0,0 +1,107 @@ +namespace OpenDeepWiki.Services.Repositories; + +/// +/// Resolves the on-disk workspace (source checkout) of a repository. +/// Layout written by : +/// {RepositoriesDirectory}/{organization}/{name}/branches/{branch}/tree/ +/// +public static class RepositoryWorkspacePath +{ + private const string BranchesDirectoryName = "branches"; + private const string TreeDirectoryName = "tree"; + + /// + /// Gets the workspace path of a specific branch, whether or not it exists on disk. + /// + public static string ForBranch( + RepositoryAnalyzerOptions options, + string organization, + string repositoryName, + string branchName) + { + return Path.Combine( + RepositoryRoot(options, organization, repositoryName), + BranchesDirectoryName, + Sanitize(branchName), + TreeDirectoryName); + } + + /// + /// Resolves the best workspace available to a reader (MCP tools, chat services). + /// Prefers the requested branch, then any other branch workspace, and finally the + /// flat layout used before branch workspaces were introduced. The returned path is + /// not guaranteed to exist -- callers still have to check. + /// + public static string Resolve( + RepositoryAnalyzerOptions options, + string organization, + string repositoryName, + string? branchName) + { + var repositoryRoot = RepositoryRoot(options, organization, repositoryName); + var hasBranch = !string.IsNullOrWhiteSpace(branchName); + + var branchWorkspace = hasBranch + ? Path.Combine(repositoryRoot, BranchesDirectoryName, Sanitize(branchName!), TreeDirectoryName) + : null; + + if (branchWorkspace != null && Directory.Exists(branchWorkspace)) + { + return branchWorkspace; + } + + // Callers without branch context (e.g. the MCP scope carries owner/repo only) + // fall back to the most recently written branch workspace. + if (!hasBranch && NewestBranchWorkspace(repositoryRoot) is { } newestWorkspace) + { + return newestWorkspace; + } + + var legacyWorkspace = Path.Combine(repositoryRoot, TreeDirectoryName); + if (Directory.Exists(legacyWorkspace)) + { + return legacyWorkspace; + } + + return branchWorkspace ?? legacyWorkspace; + } + + private static string RepositoryRoot( + RepositoryAnalyzerOptions options, + string organization, + string repositoryName) + { + return Path.Combine(options.RepositoriesDirectory, Sanitize(organization), Sanitize(repositoryName)); + } + + private static string? NewestBranchWorkspace(string repositoryRoot) + { + var branchesRoot = Path.Combine(repositoryRoot, BranchesDirectoryName); + if (!Directory.Exists(branchesRoot)) + { + return null; + } + + return Directory.EnumerateDirectories(branchesRoot) + .Select(branchDirectory => Path.Combine(branchDirectory, TreeDirectoryName)) + .Where(Directory.Exists) + .OrderByDescending(Directory.GetLastWriteTimeUtc) + .FirstOrDefault(); + } + + /// + /// Strips path separators and traversal sequences from a path component. Readers use a + /// placeholder for empty input instead of throwing, because they resolve untrusted + /// owner/repo values coming straight from a request. + /// + private static string Sanitize(string? component) + { + var sanitized = (component ?? string.Empty) + .Replace('/', '_') + .Replace('\\', '_') + .Replace("..", "_") + .Trim(); + + return string.IsNullOrWhiteSpace(sanitized) ? "_" : sanitized; + } +} diff --git a/tests/OpenDeepWiki.Tests/Services/Repositories/RepositoryWorkspacePathTests.cs b/tests/OpenDeepWiki.Tests/Services/Repositories/RepositoryWorkspacePathTests.cs new file mode 100644 index 00000000..1a997f87 --- /dev/null +++ b/tests/OpenDeepWiki.Tests/Services/Repositories/RepositoryWorkspacePathTests.cs @@ -0,0 +1,97 @@ +using OpenDeepWiki.Services.Repositories; +using Xunit; + +namespace OpenDeepWiki.Tests.Services.Repositories; + +public class RepositoryWorkspacePathTests +{ + [Fact] + public void ForBranch_ShouldUseBranchScopedLayout() + { + var options = CreateOptions(CreateTempDirectory()); + + var path = RepositoryWorkspacePath.ForBranch(options, "acme", "widgets", "main"); + + Assert.Equal( + Path.Combine(options.RepositoriesDirectory, "acme", "widgets", "branches", "main", "tree"), + path); + } + + [Fact] + public void ForBranch_ShouldStripPathTraversalFromComponents() + { + var options = CreateOptions(CreateTempDirectory()); + + var path = RepositoryWorkspacePath.ForBranch(options, "../etc", "wid/gets", @"feature\x"); + + // Separators become underscores first, so "../etc" ends up as "__etc". + Assert.Equal( + Path.Combine(options.RepositoriesDirectory, "__etc", "wid_gets", "branches", "feature_x", "tree"), + path); + } + + [Fact] + public void Resolve_ShouldReturnRequestedBranchWorkspace() + { + var repositoriesRoot = CreateTempDirectory(); + var options = CreateOptions(repositoriesRoot); + var expected = CreateWorkspace(repositoriesRoot, "acme", "widgets", "main"); + CreateWorkspace(repositoriesRoot, "acme", "widgets", "develop"); + + Assert.Equal(expected, RepositoryWorkspacePath.Resolve(options, "acme", "widgets", "main")); + } + + [Fact] + public void Resolve_WithoutBranch_ShouldReturnMostRecentlyWrittenBranchWorkspace() + { + var repositoriesRoot = CreateTempDirectory(); + var options = CreateOptions(repositoriesRoot); + var stale = CreateWorkspace(repositoriesRoot, "acme", "widgets", "develop"); + var recent = CreateWorkspace(repositoriesRoot, "acme", "widgets", "main"); + Directory.SetLastWriteTimeUtc(stale, DateTime.UtcNow.AddDays(-1)); + Directory.SetLastWriteTimeUtc(recent, DateTime.UtcNow); + + Assert.Equal(recent, RepositoryWorkspacePath.Resolve(options, "acme", "widgets", branchName: null)); + } + + [Fact] + public void Resolve_ShouldFallBackToLegacyFlatWorkspace() + { + var repositoriesRoot = CreateTempDirectory(); + var options = CreateOptions(repositoriesRoot); + var legacy = Path.Combine(repositoriesRoot, "acme", "widgets", "tree"); + Directory.CreateDirectory(legacy); + + Assert.Equal(legacy, RepositoryWorkspacePath.Resolve(options, "acme", "widgets", "main")); + } + + [Fact] + public void Resolve_WhenNothingExists_ShouldReturnBranchWorkspacePath() + { + var options = CreateOptions(CreateTempDirectory()); + + var path = RepositoryWorkspacePath.Resolve(options, "acme", "widgets", "main"); + + Assert.Equal(RepositoryWorkspacePath.ForBranch(options, "acme", "widgets", "main"), path); + Assert.False(Directory.Exists(path)); + } + + private static RepositoryAnalyzerOptions CreateOptions(string repositoriesRoot) + { + return new RepositoryAnalyzerOptions { RepositoriesDirectory = repositoriesRoot }; + } + + private static string CreateWorkspace(string repositoriesRoot, string organization, string repository, string branch) + { + var path = Path.Combine(repositoriesRoot, organization, repository, "branches", branch, "tree"); + Directory.CreateDirectory(path); + return path; + } + + private static string CreateTempDirectory() + { + var path = Path.Combine(Path.GetTempPath(), "OpenDeepWiki.Tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(path); + return path; + } +}