Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions src/Maestro/Maestro.DataProviders/RemoteFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class RemoteFactory : IRemoteFactory
private readonly IGitHubTokenProvider _gitHubTokenProvider;
private readonly IAzureDevOpsTokenProvider _azdoTokenProvider;
private readonly IServiceProvider _serviceProvider;
private readonly IVersionDetailsParser _versionDetailsParser;

public RemoteFactory(
BuildAssetRegistryContext context,
Expand All @@ -45,6 +46,7 @@ public RemoteFactory(
_azdoTokenProvider = azdoTokenProvider;
_cache = memoryCache;
_serviceProvider = serviceProvider;
_versionDetailsParser = versionDetailsParser;
}

public async Task<IRemote> CreateRemoteAsync(string repoUrl)
Expand Down Expand Up @@ -87,6 +89,7 @@ private async Task<IRemoteGitRepo> GetRemoteGitClient(string repoUrl)
: new GitHubClient(
new Microsoft.DotNet.DarcLib.GitHubTokenProvider(_gitHubTokenProvider),
_processManager,
_versionDetailsParser,
_loggerFactory.CreateLogger<GitHubClient>(),
_cache.Cache),

Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,22 @@ internal class RemoteFactory : IRemoteFactory
private readonly IServiceProvider _serviceProvider;
private readonly IAzureDevOpsTokenProvider _azdoTokenProvider;
private readonly IRemoteTokenProvider _githubTokenProvider;
private readonly IVersionDetailsParser _versionDetailsParser;

public RemoteFactory(
IAzureDevOpsTokenProvider azdoTokenProvider,
[FromKeyedServices("github")]IRemoteTokenProvider githubTokenProvider,
ILoggerFactory loggerFactory,
ICommandLineOptions options,
IVersionDetailsParser versionDetailsParser,
IServiceProvider serviceProvider)
{
_loggerFactory = loggerFactory;
_options = options;
_serviceProvider = serviceProvider;
_azdoTokenProvider = azdoTokenProvider;
_githubTokenProvider = githubTokenProvider;
_versionDetailsParser = versionDetailsParser;
}

public Task<IRemote> CreateRemoteAsync(string repoUrl)
Expand Down Expand Up @@ -62,6 +65,7 @@ private IRemoteGitRepo CreateRemoteGitClient(ICommandLineOptions options, string
new ProcessManager(_loggerFactory.CreateLogger<IProcessManager>(), options.GitLocation),
_loggerFactory.CreateLogger<GitHubClient>(),
temporaryRepositoryRoot,
_versionDetailsParser,
// Caching not in use for Darc local client.
null),

Expand Down
17 changes: 17 additions & 0 deletions src/Microsoft.DotNet.Darc/DarcLib/AzureDevOpsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.DotNet.DarcLib.Helpers;
using Microsoft.DotNet.DarcLib.Models;
using Microsoft.DotNet.DarcLib.Models.AzureDevOps;
using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo;
using Microsoft.DotNet.Services.Utility;
using Microsoft.Extensions.Logging;
using Microsoft.TeamFoundation.SourceControl.WebApi;
Expand Down Expand Up @@ -1944,4 +1945,20 @@ public async Task<IReadOnlyCollection<string>> GetGitTreeNames(string path, stri
UpdatedAt = DateTimeOffset.UtcNow,
HeadBranchSha = pr.LastMergeSourceCommit.CommitId,
};

public Task<List<Commit>> FetchLatestRepoCommits(string repoUrl, string branch, int maxCount)
=> throw new NotImplementedException();

public Task<List<Commit>> FetchLatestFetchNewerRepoCommitsAsyncRepoCommits(
string repoUrl,
string branch,
string commitSha,
int maxCount)
=> throw new NotImplementedException();

Comment on lines +1949 to +1958
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

There are duplicate method definitions at lines 1949-1950 and 1959-1960. The methods FetchLatestRepoCommits/FetchLatestRepoCommitsAsync and the malformed method name are all duplicates. These duplicate stub implementations should be consolidated into the correctly named versions at lines 1959-1960.

Suggested change
public Task<List<Commit>> FetchLatestRepoCommits(string repoUrl, string branch, int maxCount)
=> throw new NotImplementedException();
public Task<List<Commit>> FetchLatestFetchNewerRepoCommitsAsyncRepoCommits(
string repoUrl,
string branch,
string commitSha,
int maxCount)
=> throw new NotImplementedException();

Copilot uses AI. Check for mistakes.
public Task<List<Commit>> FetchLatestRepoCommitsAsync(string repoUrl, string branch, int maxCount) => throw new NotImplementedException();
public Task<List<Commit>> FetchNewerRepoCommitsAsync(string repoUrl, string branch, string commitSha, int maxCount) => throw new NotImplementedException();
public Task<ForwardFlow> GetLastIncomingForwardFlowAsync(string vmrUrl, string commit) => throw new NotImplementedException();
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The method signature at line 1961 for GetLastIncomingForwardFlowAsync is missing the mappingName parameter that exists in the interface definition (line 186 of IRemoteGitRepo.cs). This signature mismatch will prevent the code from compiling.

Suggested change
public Task<ForwardFlow> GetLastIncomingForwardFlowAsync(string vmrUrl, string commit) => throw new NotImplementedException();

Copilot uses AI. Check for mistakes.
public Task<Backflow> GetLastIncomingBackflowAsync(string repoUrl, string commit) => throw new NotImplementedException();
public Task<ForwardFlow> GetLastIncomingForwardFlowAsync(string vmrUrl, string mappingName, string commit) => throw new NotImplementedException();
}
215 changes: 214 additions & 1 deletion src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@
using System.Net.Http;
using System.Reflection;
using System.Text;
using System.Text.Json;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Maestro.Common;
using Maestro.MergePolicyEvaluation;
using Microsoft.DotNet.DarcLib.Helpers;
using Microsoft.DotNet.DarcLib.Models;
using Microsoft.DotNet.DarcLib.Models.GitHub;
using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo;
using Microsoft.DotNet.DarcLib.VirtualMonoRepo;
using Microsoft.DotNet.Services.Utility;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -48,6 +51,7 @@ public class GitHubClient : RemoteRepoBase, IRemoteGitRepo
private readonly string _userAgent = $"DarcLib-{DarcLibVersion}";
private IGitHubClient? _lazyClient = null;
private readonly Dictionary<(string, string, string?), string> _gitRefCommitCache;
private readonly IVersionDetailsParser _versionDetailsParser;

static GitHubClient()
{
Expand All @@ -60,9 +64,10 @@ static GitHubClient()
public GitHubClient(
IRemoteTokenProvider remoteTokenProvider,
IProcessManager processManager,
IVersionDetailsParser versionDetailsParser,
ILogger logger,
IMemoryCache? cache)
: this(remoteTokenProvider, processManager, logger, null, cache)
: this(remoteTokenProvider, processManager, logger, null, versionDetailsParser, cache)
{
}

Expand All @@ -71,6 +76,7 @@ public GitHubClient(
IProcessManager processManager,
ILogger logger,
string? temporaryRepositoryPath,
IVersionDetailsParser versionDetailsParser,
IMemoryCache? cache)
: base(remoteTokenProvider, processManager, temporaryRepositoryPath, cache, logger)
{
Expand All @@ -82,6 +88,7 @@ public GitHubClient(
NullValueHandling = NullValueHandling.Ignore
};
_gitRefCommitCache = [];
_versionDetailsParser = versionDetailsParser;
}

public bool AllowRetries { get; set; } = true;
Expand Down Expand Up @@ -1491,4 +1498,210 @@ private static PullRequest ToDarcLibPullRequest(Octokit.PullRequest pr)
HeadBranchSha = pr.Head.Sha,
};
}

public Task<List<Commit>> FetchLatestRepoCommitsAsync(string repoUrl, string branch, int maxCount)
=> throw new NotImplementedException();

public async Task<List<Commit>> FetchNewerRepoCommitsAsync(
string repoUrl,
string branch,
string commitSha,
int maxCount)
{
if (maxCount <= 0)
{
maxCount = 100;
}

(string owner, string repo) = ParseRepoUri(repoUrl);

var request = new CommitRequest
{
Sha = branch,
};

var options = new ApiOptions
{
PageSize = maxCount,
PageCount = 1,
StartPage = 1
};

var allCommits = new List<Commit>();

while (allCommits.Count < maxCount)
{
var commits = await GetClient(owner, repo)
.Repository
.Commit
.GetAll(owner, repo, request, options);

foreach (Octokit.GitHubCommit c in commits)
{
var convertedCommit = new Commit(
c.Author?.Login,
c.Sha,
c.Commit.Message);

allCommits.Add(convertedCommit);

if (convertedCommit.Sha == commitSha)
{
break;
}
}
Comment on lines +1539 to +1552
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Copilot uses AI. Check for mistakes.

if (commits.Count < options.PageSize)
{
break;
}

options.StartPage++;
}
Comment on lines +1532 to +1560
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The pagination logic has a potential infinite loop issue. If commits.Count equals options.PageSize on every iteration but allCommits.Count never reaches commitSha match (line 1548), the loop at line 1532 will continue indefinitely until it reaches maxCount. There should be a safeguard to prevent fetching an unbounded number of pages if the target commit is never found.

Copilot uses AI. Check for mistakes.

return [.. allCommits.Take(maxCount)];
}

public async Task<ForwardFlow?> GetLastIncomingForwardFlowAsync(string vmrUrl, string mappingName, string commit)
{
var content = await GetFileContentAtCommit(
vmrUrl,
commit,
VmrInfo.DefaultRelativeSourceManifestPath);

var lastForwardFlowRepoSha = SourceManifest
.FromJson(content)?
.GetRepoVersion(mappingName)
.CommitSha;

if (lastForwardFlowRepoSha == null)
{
return null;
}

int lineNumber = content.Split('\n')
.ToList()
.FindIndex(line => line.Contains(lastForwardFlowRepoSha))
+ 1; // result is 0-indexed, but github lines are 1-indexed;

// todo: we can skip this call if the last flown SHA is one that we already cached
string lastForwardFlowVmrSha = await BlameLineAsync(
vmrUrl,
commit,
VmrInfo.DefaultRelativeSourceManifestPath,
lineNumber);

return new ForwardFlow(lastForwardFlowRepoSha, lastForwardFlowVmrSha);
}

public async Task<Backflow?> GetLastIncomingBackflowAsync(string repoUrl, string commit)
{
var content = await GetFileContentAtCommit(
repoUrl,
commit,
VersionFiles.VersionDetailsXml);

var lastBackflowVmrSha = _versionDetailsParser.ParseVersionDetailsXml(content)
.Source?
.Sha;

if (lastBackflowVmrSha == null)
{
return null;
}

int lineNumber = content
.Split('\n')
.ToList()
.FindIndex(line =>
line.Contains(VersionDetailsParser.SourceElementName) &&
line.Contains(lastBackflowVmrSha))
+ 1; // result is 0-indexed, but github lines are 1-indexed

// todo: we can skip this call if the last flown SHA is one that we already cached
string lastBackflowRepoSha = await BlameLineAsync(
repoUrl,
commit,
VersionFiles.VersionDetailsXml,
lineNumber);

return new Backflow(lastBackflowVmrSha, lastBackflowRepoSha);
}

private async Task<string> BlameLineAsync(string repoUrl, string commitOrBranch, string filePath, int lineNumber)
{
(string owner, string repo) = ParseRepoUri(repoUrl);

var query = $@"
query {{
repository(owner: {JsonConvert.SerializeObject(owner)}, name: {JsonConvert.SerializeObject(repo)}) {{
object(expression: {JsonConvert.SerializeObject(commitOrBranch)}) {{
... on Commit {{
blame(path: {JsonConvert.SerializeObject(filePath)}) {{
ranges {{
startingLine
endingLine
commit {{
oid
}}
}}
}}
}}
}}
}}
}}";

using var client = CreateHttpClient(repoUrl);

var requestBody = new { query };
Comment on lines +1635 to +1656
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The BlameLineAsync method constructs a GraphQL query with user-provided data (owner, repo, commitOrBranch, filePath) using string interpolation with JsonConvert.SerializeObject. While JSON serialization does provide some escaping, the constructed query string should be reviewed for potential GraphQL injection vulnerabilities. Consider using a typed GraphQL client library instead of manual query construction.

Suggested change
var query = $@"
query {{
repository(owner: {JsonConvert.SerializeObject(owner)}, name: {JsonConvert.SerializeObject(repo)}) {{
object(expression: {JsonConvert.SerializeObject(commitOrBranch)}) {{
... on Commit {{
blame(path: {JsonConvert.SerializeObject(filePath)}) {{
ranges {{
startingLine
endingLine
commit {{
oid
}}
}}
}}
}}
}}
}}
}}";
using var client = CreateHttpClient(repoUrl);
var requestBody = new { query };
var query = @"
query ($owner: String!, $name: String!, $expression: String!, $path: String!) {
repository(owner: $owner, name: $name) {
object(expression: $expression) {
... on Commit {
blame(path: $path) {
ranges {
startingLine
endingLine
commit {
oid
}
}
}
}
}
}
}";
using var client = CreateHttpClient(repoUrl);
var requestBody = new
{
query,
variables = new
{
owner,
name = repo,
expression = commitOrBranch,
path = filePath
}
};

Copilot uses AI. Check for mistakes.
var content = new StringContent(
JsonConvert.SerializeObject(requestBody, _serializerSettings),
Encoding.UTF8,
"application/json"
);
Comment on lines +1657 to +1661
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

Disposable 'StringContent' is created but not disposed.

Copilot uses AI. Check for mistakes.

var response = await client.PostAsync("graphql", content);
response.EnsureSuccessStatusCode();

using var stream = await response.Content.ReadAsStreamAsync();
using var doc = await JsonDocument.ParseAsync(stream);

if (doc.RootElement.TryGetProperty("errors", out var errors))
throw new InvalidOperationException($"GitHub GraphQL error: {errors}");

var obj = doc.RootElement
.GetProperty("data")
.GetProperty("repository")
.GetProperty("object");

if (obj.ValueKind == JsonValueKind.Null)
throw new InvalidOperationException($"Commit or branch '{commitOrBranch}' not found.");

// The blame field comes directly from the Commit node
var blame = obj.GetProperty("blame");
var ranges = blame.GetProperty("ranges").EnumerateArray();

foreach (var range in ranges)
{
int start = range.GetProperty("startingLine").GetInt32();
int end = range.GetProperty("endingLine").GetInt32();

if (lineNumber >= start && lineNumber <= end)
{
var oid = range.GetProperty("commit").GetProperty("oid").GetString();
return oid ?? throw new InvalidOperationException("Commit OID was null.");
}
}

throw new InvalidOperationException($"Line {lineNumber} not found in blame data.");
Comment on lines +1631 to +1696
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The method lacks error handling for the case where the specified line number is not found in the blame data. While line 1696 throws an exception, there's no handling for edge cases like empty blame ranges or when the file doesn't exist at the specified commit. The calling code should be prepared to handle these exceptions or the method should provide more detailed error messages.

Copilot uses AI. Check for mistakes.
}



private async Task<string> GetFileContentAtCommit(string repoUrl, string commit, string filePath)
{
(string owner, string repo) = ParseRepoUri(repoUrl);
var file = await GetClient(repoUrl).Repository.Content.GetAllContentsByRef(owner, repo, filePath, commit);
return file[0].Content;
}
}
4 changes: 4 additions & 0 deletions src/Microsoft.DotNet.Darc/DarcLib/GitRepoFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class GitRepoFactory : IGitRepoFactory
private readonly ITelemetryRecorder _telemetryRecorder;
private readonly IProcessManager _processManager;
private readonly IFileSystem _fileSystem;
private readonly IVersionDetailsParser _versionDetailsParser;
private readonly ILoggerFactory _loggerFactory;
private readonly string? _temporaryPath = null;

Expand All @@ -31,6 +32,7 @@ public GitRepoFactory(
ITelemetryRecorder telemetryRecorder,
IProcessManager processManager,
IFileSystem fileSystem,
IVersionDetailsParser versionDetailsParser,
ILoggerFactory loggerFactory,
string temporaryPath)
{
Expand All @@ -39,6 +41,7 @@ public GitRepoFactory(
_telemetryRecorder = telemetryRecorder;
_processManager = processManager;
_fileSystem = fileSystem;
_versionDetailsParser = versionDetailsParser;
_loggerFactory = loggerFactory;
_temporaryPath = temporaryPath;
}
Expand All @@ -56,6 +59,7 @@ public GitRepoFactory(
_processManager,
_loggerFactory.CreateLogger<GitHubClient>(),
_temporaryPath,
_versionDetailsParser,
// Caching not in use for Darc local client.
null),

Expand Down
Loading
Loading