Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ public Task<bool> HasWritePermission(string owner, string repo, string username)
return Task.FromResult(true);
}

public Task<bool> HasPushPermission(string owner, string repo)
{
return Task.FromResult(true);
}

private RepositoryContent CreateMockRepositoryContent(string name, string path, string encodedContent)
{
return new RepositoryContent(
Expand Down
17 changes: 16 additions & 1 deletion tools/azsdk-cli/Azure.Sdk.Tools.Cli/Services/GitHubService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public interface IGitHubService
public Task<RepositoryContent> GetContentsSingleAsync(string owner, string repoName, string path, string? branch = null);
public Task<HashSet<string>?> GetPublicOrgMembership(string username);
public Task<bool> HasWritePermission(string owner, string repo, string username);
public Task<bool> HasPushPermission(string owner, string repo);
}

public class GitHubService : GitConnection, IGitHubService
Expand All @@ -122,7 +123,7 @@ public async Task<PullRequest> GetPullRequestAsync(string repoOwner, string repo
{
var pullRequest = await gitHubClient.PullRequest.Get(repoOwner, repoName, pullRequestNumber);
return pullRequest;
}
}
public async Task UpdatePullRequestAsync(string repoOwner, string repoName, int pullRequestNumber, string title, string body, ItemState state)
{
// This method now accepts title, body, and state directly, so caller must fetch the PR first if needed.
Expand Down Expand Up @@ -549,5 +550,19 @@ public async Task<bool> HasWritePermission(string owner, string repo, string use
throw;
}
}

public async Task<bool> HasPushPermission(string owner, string repo)
{
try
{
var repository = await gitHubClient.Repository.Get(owner, repo);
return repository.Permissions?.Push ?? false;
}
catch (Exception ex)
{
logger.LogError(ex, "Error checking push permissions for repo: {Owner}/{Repo}", owner, repo);
throw;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class CodeownersTools : MCPTool
private readonly Option<string[]> serviceOwnersOption = new(["--service-owners"], "The service owners (space-separated)") { IsRequired = false };
private readonly Option<string[]> sourceOwnersOption = new(["--source-owners"], "The source owners (space-separated)") { IsRequired = false };
private readonly Option<bool> isAddingOption = new(["--is-adding"], "Whether to add (true) or remove (false) owners") { IsRequired = false };
private readonly Option<string> workingBranchOption = new(["--branch"], "Branch to make edits to, only if provided.") { IsRequired = false };
private readonly Option<int> prNumberOption = new(["--pr-number"], "PR number.") { IsRequired = false };

public CodeownersTools(
IGitHubService githubService,
Expand Down Expand Up @@ -74,13 +74,14 @@ public override Command GetCommand()
serviceOwnersOption,
sourceOwnersOption,
isAddingOption,
workingBranchOption,
prNumberOption,
},
new Command(validateCodeownersEntryCommandName, "Validate codeowners for an existing service entry")
{
repoOption,
serviceLabelOption,
pathOptionOptional
pathOptionOptional,
prNumberOption,
}
};

Expand All @@ -106,7 +107,7 @@ public override async Task HandleCommand(InvocationContext ctx, CancellationToke
var serviceOwnersValue = commandParser.GetValueForOption(serviceOwnersOption);
var sourceOwnersValue = commandParser.GetValueForOption(sourceOwnersOption);
var isAddingValue = commandParser.GetValueForOption(isAddingOption);
var workingBranchValue = commandParser.GetValueForOption(workingBranchOption);
var prNumberValue = commandParser.GetValueForOption(prNumberOption);

var addResult = await UpdateCodeowners(
repoValue ?? "",
Expand All @@ -116,7 +117,7 @@ public override async Task HandleCommand(InvocationContext ctx, CancellationToke
serviceOwnersValue?.ToList() ?? new List<string>(),
sourceOwnersValue?.ToList() ?? new List<string>(),
isAddingValue,
workingBranchValue ?? "");
prNumberValue);
ctx.ExitCode = ExitCode;
output.Output(addResult);
return;
Expand All @@ -126,11 +127,13 @@ public override async Task HandleCommand(InvocationContext ctx, CancellationToke
var validateRepo = commandParser.GetValueForOption(repoOption);
var validateServiceLabel = commandParser.GetValueForOption(serviceLabelOption);
var validateRepoPath = commandParser.GetValueForOption(pathOptionOptional);
var prNumberValue = commandParser.GetValueForOption(prNumberOption);

var validateResult = await ValidateCodeownersEntryForService(
validateRepo ?? "",
validateServiceLabel,
validateRepoPath);
validateRepoPath,
prNumberValue);
ctx.ExitCode = ExitCode;
output.Output(validateResult);
return;
Expand All @@ -152,7 +155,7 @@ public async Task<string> UpdateCodeowners(
List<string> serviceOwners = null,
List<string> sourceOwners = null,
bool isAdding = false,
string workingBranch = "")
int prNumber = 0)
{
try
{
Expand All @@ -162,29 +165,28 @@ public async Task<string> UpdateCodeowners(
throw new Exception($"Service label: {serviceLabel} and Path: {path} are both invalid. At least one must be valid");
}

if (workingBranch.Equals("main", StringComparison.OrdinalIgnoreCase))
{
throw new Exception($"Cannot make changes on branch: {workingBranch}");
}
else if (string.IsNullOrEmpty(workingBranch))
{
var codeownersPullRequests = (await githubService.SearchPullRequestsByTitleAsync(Constants.AZURE_OWNER_PATH, repo, "[CODEOWNERS]"))
?? new List<PullRequest?>().AsReadOnly();
string workingBranch = "";
string repoOwner = "";

foreach (var codeownersPullRequest in codeownersPullRequests)
// Resolve PR number to actual branch name if provided.
if (prNumber > 0)
{
var pr = await githubService.GetPullRequestAsync(Constants.AZURE_OWNER_PATH, repo, prNumber);
Comment thread
shirelmr marked this conversation as resolved.
if (pr == null)
{
if (codeownersPullRequest != null &&
((!string.IsNullOrEmpty(serviceLabel) && codeownersPullRequest.Title.Contains(serviceLabel, StringComparison.OrdinalIgnoreCase)) ||
(!string.IsNullOrEmpty(path) && codeownersPullRequest.Title.Contains(path, StringComparison.OrdinalIgnoreCase))))
{
workingBranch = codeownersPullRequest.Head.Ref;
break;
}
throw new Exception($"Pull request #{prNumber} could not be found or retrieved from repository '{repo}'.");
}
workingBranch = pr.Head.Ref;
repoOwner = pr.Head.Repository.Owner.Login;
}

if (string.IsNullOrEmpty(repoOwner))
{
repoOwner = Constants.AZURE_OWNER_PATH;
}
Comment thread
shirelmr marked this conversation as resolved.

// Get codeowners file contents.
var codeownersFileContent = await githubService.GetContentsSingleAsync(Constants.AZURE_OWNER_PATH, repo, Constants.AZURE_CODEOWNERS_PATH, workingBranch);
var codeownersFileContent = await githubService.GetContentsSingleAsync(repoOwner, repo, Constants.AZURE_CODEOWNERS_PATH, workingBranch);

if (codeownersFileContent == null)
{
Expand Down Expand Up @@ -233,7 +235,8 @@ public async Task<string> UpdateCodeowners(
$"Update codeowners entry for {identifier}", // Description for commit message, PR title, and description
"update-codeowners-entry", // Branch prefix for the action
identifier, // Identifier for the PR
workingBranch);
workingBranch,
repoOwner);

return string.Join("\n", resultMessages.Concat(new[] { codeownersValidationResultMessage }));
}
Expand All @@ -251,13 +254,22 @@ private async Task<List<string>> CreateCodeownersPR(
string description, // used for commit message, PR title, and PR description
string branchPrefix,
string identifier,
string workingBranch)
string workingBranch,
string repoOwner)
{
List<string> resultMessages = new();
var branchName = "";
var hasPushPermissions = await githubService.HasPushPermission(repoOwner, repo);

if (!hasPushPermissions)
{
resultMessages.Add($"GitHub token does not have permission to push to {repoOwner} repository, opening a new PR on a branch in the main repo");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not sure I understand this message. The user can't create a PR so the tool is going to create it for them?

repoOwner = Constants.AZURE_OWNER_PATH;
workingBranch = "";
}

// Check if we have a working branch from SDK generation
if (!string.IsNullOrEmpty(workingBranch) && await githubService.IsExistingBranchAsync(Constants.AZURE_OWNER_PATH, repo, workingBranch))
if (!string.IsNullOrEmpty(workingBranch) && await githubService.IsExistingBranchAsync(repoOwner, repo, workingBranch))
{
branchName = workingBranch;
resultMessages.Add($"Using existing branch: {branchName}");
Expand All @@ -271,7 +283,7 @@ private async Task<List<string>> CreateCodeownersPR(
}

// After branchName is set
var codeownersFileContent = await githubService.GetContentsSingleAsync(Constants.AZURE_OWNER_PATH, repo, Constants.AZURE_CODEOWNERS_PATH, branchName);
var codeownersFileContent = await githubService.GetContentsSingleAsync(repoOwner, repo, Constants.AZURE_CODEOWNERS_PATH, branchName);

if (codeownersFileContent == null)
{
Expand All @@ -281,7 +293,7 @@ private async Task<List<string>> CreateCodeownersPR(
var codeownersSha = codeownersFileContent.Sha;

// Use codeownersSha in UpdateFileAsync
await githubService.UpdateFileAsync(Constants.AZURE_OWNER_PATH, repo, Constants.AZURE_CODEOWNERS_PATH, description, modifiedContent, codeownersSha, branchName);
await githubService.UpdateFileAsync(repoOwner, repo, Constants.AZURE_CODEOWNERS_PATH, description, modifiedContent, codeownersSha, branchName);

var prInfoList = await githubService.CreatePullRequestAsync(repo, Constants.AZURE_OWNER_PATH, "main", branchName, "[CODEOWNERS] " + description, description);
if (prInfoList != null)
Expand All @@ -298,7 +310,7 @@ private async Task<List<string>> CreateCodeownersPR(
}

[McpServerTool(Name = "azsdk_engsys_validate_codeowners_entry_for_service"), Description("Validates codeowners in a specific repository for a given service or repo path.")]
public async Task<ServiceCodeownersResult> ValidateCodeownersEntryForService(string repoName, string? serviceLabel = null, string? path = null)
public async Task<ServiceCodeownersResult> ValidateCodeownersEntryForService(string repoName, string? serviceLabel = null, string? path = null, int prNumber = 0)
{
ServiceCodeownersResult response = new() { };

Expand All @@ -316,17 +328,19 @@ public async Task<ServiceCodeownersResult> ValidateCodeownersEntryForService(str
throw new Exception("Must provide a service label or a repository path.");
}

var workingBranch = "";
var codeownersPullRequests = await githubService.SearchPullRequestsByTitleAsync(Constants.AZURE_OWNER_PATH, repoName, "[CODEOWNERS]");
string workingBranch = "";
string repoOwner = Constants.AZURE_OWNER_PATH;

foreach (var codeownersPullRequest in codeownersPullRequests)
if (prNumber > 0)
{
if (codeownersPullRequest != null &&
((!string.IsNullOrEmpty(serviceLabel) && codeownersPullRequest.Title.Contains(serviceLabel, StringComparison.OrdinalIgnoreCase)) ||
(!string.IsNullOrEmpty(path) && codeownersPullRequest.Title.Contains(path, StringComparison.OrdinalIgnoreCase))))
var pr = await githubService.GetPullRequestAsync(Constants.AZURE_OWNER_PATH, repoName, prNumber);
Comment thread
shirelmr marked this conversation as resolved.
if (pr == null)
{
workingBranch = codeownersPullRequest.Head.Ref;
response.Message += $"Pull request #{prNumber} not found in repository '{repoName}'.";
return response;
}
workingBranch = pr.Head.Ref;
repoOwner = pr.Head.Repository.Owner.Login;
}

if (workingBranch.Equals("main", StringComparison.OrdinalIgnoreCase))
Expand All @@ -337,10 +351,10 @@ public async Task<ServiceCodeownersResult> ValidateCodeownersEntryForService(str
CodeownersEntry? matchingEntry;
try
{
var contents = await githubService.GetContentsSingleAsync("Azure", "azure-sdk-for-net", ".github/CODEOWNERS", workingBranch);
var contents = await githubService.GetContentsSingleAsync(repoOwner, repoName, ".github/CODEOWNERS", workingBranch);
if (contents == null)
{
throw new Exception("Could not retrieve upstream CODEOWNERS (azure-sdk-for-net) for the requested branch.");
throw new Exception($"Could not retrieve upstream CODEOWNERS ({repoName}) for the requested branch.");
}
var codeownersContent = contents.Content;
var codeownersSha = contents.Sha;
Expand Down
Loading