diff --git a/CLAUDE.md b/CLAUDE.md index ec79fea..aa1b7d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,9 +35,9 @@ results and coverage, in one Avalonia window. See `README.md` for the pitch. ## Project layout -- `src/Stampeded.Core/` - everything that does not need a UI: git and GitHub access, diff and - fold building, Roslyn hosting, the LSP client, the review store. No Avalonia reference; keep - it that way. +- `src/Stampeded.Core/` - everything that does not need a UI: git and pull-request-host access, + diff and fold building, Roslyn hosting, the LSP client, the review store. No Avalonia + reference; keep it that way. - `src/Stampeded/` - the Avalonia app: panes, documents, controls, view models. - `src/Stampeded.RoslynLsp/` - Roslyn as a language server, for reading C# out of process. - `tests/Stampeded.Core.Tests/` - NUnit, covering `Stampeded.Core` only. The UI layer has no @@ -46,11 +46,11 @@ results and coverage, in one Avalonia window. See `README.md` for the pitch. ## Everything external is a CLI -`git`, `gh`, `dotnet`, `code` and `xdg-open` are the only ways out of the process, all through -`ExternalTool.RunAsync` (which logs the command, and on failure the first line of its output - -an exit code alone never says what went wrong). There are no API tokens of the tool's own: auth, -SSO and token refresh ride on the user's `gh` login. Keep it that way; do not add an HTTP client -for GitHub. +`git`, `gh`, `az`, `dotnet`, `code` and `xdg-open` are the only ways out of the process, all +through `ExternalTool.RunAsync` (which logs the command, and on failure the first line of its +output - an exit code alone never says what went wrong). There are no API tokens of the tool's +own: auth, SSO and token refresh ride on the user's `gh auth` and `az login`. Keep it that way; +do not add an HTTP client for any host. A language server is the one exception, because it is not a command with an exit code: it starts once and answers until the review closes, over JSON-RPC on its stdin and stdout @@ -60,6 +60,34 @@ requests that take a noticeable while, every line of its stderr. `CliLog.Write` is the log sink the Log pane shows. Anything a user might have to explain to someone else belongs in it. +## A pull request comes from a host, not from GitHub + +Everything a review asks about a pull request - the open list, its branches and description, its +checks, merge state, posted comments and thread resolution, the reviews, a verdict, a merge - +goes through `IPullRequestHost` (`Stampeded.Core/PullRequests/`). There are two: +`GitHubService` over `gh`, and `AzureDevOpsService` over `az` with its `azure-devops` extension +(`az repos pr ...`, `az repos policy ...`, and `az devops invoke` for the REST surface the +extension has no verb for - the analogue of `gh api`). + +Which one answers is decided once per workspace, in `PullRequestHosts.ForAsync`, from origin's +URL: what `AzureDevOpsUrl` parses is Azure DevOps, anything else is GitHub on purpose - `gh` +also serves GitHub Enterprise hosts, which nothing here can enumerate, and a clone with no +origin behaves as it always did. `STAMPEDED_PR_HOST=github|azdo` overrides it. The answer is a +property of the repository, so it sits on `Program.Host` beside `Program.RepoPath` and reaches +the review as `ReviewWorkspace.Host`. + +**GitHub's words are the model's words.** `APPROVE` / `REQUEST_CHANGES` / `COMMENT`, `APPROVED` +/ `CHANGES_REQUESTED`, `LEFT` / `RIGHT`, `MERGEABLE` / `BLOCKED`: the panes and the pure +functions under them already speak them, and Azure DevOps - which counts votes from 10 to -10 +and has no review object at all - maps onto them inside its own implementation and nowhere +else. Nothing above `IPullRequestHost` knows which host answered; what a pane shows the reader +comes from `HostName`. + +Not on Azure DevOps, and refused with a reason rather than hidden: pull requests from forks, +line totals and check state in the pull-request list (a call per row), and the server-side +branch update, for which there is no API. A ```suggestion``` block is GitHub's alone, and posts +as a plain code block elsewhere. + ## Semantics come from a provider, not from Roslyn Everything the review asks about source - what a token means, where it is declared, who uses @@ -137,7 +165,7 @@ than with a tree at the wrong lines. A parser in this process answers before it `~/.cache/stampeded/prs`, and `OpenPrAsync` falls back to it when `gh` fails and the commits are still in the object database. The change itself is never cached: it is read from those commits. An offline review says so and refuses to submit a verdict or a merge. -- **GitHub is the authority for facts git cannot know**: the viewer's login, a repository's +- **The host is the authority for facts git cannot know**: the viewer's login, a repository's default branch. The local `origin/HEAD` is a clone-time snapshot and goes stale. - **`Stampeded.Core/TreeView/` and `Stampeded/Controls/TreeView/` are vendored from ILSpy.** They are meant to stay close to upstream so fixes can move both ways - read diff --git a/README.md b/README.md index 12f4c04..a047e37 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A keyboard-driven desktop code-review tool: PR diffs with real semantic code navigation (go to definition, find references, hover docs), git blame, CI results, and unit-test results in one Avalonia UI. -Built on AvaloniaEdit; editor components adapted from [ILSpy](https://github.com/icsharpcode/ILSpy) (MIT); diff-view concepts inspired by [Aehnlich](https://github.com/Dirkster99/Aehnlich) (MIT). Uses the `git` and `gh` CLIs for repository and GitHub access. +Built on AvaloniaEdit; editor components adapted from [ILSpy](https://github.com/icsharpcode/ILSpy) (MIT); diff-view concepts inspired by [Aehnlich](https://github.com/Dirkster99/Aehnlich) (MIT). Uses the `git`, `gh` and `az` CLIs for repository, GitHub and Azure DevOps access. Siegi and Chris recorded a brief [Introduction to Stampeded!](https://youtu.be/r16YIcvLlg4) for you to get a glimpse at what the IRE is capable of. diff --git a/src/Stampeded.Core/AzureDevOps/AzureDevOpsService.cs b/src/Stampeded.Core/AzureDevOps/AzureDevOpsService.cs new file mode 100644 index 0000000..5b1864c --- /dev/null +++ b/src/Stampeded.Core/AzureDevOps/AzureDevOpsService.cs @@ -0,0 +1,761 @@ +using System.Text.Json; + +using Stampeded.Core.Infra; +using Stampeded.Core.PullRequests; + +namespace Stampeded.Core.AzureDevOps; + +/// +/// Azure DevOps access through the `az` CLI and its azure-devops extension, run in the +/// repository directory. Auth rides on the user's `az login`, the way GitHub's rides on gh. +/// +/// Every command names the organization and project explicitly rather than leaning on +/// `az devops configure --defaults`, which is a per-machine setting a reader may have pointed +/// at another project entirely. +/// +/// What the extension has no verb for goes through az devops invoke, which is the +/// analogue of `gh api`: threads, iterations, build logs and the completion PATCH. +/// +public sealed class AzureDevOpsService(string repoPath, string org, string project, string repo) + : IPullRequestHost +{ + /// The REST version the routes below are written against. Named on every invoke: + /// az otherwise picks the newest the server offers, which is a moving target. + const string ApiVersion = "7.1"; + + readonly string orgUrl = $"https://dev.azure.com/{org}"; + readonly Dictionary> pullRequests = []; + readonly Dictionary> workItemTitles = []; + string? viewerLogin; + MergeMethods? mergeMethods; + + public string Name => "Azure DevOps"; + + /// Azure DevOps lets an author vote on their own pull request. + public bool AcceptsOwnApproval => true; + + // ---- plumbing ---------------------------------------------------------------------- + + /// Named on every command rather than left to `az devops configure --defaults`, + /// which is a per-machine setting a reader may have pointed at another project entirely. + /// The project is not among them: the commands addressed by pull-request id - show, policy + /// list, set-vote, update - and `az devops invoke` take no --project and reject it. + string[] OrgArgs => ["--organization", orgUrl, "--output", "json"]; + + string[] ProjectArgs => [.. OrgArgs, "--project", project]; + + /// One `az` command whose answer is JSON. for the + /// commands that name the project rather than an id. + async Task JsonAsync(CancellationToken ct, bool inProject, params string[] args) + { + string output = await ExternalTool.RunAsync("az", + [.. args, .. inProject ? ProjectArgs : OrgArgs], repoPath, ct); + return JsonDocument.Parse(output); + } + + /// + /// One REST call the extension has no verb for. A body can only be handed over in a file, + /// so it is written to a temporary one and deleted afterwards; the log names the route, not + /// the body, which is review prose and can be a page long. + /// + async Task InvokeAsync(string area, string resource, string httpMethod, + IReadOnlyDictionary route, string? jsonBody, CancellationToken ct) + { + string? file = null; + try + { + string[] args = [ + "devops", "invoke", + "--area", area, "--resource", resource, + "--http-method", httpMethod, + "--api-version", ApiVersion, + .. route.Count > 0 + ? (string[])["--route-parameters", .. route.Select(p => $"{p.Key}={p.Value}")] + : [], + ]; + if (jsonBody is not null) + { + file = Path.Combine(Path.GetTempPath(), $"stampeded-{Guid.NewGuid():N}.json"); + await File.WriteAllTextAsync(file, jsonBody, ct); + args = [.. args, "--in-file", file]; + } + CliLog.Write("host", $"{httpMethod} {area}/{resource} {string.Join(' ', route.Select(p => $"{p.Key}={p.Value}"))}"); + string output = await ExternalTool.RunAsync("az", [.. args, .. OrgArgs], repoPath, ct); + // A PATCH or POST that returns nothing is a success with no document to read. + return JsonDocument.Parse(output.Trim().Length == 0 ? "{}" : output); + } + finally + { + if (file is not null && File.Exists(file)) + File.Delete(file); + } + } + + /// + /// What `az repos pr show` says, asked once per pull request for the session. The refspec, + /// the merge state, the reviews and the completion all read from the same answer, and a + /// round trip each would be four for one screen. + /// + Task PrAsync(int number, CancellationToken ct) + { + if (!pullRequests.TryGetValue(number, out var pending)) + pullRequests[number] = pending = JsonAsync(ct, inProject: false, "repos", "pr", "show", "--id", number.ToString()); + return pending; + } + + /// Forgets the cached answer, so the next reader sees a pull request as it is now: + /// a vote, a completion or a push changes it. + void Forget(int number) => pullRequests.Remove(number); + + static string? Str(JsonElement element, params string[] path) + { + foreach (string name in path) + { + if (element.ValueKind != JsonValueKind.Object || !element.TryGetProperty(name, out element)) + return null; + } + return element.ValueKind == JsonValueKind.String ? element.GetString() : null; + } + + static JsonElement? Node(JsonElement element, params string[] path) + { + foreach (string name in path) + { + if (element.ValueKind != JsonValueKind.Object || !element.TryGetProperty(name, out element)) + return null; + } + return element; + } + + static IEnumerable Array(JsonElement? element) + => element is { ValueKind: JsonValueKind.Array } array ? array.EnumerateArray() : []; + + static string StripRefsHeads(string? refName) + => refName is { Length: > 0 } name && name.StartsWith("refs/heads/", StringComparison.Ordinal) + ? name["refs/heads/".Length..] + : refName ?? ""; + + // ---- identity and repository ------------------------------------------------------- + + /// + /// The account az is signed in as, as the user principal name that + /// createdBy.uniqueName and reviewers[].uniqueName carry - so "approved" and + /// "approved by the reader" are compared like with like. Cached: it cannot change without + /// az being signed in again underneath the app. + /// + public async Task GetViewerLoginAsync(CancellationToken ct = default) + { + if (viewerLogin is { Length: > 0 }) + return viewerLogin; + try + { + using var doc = await InvokeAsync("Location", "ConnectionData", "GET", + new Dictionary(), null, ct); + if (Str(doc.RootElement, "authenticatedUser", "properties", "Account", "$value") is { Length: > 0 } upn) + { + CliLog.Write("host", $"viewer {upn} (connection data)"); + return viewerLogin = upn; + } + } + catch (ToolFailedException) + { + // Some extension versions do not know that resource name; the signed-in account + // answers the same question. + } + string name = (await ExternalTool.RunAsync("az", + ["account", "show", "--query", "user.name", "--output", "tsv"], repoPath, ct)).Trim(); + CliLog.Write("host", $"viewer {name} (az account)"); + return viewerLogin = name; + } + + public async Task GetDefaultBranchAsync(CancellationToken ct = default) + => StripRefsHeads(Str((await RepositoryAsync(ct)).RootElement, "defaultBranch")); + + /// What `az repos show` says about the repository: its default branch and its id. + /// Read once - neither changes while a review is open - and the id is what the policy + /// commands want, a name being no answer to `--repository-id`. + Task RepositoryAsync(CancellationToken ct) + => repository ??= JsonAsync(ct, inProject: true, "repos", "show", "--repository", repo); + + Task? repository; + + public Task PrUrlAsync(int number, CancellationToken ct = default) + => Task.FromResult($"{WebBase}/pullrequest/{number}"); + + public Task CommitUrlAsync(string sha, CancellationToken ct = default) + => Task.FromResult($"{WebBase}/commit/{sha}"); + + string WebBase => $"{orgUrl}/{Uri.EscapeDataString(project)}/_git/{Uri.EscapeDataString(repo)}"; + + // ---- the pull request -------------------------------------------------------------- + + /// + /// Azure DevOps advertises refs/pull/N/merge but no /head, so the head is + /// fetched from the source branch itself. A push between reading the pull request and + /// fetching would hand over a newer commit than the one the rest of the review was built + /// from, so the two are compared and the difference is logged rather than passed over. + /// + public async Task PrHeadRefspecAsync(int number, CancellationToken ct = default) + { + var doc = await PrAsync(number, ct); + if (Node(doc.RootElement, "forkSource") is { ValueKind: not JsonValueKind.Null }) + throw new RefusedException("Pull requests from forks are not supported on Azure DevOps yet."); + string branch = StripRefsHeads(Str(doc.RootElement, "sourceRefName")); + if (branch.Length == 0) + throw new RefusedException($"Azure DevOps did not name a source branch for pull request {number}."); + return $"+refs/heads/{branch}:refs/stampeded/pr/{number}"; + } + + public async Task GetPrAsync(int number, CancellationToken ct = default) + { + Forget(number); + var doc = await PrAsync(number, ct); + var pr = doc.RootElement; + return new PrDetail( + number, + Str(pr, "title") ?? "", + Str(pr, "description"), + StripRefsHeads(Str(pr, "targetRefName")), + StripRefsHeads(Str(pr, "sourceRefName")), + // active / completed / abandoned. Nothing in the review compares it to GitHub's + // words; it is shown and logged. + Str(pr, "status") ?? "", + new PrAuthor(Str(pr, "createdBy", "uniqueName") ?? Str(pr, "createdBy", "displayName") ?? ""), + IsDraft: Node(pr, "isDraft")?.ValueKind == JsonValueKind.True); + } + + public async Task> ListOpenPrsAsync(CancellationToken ct = default) + { + using var doc = await JsonAsync(ct, inProject: true, + "repos", "pr", "list", "--repository", repo, "--status", "active", "--top", "50"); + var summaries = new List(); + foreach (var pr in Array(doc.RootElement)) + { + var reviewers = Array(Node(pr, "reviewers")).ToList(); + // Votes: 10 approved, 5 approved with suggestions, 0 no vote, -5 waiting for the + // author, -10 rejected. Approval is the two positive ones; anything negative is a + // reviewer asking for something. + var latest = reviewers + .Select(r => (User: Str(r, "uniqueName") ?? Str(r, "displayName") ?? "", Vote: Vote(r))) + .Where(r => r.Vote != 0) + .Select(r => new PrLatestReview(new PrAuthor(r.User), + r.Vote >= 5 ? "APPROVED" : "CHANGES_REQUESTED")) + .ToList(); + var requested = reviewers + .Where(r => Vote(r) == 0) + .Select(r => new PrReviewRequest(Str(r, "uniqueName") ?? Str(r, "displayName"))) + .ToList(); + summaries.Add(new PrSummary( + Node(pr, "pullRequestId")?.GetInt32() ?? 0, + Str(pr, "title") ?? "", + new PrAuthor(Str(pr, "createdBy", "uniqueName") ?? Str(pr, "createdBy", "displayName") ?? ""), + StripRefsHeads(Str(pr, "sourceRefName")), + StripRefsHeads(Str(pr, "targetRefName")), + Node(pr, "isDraft")?.ValueKind == JsonValueKind.True, + // The list carries no last-touched date, only when the pull request was opened. + Node(pr, "creationDate")?.GetDateTimeOffset() ?? default, + // ponytail: no line totals and no check state in the list - both would be a + // call per row (`az repos pr policy list`). Add them if the list has to show + // them, one request per row at that point. + StatusCheckRollup: null, + HeadRefOid: Str(pr, "lastMergeSourceCommit", "commitId"), + ReviewDecision: Decision(reviewers), + LatestReviews: latest, + ReviewRequests: requested)); + } + return summaries; + } + + static int Vote(JsonElement reviewer) + => Node(reviewer, "vote") is { ValueKind: JsonValueKind.Number } vote ? vote.GetInt32() : 0; + + /// + /// The pull request's standing, in GitHub's word for it. Azure DevOps has no such verdict of + /// its own: it counts votes and requires the ones its policies mark required, so that is + /// what is counted here. + /// + static string? Decision(IReadOnlyList reviewers) + { + if (reviewers.Any(r => Vote(r) < 0)) + return "CHANGES_REQUESTED"; + var required = reviewers.Where(r => Node(r, "isRequired")?.ValueKind == JsonValueKind.True).ToList(); + bool anyVote = reviewers.Any(r => Vote(r) != 0); + return anyVote && required.All(r => Vote(r) >= 5) ? "APPROVED" : null; + } + + // ---- policies: checks, merge state, merge methods ---------------------------------- + + /// The policy evaluations of a pull request: builds, reviewer counts, linked work + /// items, resolved comments. Only the builds are checks; the rest is why a merge is + /// blocked, and is read as such. + Task PolicyEvaluationsAsync(int number, CancellationToken ct) + => JsonAsync(ct, inProject: false, "repos", "pr", "policy", "list", "--id", number.ToString()); + + static bool IsBuildPolicy(JsonElement evaluation) + => Str(evaluation, "configuration", "type", "displayName") == "Build"; + + public async Task> GetChecksAsync(int number, CancellationToken ct = default) + { + using var doc = await PolicyEvaluationsAsync(number, ct); + var checks = new List(); + foreach (var evaluation in Array(doc.RootElement).Where(IsBuildPolicy)) + { + string status = Str(evaluation, "status") ?? ""; + long? buildId = Node(evaluation, "context", "buildId") is { ValueKind: JsonValueKind.Number } id + ? id.GetInt64() + : null; + checks.Add(new CheckRun( + Str(evaluation, "configuration", "settings", "displayName") ?? "Build", + status, + Bucket(status), + buildId is { } build ? $"{orgUrl}/{Uri.EscapeDataString(project)}/_build/results?buildId={build}" : null, + Workflow: null, + RunId: buildId)); + } + return checks; + } + + /// + /// A policy evaluation's status in the words `gh pr checks` uses, which is what a + /// holds. "notApplicable" is a policy that does not apply to + /// this pull request at all, which is not a check saying no - it is skipped. + /// + static string Bucket(string status) => status switch { + "rejected" or "broken" => "fail", + "queued" or "running" => "pending", + "notApplicable" => "skipping", + _ => "pass", + }; + + public async Task GetMergeStateAsync(int number, CancellationToken ct = default) + { + Forget(number); + var pr = (await PrAsync(number, ct)).RootElement; + using var policies = await PolicyEvaluationsAsync(number, ct); + var evaluations = Array(policies.RootElement).ToList(); + var builds = evaluations.Where(IsBuildPolicy).ToList(); + string mergeStatus = Str(pr, "mergeStatus") ?? ""; + bool isDraft = Node(pr, "isDraft")?.ValueKind == JsonValueKind.True; + // Only a blocking policy refuses the completion; an optional build that failed is a + // check the reader may take or leave, which is what UNSTABLE says on the other host. + bool Unsettled(JsonElement e) => Bucket(Str(e, "status") ?? "") is "fail" or "pending"; + bool blocked = evaluations.Any(e => + Node(e, "configuration", "isBlocking")?.ValueKind == JsonValueKind.True && Unsettled(e)); + bool unstable = !blocked && builds.Any(Unsettled); + // The checks are handed over in the shape the panes already fold, so one reading of a + // rollup serves both hosts. + string rollup = JsonSerializer.Serialize(builds.Select(b => new { + name = Str(b, "configuration", "settings", "displayName") ?? "Build", + state = Str(b, "status") ?? "", + conclusion = Bucket(Str(b, "status") ?? "") switch { + "fail" => "FAILURE", + "pending" => "PENDING", + _ => "SUCCESS", + }, + })); + using var rollupDoc = JsonDocument.Parse(rollup); + return new MergeState( + mergeStatus switch { + "succeeded" => "MERGEABLE", + "conflicts" => "CONFLICTING", + _ => "UNKNOWN", + }, + isDraft ? "DRAFT" : blocked ? "BLOCKED" : unstable ? "UNSTABLE" : "CLEAN", + Decision([.. Array(Node(pr, "reviewers"))]), + isDraft, + StripRefsHeads(Str(pr, "targetRefName")), + rollupDoc.RootElement.Clone(), + Str(pr, "status"), + Str(pr, "lastMergeSourceCommit", "commitId")) { + Host = Name, + }; + } + + /// The merge strategies the target branch's policy allows. Without such a policy + /// all three are on, which is what Azure DevOps itself does. + public async Task GetMergeMethodsAsync(CancellationToken ct = default) + { + if (mergeMethods is { } known) + return known; + var repository = (await RepositoryAsync(ct)).RootElement; + string branch = StripRefsHeads(Str(repository, "defaultBranch")); + using var doc = await JsonAsync(ct, inProject: true, "repos", "policy", "list", + "--repository-id", Str(repository, "id") ?? repo, "--branch", branch); + // fa4e907d-c16b-4a4c-9dfa-4916e5d171ab is the "Require a merge strategy" policy type. + var strategy = Array(doc.RootElement).FirstOrDefault(p => + string.Equals(Str(p, "type", "id"), "fa4e907d-c16b-4a4c-9dfa-4916e5d171ab", + StringComparison.OrdinalIgnoreCase)); + if (strategy.ValueKind != JsonValueKind.Object) + return mergeMethods = new MergeMethods(true, true, true); + bool On(string setting) => Node(strategy, "settings", setting)?.ValueKind == JsonValueKind.True; + return mergeMethods = new MergeMethods(On("allowNoFastForward"), On("allowSquash"), On("allowRebase")); + } + + // ---- work items --------------------------------------------------------------------- + + /// The title of a work item, or null when the number is not one. Azure DevOps + /// autolinks "#123" to a work item exactly as GitHub does to an issue. + public Task GetIssueTitleAsync(int number, CancellationToken ct = default) + { + if (!workItemTitles.TryGetValue(number, out var pending)) + workItemTitles[number] = pending = AskAsync(); + return pending; + + async Task AskAsync() + { + try + { + string title = (await ExternalTool.RunAsync("az", + ["boards", "work-item", "show", "--id", number.ToString(), + "--query", "fields.\"System.Title\"", "--output", "tsv", + "--organization", orgUrl], repoPath, ct)).Trim(); + return title.Length > 0 ? title : null; + } + catch (ToolFailedException) + { + // Not a work item, which is the answer rather than a failure. + return null; + } + } + } + + public Task GetIssueUrlPrefixAsync(CancellationToken ct = default) + => Task.FromResult($"{orgUrl}/{Uri.EscapeDataString(project)}/_workitems/edit/"); + + // ---- completing --------------------------------------------------------------------- + + /// + /// Completes the pull request. az repos pr update knows only --squash, so the + /// strategy is set through the REST PATCH instead. The head commit is named in it: Azure + /// DevOps refuses a completion that names a commit the branch has moved past, which is the + /// guard a merge wants. + /// + public async Task MergePrAsync(int number, string method, bool deleteBranch = false, + CancellationToken ct = default) + { + Forget(number); + var pr = (await PrAsync(number, ct)).RootElement; + string? head = Str(pr, "lastMergeSourceCommit", "commitId"); + if (head is not { Length: > 0 }) + throw new RefusedException($"Azure DevOps did not name a head commit for pull request {number}."); + string body = JsonSerializer.Serialize(new { + status = "completed", + lastMergeSourceCommit = new { commitId = head }, + completionOptions = new { + mergeStrategy = method switch { + "squash" => "squash", + "rebase" => "rebase", + _ => "noFastForward", + }, + deleteSourceBranch = deleteBranch, + }, + }); + using var result = await InvokeAsync("git", "pullRequests", "PATCH", new Dictionary { + ["project"] = project, + ["repositoryId"] = repo, + ["pullRequestId"] = number.ToString(), + }, body, ct); + Forget(number); + return $"completed with {method}"; + } + + /// Takes the pull request out of draft, which is what makes Azure DevOps start + /// evaluating its policies and asking its reviewers. + public async Task MarkReadyForReviewAsync(int number, CancellationToken ct = default) + { + Forget(number); + using var doc = await JsonAsync(ct, inProject: false, + "repos", "pr", "update", "--id", number.ToString(), "--draft", "false"); + return $"pull request {number} is ready for review"; + } + + /// Azure DevOps has no server-side update-branch: the branch is rebased where it + /// is checked out and pushed. + public Task UpdateBranchAsync(int number, CancellationToken ct = default) + => throw new RefusedException( + "Azure DevOps has no server-side update-branch; rebase the branch locally and push."); + + // ---- build logs ---------------------------------------------------------------------- + + /// + /// The logs of the failed steps of a build, in the shape `gh run view --log-failed` gives: + /// each failed record's log under a heading naming it. A build's timeline is what says + /// which records failed and where their logs are. + /// + public async Task GetFailedLogAsync(long runId, CancellationToken ct = default) + { + using var timeline = await InvokeAsync("build", "timeline", "GET", new Dictionary { + ["project"] = project, + ["buildId"] = runId.ToString(), + }, null, ct); + var text = new System.Text.StringBuilder(); + foreach (var record in Array(Node(timeline.RootElement, "records"))) + { + if (Str(record, "result") != "failed" || Node(record, "log", "id") is not { } logId) + continue; + text.AppendLine($"### {Str(record, "name") ?? "(unnamed step)"}"); + using var log = await InvokeAsync("build", "logs", "GET", new Dictionary { + ["project"] = project, + ["buildId"] = runId.ToString(), + ["logId"] = logId.GetInt32().ToString(), + }, null, ct); + foreach (var line in Array(Node(log.RootElement, "value"))) + text.AppendLine(line.GetString()); + text.AppendLine(); + } + return text.Length > 0 ? text.ToString() : "No failed step in this build has a log."; + } + + // ---- threads: comments, replies, resolution ------------------------------------------ + + /// + /// Azure DevOps numbers a thread's comments from 1 again in every thread, and the review + /// carries one number per comment. The two are packed into it and split apart wherever a + /// reply or a resolution has to name the thread again. + /// + public static long PackId(int threadId, int commentId) => threadId * 1_000_000L + commentId; + + public static (int Thread, int Comment) SplitId(long packed) + => ((int)(packed / 1_000_000L), (int)(packed % 1_000_000L)); + + Task ThreadsAsync(int number, CancellationToken ct) + => InvokeAsync("git", "pullRequestThreads", "GET", new Dictionary { + ["project"] = project, + ["repositoryId"] = repo, + ["pullRequestId"] = number.ToString(), + }, null, ct); + + public async Task> GetReviewCommentsAsync(int number, + CancellationToken ct = default) + { + using var doc = await ThreadsAsync(number, ct); + string? prUrl = await PrUrlAsync(number, ct); + var iterationCommits = await IterationCommitsAsync(number, ct); + var comments = new List(); + foreach (var thread in Array(Node(doc.RootElement, "value"))) + { + // A thread with no file is the pull request's own conversation, which this review + // does not show - as it does not show GitHub's issue comments. + if (Node(thread, "threadContext", "filePath") is not { ValueKind: JsonValueKind.String } pathNode) + continue; + int threadId = Node(thread, "id")?.GetInt32() ?? 0; + string path = (pathNode.GetString() ?? "").TrimStart('/'); + var right = Node(thread, "threadContext", "rightFileStart", "line"); + var left = Node(thread, "threadContext", "leftFileStart", "line"); + string side = right is not null ? "RIGHT" : "LEFT"; + int? line = (right ?? left)?.GetInt32(); + // Which revision the thread was written against: Azure DevOps names it by iteration + // rather than by commit, and the iterations say which commit each one was. + string? commit = Node(thread, "pullRequestThreadContext", "iterationContext", + "secondComparingIteration") is { ValueKind: JsonValueKind.Number } iteration + && iterationCommits.TryGetValue(iteration.GetInt32(), out string? sha) + ? sha + : null; + foreach (var comment in Array(Node(thread, "comments"))) + { + if (Str(comment, "commentType") != "text" + || Node(comment, "isDeleted")?.ValueKind == JsonValueKind.True) + continue; + comments.Add(new PostedComment( + PackId(threadId, Node(comment, "id")?.GetInt32() ?? 0), + Str(comment, "content") ?? "", + path, + line, + side, + new PostedUser(Str(comment, "author", "uniqueName") + ?? Str(comment, "author", "displayName") ?? ""), + // Azure DevOps tracks a thread's line across iterations itself, so the line + // above is the line as the change stands now and never has to be located in + // a hunk the way a GitHub comment's original line does. + OriginalLine: line, + DiffHunk: null, + OriginalCommitId: commit, + HtmlUrl: prUrl is null ? null : $"{prUrl}?discussionId={threadId}")); + } + } + return comments; + } + + /// Which commit each iteration of the pull request was, so a thread pinned to an + /// iteration can name the revision it was written against. + async Task> IterationCommitsAsync(int number, CancellationToken ct) + { + using var doc = await InvokeAsync("git", "pullRequestIterations", "GET", new Dictionary { + ["project"] = project, + ["repositoryId"] = repo, + ["pullRequestId"] = number.ToString(), + }, null, ct); + var commits = new Dictionary(); + foreach (var iteration in Array(Node(doc.RootElement, "value"))) + { + if (Node(iteration, "id")?.GetInt32() is { } id + && Str(iteration, "sourceRefCommit", "commitId") is { Length: > 0 } sha) + commits[id] = sha; + } + return commits; + } + + /// + /// Every vote cast on the pull request, as a review each. Azure DevOps does not record which + /// commit a vote was cast on - its "reset votes on push" policy is how a repository makes a + /// vote mean the head it was cast on - so the overview's stale-review marker never shows. + /// + public async Task> GetReviewsAsync(int number, CancellationToken ct = default) + { + Forget(number); + var pr = (await PrAsync(number, ct)).RootElement; + return [.. Array(Node(pr, "reviewers")) + .Where(r => Vote(r) != 0) + .Select(r => new PrReview( + new PostedUser(Str(r, "uniqueName") ?? Str(r, "displayName") ?? ""), + Vote(r) >= 5 ? "APPROVED" : "CHANGES_REQUESTED", + CommitId: null, + SubmittedAt: null))]; + } + + public async Task> GetThreadResolutionsAsync(int number, + CancellationToken ct = default) + { + using var doc = await ThreadsAsync(number, ct); + var resolutions = new List(); + foreach (var thread in Array(Node(doc.RootElement, "value"))) + { + int threadId = Node(thread, "id")?.GetInt32() ?? 0; + var ids = Array(Node(thread, "comments")) + .Where(c => Str(c, "commentType") == "text") + .Select(c => PackId(threadId, Node(c, "id")?.GetInt32() ?? 0)) + .ToList(); + resolutions.Add(new ThreadResolution( + $"{number}/{threadId}", + Str(thread, "status") is "fixed" or "closed" or "wontFix" or "byDesign", + ids)); + } + return resolutions; + } + + /// + /// Resolving a thread names the pull request as well as the thread, and the review hands a + /// resolution nothing but the thread id it was given - so the id it is given is both, + /// written "pullRequest/thread". GitHub's thread id is an opaque string too, which is why + /// this fits through the same member. + /// + public async Task SetThreadResolvedAsync(string threadId, bool resolved, CancellationToken ct = default) + { + string[] parts = threadId.Split('/'); + if (parts.Length != 2) + throw new RefusedException($"Not an Azure DevOps thread id: {threadId}"); + string body = JsonSerializer.Serialize(new { status = resolved ? "fixed" : "active" }); + using var _ = await InvokeAsync("git", "pullRequestThreads", "PATCH", new Dictionary { + ["project"] = project, + ["repositoryId"] = repo, + ["pullRequestId"] = parts[0], + ["threadId"] = parts[1], + }, body, ct); + } + + // ---- writing ------------------------------------------------------------------------- + + /// + /// Azure DevOps has no review object: a review is its comments and a vote. The comments are + /// posted first, so a failure never leaves a vote standing with no reasons written down; a + /// failure part-way says how many went through, which is what tells the reader what to look + /// for on the site. + /// + public async Task SubmitReviewAsync(int number, ReviewSubmission submission, + CancellationToken ct = default) + { + var attributed = ReviewAttribution.Attributed(submission); + int posted = 0; + try + { + foreach (var comment in attributed.Comments) + { + await PostThreadAsync(number, comment.Body, comment.Path, comment.Line, comment.Side, ct); + posted++; + } + if (attributed.Body.Trim().Length > 0) + { + await PostThreadAsync(number, attributed.Body, path: null, line: null, side: null, ct); + posted++; + } + } + catch (Exception) + { + if (posted > 0) + { + CliLog.Write("host", + $"{posted} comment(s) were posted before the failure; no vote was cast"); + } + throw; + } + string? vote = attributed.Event switch { + // "reject" is stronger than GitHub's request-changes: it blocks completion outright + // where a request for changes is a reviewer asking. "wait-for-author" is the one + // that means the same thing. + "APPROVE" => "approve", + "REQUEST_CHANGES" => "wait-for-author", + _ => null, + }; + if (vote is null) + return; + Forget(number); + await ExternalTool.RunAsync("az", + ["repos", "pr", "set-vote", "--id", number.ToString(), "--vote", vote, .. OrgArgs], repoPath, ct); + } + + async Task PostThreadAsync(int number, string body, string? path, int? line, string? side, + CancellationToken ct) + { + var comments = new[] { new { content = body, commentType = "text" } }; + // The side decides which pair of positions the thread carries: a comment on a removed + // line belongs to the left file, one on anything else to the right. Azure DevOps reads + // a thread with neither as the pull request's own conversation, which is what a review + // body is. + var position = new { line = line ?? 1, offset = 1 }; + var context = new Dictionary { ["filePath"] = "/" + path }; + if (side == "LEFT") + { + context["leftFileStart"] = position; + context["leftFileEnd"] = position; + } + else + { + context["rightFileStart"] = position; + context["rightFileEnd"] = position; + } + object thread = path is null + ? new { comments, status = "active" } + : new { comments, status = "active", threadContext = context }; + using var _ = await InvokeAsync("git", "pullRequestThreads", "POST", new Dictionary { + ["project"] = project, + ["repositoryId"] = repo, + ["pullRequestId"] = number.ToString(), + }, JsonSerializer.Serialize(thread), ct); + } + + public async Task ReplyToCommentAsync(int number, long commentId, string body, + CancellationToken ct = default) + { + var (thread, comment) = SplitId(commentId); + string json = JsonSerializer.Serialize(new { + content = body, + parentCommentId = comment, + commentType = "text", + }); + using var _ = await InvokeAsync("git", "pullRequestThreadComments", "POST", + new Dictionary { + ["project"] = project, + ["repositoryId"] = repo, + ["pullRequestId"] = number.ToString(), + ["threadId"] = thread.ToString(), + }, json, ct); + } + + // ---- merge queue --------------------------------------------------------------------- + + /// Nothing on Azure DevOps empties the queue on its own, so whichever window is + /// open drains it - as on a GitHub repository without the drainer workflow. + public Task HasMergeQueueWorkflowAsync(CancellationToken ct = default) => Task.FromResult(false); + + public Task DispatchMergeQueueAsync(CancellationToken ct = default) => Task.CompletedTask; +} diff --git a/src/Stampeded.Core/AzureDevOps/AzureDevOpsUrl.cs b/src/Stampeded.Core/AzureDevOps/AzureDevOpsUrl.cs new file mode 100644 index 0000000..6c69c73 --- /dev/null +++ b/src/Stampeded.Core/AzureDevOps/AzureDevOpsUrl.cs @@ -0,0 +1,74 @@ +using System.Text.RegularExpressions; + +namespace Stampeded.Core.AzureDevOps; + +/// +/// Parses Azure DevOps repository and pull-request URLs into organization, project, repository +/// and an optional pull-request id. Four shapes reach a reader: what the portal's clone button +/// writes, the old visualstudio.com host it still answers on, its DefaultCollection form, and +/// the ssh remote. +/// +public static partial class AzureDevOpsUrl +{ + // A project and a repository may both contain spaces, which is why the segments are matched + // as "anything but a slash" and decoded afterwards rather than spelled out as a character + // class. Anything past the repository - or past the pull request - is the tab the browser + // happened to be on, and names nothing. + [GeneratedRegex( + @"^(?:(?:https?://)?(?:[^/@]+@)?dev\.azure\.com/(?[^/]+)" + + @"|(?:https?://)?(?:[^/@]+@)?(?[^./]+)\.visualstudio\.com(?:/DefaultCollection)?" + + @"|git@ssh\.dev\.azure\.com:v3/(?[^/]+))" + + @"/(?[^/]+)/(?:_git/)?(?[^/]+?)(?:\.git)?" + + @"(?:/pullrequest/(?\d+))?(?:/.*)?$", + RegexOptions.IgnoreCase)] + private static partial Regex Pattern(); + + public static bool TryParse(string input, out string org, out string project, out string repo, + out int? prNumber) + { + org = project = repo = ""; + prNumber = null; + string text = input.Trim(); + // A copied link carries the page's state with it - "?_a=files", a discussion anchor - + // and none of that is part of the address. + int state = text.IndexOfAny(['#', '?']); + if (state >= 0) + text = text[..state]; + var match = Pattern().Match(text); + if (!match.Success) + return false; + // The ssh form has no _git segment, so a three-segment path there is org/project/repo + // and the same regex serves both. + org = Uri.UnescapeDataString(First(match, "org", "org2", "org3")); + project = Uri.UnescapeDataString(match.Groups["project"].Value); + repo = Uri.UnescapeDataString(match.Groups["repo"].Value); + if (match.Groups["pr"].Success) + prNumber = int.Parse(match.Groups["pr"].Value); + return org.Length > 0 && project.Length > 0 && repo.Length > 0; + } + + static string First(Match match, params string[] names) + => names.Select(n => match.Groups[n]).FirstOrDefault(g => g.Success)?.Value ?? ""; + + /// + /// True when any remote of a checkout names this repository, given the output of + /// `git config --get-regexp ^remote\..*\.url`. + /// + public static bool AnyRemoteMatches(string gitConfigOutput, string org, string project, string repo) + { + foreach (string line in gitConfigOutput.Split('\n', StringSplitOptions.RemoveEmptyEntries)) + { + int space = line.IndexOf(' '); + if (space > 0 && RemoteMatches(line[(space + 1)..].Trim(), org, project, repo)) + return true; + } + return false; + } + + /// True when a git remote URL names the same organization, project and repository. + public static bool RemoteMatches(string remoteUrl, string org, string project, string repo) + => TryParse(remoteUrl, out string u, out string p, out string r, out _) + && string.Equals(u, org, StringComparison.OrdinalIgnoreCase) + && string.Equals(p, project, StringComparison.OrdinalIgnoreCase) + && string.Equals(r, repo, StringComparison.OrdinalIgnoreCase); +} diff --git a/src/Stampeded.Core/Git/GitService.cs b/src/Stampeded.Core/Git/GitService.cs index abf43e7..0e688a3 100644 --- a/src/Stampeded.Core/Git/GitService.cs +++ b/src/Stampeded.Core/Git/GitService.cs @@ -253,10 +253,12 @@ public async Task PinReviewHeadsAsync(string key, string head, string? previousH public Task FetchAsync(CancellationToken ct = default) => RunAsync(ct, "fetch", "origin"); - /// Fetches the PR head into refs/stampeded/pr/N and returns its SHA. - public async Task FetchPrHeadAsync(int number, CancellationToken ct = default) + /// Fetches the PR head into refs/stampeded/pr/N and returns its SHA. The refspec + /// comes from the host: GitHub advertises every pull request's head as a ref of its own, + /// Azure DevOps does not and the source branch is fetched instead. + public async Task FetchPrHeadAsync(string refspec, int number, CancellationToken ct = default) { - await RunAsync(ct, "fetch", "origin", $"+refs/pull/{number}/head:refs/stampeded/pr/{number}"); + await RunAsync(ct, "fetch", "origin", refspec); return (await RunAsync(ct, "rev-parse", $"refs/stampeded/pr/{number}")).Trim(); } @@ -292,10 +294,12 @@ public async Task> ListWorktreesAsync(Cancellati return checkouts; } - /// Whether a checkout has changes that are not committed - staged, unstaged - /// or untracked. + /// Whether a checkout has changes that are not committed - staged or unstaged. + /// Untracked files do not count: they are not part of the change under review, and a + /// checkout that has nothing but build output in it is not a review step. public async Task IsDirtyAsync(string worktreePath, CancellationToken ct = default) - => (await ExternalTool.RunAsync("git", ["status", "--porcelain"], worktreePath, ct)).Trim().Length > 0; + => (await ExternalTool.RunAsync( + "git", ["status", "--porcelain", "--untracked-files=no"], worktreePath, ct)).Trim().Length > 0; /// /// Every file a revision has, repository-relative. Read from the object database rather @@ -309,26 +313,15 @@ public async Task> ListFilesAsync(string revision, Cancell } /// - /// A checkout's current contents against a commit: everything `git diff <base>` reports - /// (staged and unstaged alike, since the comparison is with the working tree), plus the - /// untracked files, which that diff omits and which are read individually so the index - /// is never touched. + /// A checkout's current contents against a commit: everything `git diff <base>` reports, + /// staged and unstaged alike, since the comparison is with the working tree. Untracked + /// files are not in it - git does not track them and neither does a review. /// public async Task> DiffWorkingTreeAsync( string worktreePath, string baseRev, CancellationToken ct = default) { var files = GitDiffParser.Parse(await ExternalTool.RunAsync( "git", ["diff", "-U3", "--find-renames", baseRev], worktreePath, ct)).ToList(); - string untracked = await ExternalTool.RunAsync( - "git", ["ls-files", "--others", "--exclude-standard"], worktreePath, ct); - foreach (var relPath in untracked.ReplaceLineEndings("\n").Split('\n', StringSplitOptions.RemoveEmptyEntries)) - { - // --no-index reports "differences found" as exit 1, which is the normal case here. - string diff = await ExternalTool.RunAsync( - "git", ["diff", "-U3", "--no-index", "--", "/dev/null", relPath], - worktreePath, ct, okExitCodes: [1]); - files.AddRange(GitDiffParser.Parse(diff)); - } return [.. files.OrderBy(f => f.Path, StringComparer.Ordinal)]; } diff --git a/src/Stampeded.Core/GitHub/GitHubService.cs b/src/Stampeded.Core/GitHub/GitHubService.cs index 2f50b81..84be18a 100644 --- a/src/Stampeded.Core/GitHub/GitHubService.cs +++ b/src/Stampeded.Core/GitHub/GitHubService.cs @@ -4,358 +4,10 @@ using System.Text.Json.Serialization; using Stampeded.Core.Infra; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.GitHub; -public sealed record PrAuthor(string Login); - -/// One reviewer's last word on a pull request, as `latestReviews` hands it over. -public sealed record PrLatestReview(PrAuthor? Author, string? State); - -/// The account a repository belongs to, as `headRepositoryOwner` hands it over. -public sealed record PrRepoOwner(string Login); - -/// Someone a review has been asked of. A team has no login, which is why this is -/// not a : `reviewRequests` holds both. -public sealed record PrReviewRequest(string? Login); - -public sealed record PrSummary( - int Number, - string Title, - PrAuthor? Author, - string HeadRefName, - string BaseRefName, - bool IsDraft, - DateTimeOffset UpdatedAt, - System.Text.Json.JsonElement? StatusCheckRollup = null, - string? HeadRefOid = null, - string? ReviewDecision = null, - int Additions = 0, - int Deletions = 0, - int ChangedFiles = 0, - IReadOnlyList? LatestReviews = null, - IReadOnlyList? ReviewRequests = null, - PrRepoOwner? HeadRepositoryOwner = null) -{ - /// The login gh is authenticated as, stamped on after the list is read: only - /// that tells "approved" apart from "approved by the reader". - public string? ViewerLogin { get; init; } - - /// The owner origin belongs to, stamped on after the list is read: a head branch - /// name means nothing without it, because a pull request lists the branch as it is named - /// in the repository it lives in, which for a fork is not this one. - public string? OriginOwner { get; init; } - - /// The head branch is in somebody's fork, so no branch of this clone is that - /// branch however alike the two are named - "master" from a fork is not the master that - /// is checked out here. Owner alone decides it: a fork cannot sit beside its original - /// under the same account. - public bool HeadIsFork => OriginOwner is { Length: > 0 } origin - && HeadRepositoryOwner is { Login.Length: > 0 } head - && !string.Equals(head.Login, origin, StringComparison.OrdinalIgnoreCase); - - /// "fail" / "pending" / "green" / "none", folded from the check rollup. - public string ChecksBucket => CheckRollup.Bucket(StatusCheckRollup); - - public bool ChecksFailed => ChecksBucket == "fail"; - public bool ChecksPending => ChecksBucket == "pending"; - public bool ChecksGreen => ChecksBucket == "green"; - - public bool IsApproved => ReviewDecision == "APPROVED"; - public bool ChangesRequested => ReviewDecision == "CHANGES_REQUESTED"; - - /// The reader's own last review approved this. A pull request can be approved - /// without their vote, and voted on without the pull request being approved, so this is - /// read from the reviews rather than from the decision. - public bool ApprovedByMe => ViewerLogin is { Length: > 0 } me - && LatestReviews?.Any(r => r.State == "APPROVED" - && string.Equals(r.Author?.Login, me, StringComparison.OrdinalIgnoreCase)) == true; - - /// A review has been asked of the reader by name. Only of them: a request sent - /// to a team they are in is a request nobody in particular has to answer, and gh names the - /// team rather than its members. - public bool ReviewRequestedFromMe => ViewerLogin is { Length: > 0 } me - && ReviewRequests?.Any(r => string.Equals(r.Login, me, StringComparison.OrdinalIgnoreCase)) == true; - - /// Approved, but not by the reader - so the two badges never both show. - public bool ApprovedByOthers => IsApproved && !ApprovedByMe; - - public string NumberDisplay => $"#{Number}"; - - /// The size of the change, as GitHub counts it. Kept to the line totals: this - /// shares a line with the branches, and the file count is in the tooltip. - public string AddedDisplay => $"+{Additions}"; - - public string RemovedDisplay => $"-{Deletions}"; - - /// The whole of the branch line, for when the column is too narrow to show it. - public string BranchesTip => $"{HeadRefName} -> {BaseRefName}, by {Author?.Login ?? "unknown"}"; - - public string StatsTip => $"{ChangedFiles} changed file(s), {Additions} line(s) added, " - + $"{Deletions} removed, as GitHub counts them"; -} - -public sealed record PrDetail( - int Number, - string Title, - string? Body, - string BaseRefName, - string HeadRefName, - string State, - PrAuthor? Author, - bool IsDraft = false); - -public sealed record CheckRun(string Name, string State, string Bucket, string? Link, string? Workflow); - -/// -/// A pull request's status-check rollup as GitHub hands it over: check runs carry status and -/// conclusion, the older status contexts carry state, and both kinds arrive in one list. Read -/// in one place because the pull request list and the merge state fold it the same way, and -/// two foldings that drift apart would have the same review reported green in one pane and -/// failing in another. -/// -public static class CheckRollup -{ - /// "fail", "pending" or "green" for one entry. Anything not named is green: - /// SUCCESS, but also SKIPPED and NEUTRAL, which are not a check saying no. - public static string Verdict(System.Text.Json.JsonElement item) - { - string? conclusion = item.TryGetProperty("conclusion", out var c) ? c.GetString() : null; - string? state = item.TryGetProperty("state", out var s) ? s.GetString() : null; - return ((conclusion is { Length: > 0 } ? conclusion : state) ?? "").ToUpperInvariant() switch { - "FAILURE" or "ERROR" or "TIMED_OUT" or "STARTUP_FAILURE" or "CANCELLED" or "ACTION_REQUIRED" => "fail", - "" or "PENDING" or "IN_PROGRESS" or "QUEUED" or "EXPECTED" or "WAITING" or "REQUESTED" => "pending", - _ => "green", - }; - } - - /// "fail" / "pending" / "green" / "none" for the whole rollup, worst first. - public static string Bucket(System.Text.Json.JsonElement? rollup) - { - bool pending = false; - foreach (var item in Entries(rollup)) - { - switch (Verdict(item)) - { - case "fail": - return "fail"; - case "pending": - pending = true; - break; - } - } - return pending ? "pending" : Entries(rollup).Any() ? "green" : "none"; - } - - /// The checks with one verdict, named, in the order GitHub listed them. - public static IReadOnlyList Names(System.Text.Json.JsonElement? rollup, string verdict) - => [.. Entries(rollup).Where(item => Verdict(item) == verdict).Select(Name)]; - - static string Name(System.Text.Json.JsonElement item) - => (item.TryGetProperty("name", out var name) ? name.GetString() : null) - ?? (item.TryGetProperty("context", out var context) ? context.GetString() : null) - ?? "(unnamed check)"; - - static IEnumerable Entries(System.Text.Json.JsonElement? rollup) - => rollup is { ValueKind: System.Text.Json.JsonValueKind.Array } array - ? array.EnumerateArray() - : []; -} - -/// -/// Whether GitHub would take a merge of this pull request right now, in its own words: -/// is MERGEABLE / CONFLICTING / UNKNOWN, -/// is CLEAN, UNSTABLE, BLOCKED, BEHIND, DIRTY, DRAFT, HAS_HOOKS or UNKNOWN. -/// -public sealed record MergeState( - string? Mergeable, - string? MergeStateStatus, - string? ReviewDecision = null, - bool IsDraft = false, - string? BaseRefName = null, - System.Text.Json.JsonElement? StatusCheckRollup = null, - string? State = null, - string? HeadRefOid = null) -{ - /// - /// UNSTABLE is a failing or pending check on a pull request GitHub would still merge, so - /// it is the reader's call and not a refusal. BLOCKED, BEHIND and DIRTY are refusals whose - /// remedy is not a merge; UNKNOWN is what GitHub answers without push access, and offering - /// a button that will be rejected is worse than not offering one. - /// - public bool CanMerge => Mergeable == "MERGEABLE" - && MergeStateStatus is "CLEAN" or "UNSTABLE" or "HAS_HOOKS"; - - /// GitHub's own two words, for the line that quotes it rather than reads it. - public string Describe => $"{Mergeable ?? "UNKNOWN"} / {MergeStateStatus ?? "UNKNOWN"}"; - - /// - /// What is actually in the way, in the fewest words that let the reader decide what to do. - /// - /// "MERGEABLE / BLOCKED" is GitHub answering a different question: it names the kind of - /// refusal, not the thing to wait for or fix, and the two most common reasons behind it - - /// a check still running and a review not given - are indistinguishable in it. Both are - /// in the fields alongside, so they are read here and the raw pair is kept for the tooltip. - /// - /// Ordered by what the reader would do about it: what they must fix first, then what they - /// are waiting on, then what somebody else owes them. - /// - public string Summary - { - get - { - string target = BaseRefName is { Length: > 0 } ? BaseRefName : "the target branch"; - string status = MergeStateStatus is { Length: > 0 } ? MergeStateStatus : "UNKNOWN"; - var reasons = new List(); - if (Mergeable == "CONFLICTING" || status == "DIRTY") - reasons.Add($"conflicts with {target}"); - if (IsDraft || status == "DRAFT") - reasons.Add("still a draft"); - if (status == "BEHIND") - reasons.Add($"behind {target}"); - if (CheckRollup.Names(StatusCheckRollup, "fail") is { Count: > 0 } failing) - reasons.Add($"{failing.Count} check{(failing.Count == 1 ? "" : "s")} failing"); - if (CheckRollup.Names(StatusCheckRollup, "pending") is { Count: > 0 } running) - reasons.Add($"{running.Count} check{(running.Count == 1 ? "" : "s")} still running"); - if (ReviewDecision == "CHANGES_REQUESTED") - reasons.Add("changes requested"); - else if (ReviewDecision == "REVIEW_REQUIRED") - reasons.Add("no approving review yet"); - - if (reasons.Count > 0) - // Two at most: a third is detail the tooltip already carries in full. - return string.Join(", ", reasons.Take(2)); - if (status == "BLOCKED") - return "blocked by a rule this account cannot read"; - if (status == "UNKNOWN") - return "GitHub has not worked it out yet"; - return CanMerge ? "nothing blocks it" : Describe; - } - } - - /// - /// Why the merge would be refused, in as much detail as GitHub gives from here. Its two - /// words say the kind of refusal; what the reader needs is which of the several things - /// behind that word is missing, and GitHub answers that in other fields - the review - /// decision, the checks, the draft flag - which are read here alongside it. - /// - /// BLOCKED is the one it will not always explain: a rule can require a check that has not - /// reported at all, or a code-owner review, and neither shows up in what a reader can see. - /// Saying so is better than listing nothing and looking broken. - /// - public string Explain - { - get - { - string target = BaseRefName is { Length: > 0 } ? BaseRefName : "the target branch"; - // A field GitHub left out is a state it does not know, which is what UNKNOWN means. - string status = MergeStateStatus is { Length: > 0 } ? MergeStateStatus : "UNKNOWN"; - var lines = new List { $"GitHub says: {Describe}." }; - if (Mergeable == "CONFLICTING" || MergeStateStatus == "DIRTY") - { - lines.Add($"The branch conflicts with {target}. Rebase it onto {target}, or merge " - + $"{target} into it, and push."); - } - switch (status) - { - case "BEHIND": - lines.Add($"The branch is behind {target}, and this repository requires it to be " - + "up to date before a merge. Rebase it and push."); - break; - case "DRAFT": - lines.Add("The pull request is a draft. It has to be marked ready for review."); - break; - case "BLOCKED": - lines.Add($"A branch protection rule on {target} refuses it."); - break; - case "UNSTABLE": - lines.Add("GitHub would take it as it is; a check is failing or has not finished, " - + "and whether that matters is the reader's call."); - break; - case "UNKNOWN": - lines.Add("GitHub has not worked the state out yet, or this account has no push " - + "access to the repository. Refreshing in a moment usually answers it."); - break; - } - var reasons = new List(); - if (ReviewDecision == "REVIEW_REQUIRED") - reasons.Add("No approving review yet."); - else if (ReviewDecision == "CHANGES_REQUESTED") - reasons.Add("A review has requested changes."); - if (CheckRollup.Names(StatusCheckRollup, "fail") is { Count: > 0 } failing) - reasons.Add($"Checks failing: {string.Join(", ", failing)}."); - if (CheckRollup.Names(StatusCheckRollup, "pending") is { Count: > 0 } running) - reasons.Add($"Checks not finished: {string.Join(", ", running)}."); - if (IsDraft && status != "DRAFT") - reasons.Add("The pull request is a draft."); - if (reasons.Count == 0 && status == "BLOCKED") - { - reasons.Add("Which rule is not visible from here: a required check that has not " - + "reported, a review from a code owner, or a rule this account cannot read."); - } - lines.AddRange(reasons.Select(r => "- " + r)); - if (CanMerge && reasons.Count == 0 && status is "CLEAN" or "HAS_HOOKS") - lines.Add("Nothing blocks it."); - return string.Join("\n", lines); - } - } -} - -/// The merge methods the repository's settings allow. -public sealed record MergeMethods(bool MergeCommitAllowed, bool SquashMergeAllowed, bool RebaseMergeAllowed) -{ - /// The gh flags for the allowed methods, in the order GitHub's own menu lists them. - public IReadOnlyList Allowed - { - get - { - var methods = new List(); - if (MergeCommitAllowed) - methods.Add("merge"); - if (SquashMergeAllowed) - methods.Add("squash"); - if (RebaseMergeAllowed) - methods.Add("rebase"); - return methods; - } - } -} - -public sealed record PostedUser(string Login); - -public sealed record PostedComment( - long Id, - string Body, - string Path, - int? Line, - string? Side, - PostedUser? User, - [property: JsonPropertyName("original_line")] int? OriginalLine, - [property: JsonPropertyName("diff_hunk")] string? DiffHunk, - /// The commit the comment was written against. Still in the object database - /// whenever that head was ever fetched, which is what lets the code it was about be read - /// as it was. - [property: JsonPropertyName("original_commit_id")] string? OriginalCommitId, - [property: JsonPropertyName("html_url")] string? HtmlUrl = null); - -/// One submitted review of a pull request: who, what they said of it, and the head -/// they said it of - which is not always the one on screen. -public sealed record PrReview( - PostedUser? User, - string? State, - [property: JsonPropertyName("commit_id")] string? CommitId, - [property: JsonPropertyName("submitted_at")] DateTimeOffset? SubmittedAt); - -/// Resolution state of one GitHub review thread and the REST ids of its comments. -public sealed record ThreadResolution(string ThreadId, bool IsResolved, IReadOnlyList CommentIds); - -public sealed record ReviewCommentDto(string Path, int Line, string Side, string Body); - -public sealed record ReviewSubmission(string Body, string Event, IReadOnlyList Comments); - -/// The whole payload of a reply: the thread it joins is named by the URL. -public sealed record ReplyBody(string Body); - [JsonSourceGenerationOptions(PropertyNameCaseInsensitive = true)] [JsonSerializable(typeof(IReadOnlyList))] [JsonSerializable(typeof(PrDetail))] @@ -374,8 +26,36 @@ partial class GitHubJsonContext : JsonSerializerContext /// GitHub access through the `gh` CLI, run in the repository directory so gh resolves /// the repo from origin. Auth, SSO and token refresh ride on the user's gh login. /// -public sealed class GitHubService(string repoPath) +public sealed partial class GitHubService(string repoPath) : IPullRequestHost { + public string Name => "GitHub"; + + /// GitHub refuses an approval from the pull request's own author outright. + public bool AcceptsOwnApproval => false; + + /// Every pull request's head is a ref of the repository itself, fork or not. + public Task PrHeadRefspecAsync(int number, CancellationToken ct = default) + => Task.FromResult($"+refs/pull/{number}/head:refs/stampeded/pr/{number}"); + + public Task PrUrlAsync(int number, CancellationToken ct = default) + => WebUrlAsync($"pull/{number}", ct); + + public Task CommitUrlAsync(string sha, CancellationToken ct = default) + => WebUrlAsync($"commit/{sha}", ct); + + async Task WebUrlAsync(string suffix, CancellationToken ct) + { + try + { + var (owner, repo) = await GetOwnerRepoAsync(ct); + return $"https://github.com/{owner}/{repo}/{suffix}"; + } + catch (ToolFailedException) + { + return null; + } + } + static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web) { TypeInfoResolver = GitHubJsonContext.Default, }; @@ -433,9 +113,21 @@ public async Task> GetChecksAsync(int number, Cancellati throw new ToolFailedException("gh", result.ExitCode, result.StandardError); return []; } - return JsonSerializer.Deserialize>(result.StandardOutput, JsonOptions) ?? []; + var checks = JsonSerializer.Deserialize>(result.StandardOutput, JsonOptions) ?? []; + return [.. checks.Select(c => c with { RunId = RunIdOf(c.Link) })]; } + /// The run a check's link points at, which is what its failed log is fetched by. + /// Only a GitHub Actions link carries one; a check reported by anything else links + /// somewhere whose logs gh cannot fetch, and opens nothing. + public static long? RunIdOf(string? link) + => link is not null && ActionsRunUrl().Match(link) is { Success: true } m + ? long.Parse(m.Groups[1].Value) + : null; + + [System.Text.RegularExpressions.GeneratedRegex(@"/actions/runs/(\d+)")] + private static partial System.Text.RegularExpressions.Regex ActionsRunUrl(); + /// What GitHub says about merging this pull request right now. Not cached: it /// changes with every push to either branch and with every review someone else leaves. public Task GetMergeStateAsync(int number, CancellationToken ct = default) @@ -620,18 +312,8 @@ public async Task SetThreadResolvedAsync(string threadId, bool resolved, Cancell /// Where an issue number of this repository points, or null when the repository /// is not on GitHub - a review of a local branch in a clone with no such remote. - public async Task GetIssueUrlPrefixAsync(CancellationToken ct = default) - { - try - { - var (owner, repo) = await GetOwnerRepoAsync(ct); - return $"https://github.com/{owner}/{repo}/issues/"; - } - catch (ToolFailedException) - { - return null; - } - } + public Task GetIssueUrlPrefixAsync(CancellationToken ct = default) + => WebUrlAsync("issues/", ct); async Task<(string Owner, string Repo)> GetOwnerRepoAsync(CancellationToken ct) { @@ -670,7 +352,7 @@ public async Task UpdateBranchAsync(int number, CancellationToken ct = default) /// Submits a review (APPROVE / REQUEST_CHANGES / COMMENT) with line comments. public async Task SubmitReviewAsync(int number, ReviewSubmission submission, CancellationToken ct = default) { - string json = JsonSerializer.Serialize(Attributed(submission), JsonOptions); + string json = JsonSerializer.Serialize(ReviewAttribution.Attributed(submission), JsonOptions); var result = await CliWrap.Cli.Wrap("gh") .WithArguments(["api", "-X", "POST", $"repos/{{owner}}/{{repo}}/pulls/{number}/reviews", "--input", "-"]) .WithWorkingDirectory(repoPath) @@ -704,38 +386,4 @@ public async Task ReplyToCommentAsync(int number, long commentId, string body, C if (result.ExitCode != 0) throw new ToolFailedException("gh", result.ExitCode, result.StandardError + result.StandardOutput); } - - /// - /// Marks a review as posted by this tool, once, on the first thing a reader will meet. - /// That is the first line comment - the file view, a thread and a mail notification all - /// show those, and none of them shows the summary the comments were batched into. With no - /// line comments the summary is what carries it instead. - /// - /// An approval or a rejection with nothing written at all is left alone: the mark would be - /// the entire review, which says who ran it and nothing about the change. - /// - public static ReviewSubmission Attributed(ReviewSubmission submission) - { - if (submission.Comments.Count > 0) - { - return submission with { - Comments = [ - submission.Comments[0] with { Body = WithAttribution(submission.Comments[0].Body) }, - .. submission.Comments.Skip(1), - ], - }; - } - return submission.Body.Trim().Length == 0 - ? submission - : submission with { Body = WithAttribution(submission.Body) }; - } - - /// The mark for a pass that is nothing but replies: those are posted one by one - /// rather than as a review, so the review body that would otherwise carry it is never - /// sent, and the first reply is the first thing a reader will meet. - public static string AttributedReply(string body) => WithAttribution(body); - - static string WithAttribution(string body) - => (body.Length > 0 ? body.TrimEnd() + "\n\n" : "") - + "*Reviewed with [Stampeded!](https://github.com/icsharpcode/Stampeded)*"; } diff --git a/src/Stampeded.Core/Infra/ExternalTool.cs b/src/Stampeded.Core/Infra/ExternalTool.cs index f0288f8..5771758 100644 --- a/src/Stampeded.Core/Infra/ExternalTool.cs +++ b/src/Stampeded.Core/Infra/ExternalTool.cs @@ -39,16 +39,29 @@ public static async Task RunAsync( IReadOnlyDictionary? env = null, IReadOnlyList? okExitCodes = null) { var watch = System.Diagnostics.Stopwatch.StartNew(); - var result = await CliWrap.Cli.Wrap(exe) - .WithArguments(args) - .WithWorkingDirectory(workingDir) - .WithEnvironmentVariables(builder => { - StripMsBuildLocatorVariables(builder); - foreach (var (key, value) in env ?? System.Collections.Immutable.ImmutableDictionary.Empty) - builder.Set(key, value); - }) - .WithValidation(CommandResultValidation.None) - .ExecuteBufferedAsync(ct); + CliWrap.Buffered.BufferedCommandResult result; + try + { + result = await CliWrap.Cli.Wrap(exe) + .WithArguments(args) + .WithWorkingDirectory(workingDir) + .WithEnvironmentVariables(builder => { + StripMsBuildLocatorVariables(builder); + foreach (var (key, value) in env ?? System.Collections.Immutable.ImmutableDictionary.Empty) + builder.Set(key, value); + }) + .WithValidation(CommandResultValidation.None) + .ExecuteBufferedAsync(ct); + } + catch (System.ComponentModel.Win32Exception) + { + // The tool is not installed, or not on this process's PATH. Every caller is written + // to report a command that failed; one that never started would otherwise escape + // as an exception nobody catches and leave a pane loading forever. + CliLog.Write(exe, $"{string.Join(' ', args)} -> {exe} did not start"); + throw new ToolFailedException(exe, -1, + $"{exe} is not installed, or not on PATH."); + } string argsText = string.Join(' ', args); if (argsText.Length > 160) argsText = argsText[..160] + "..."; diff --git a/src/Stampeded.Core/MergeQueue/MergeQueueService.cs b/src/Stampeded.Core/MergeQueue/MergeQueueService.cs index 0f1e8b5..e1ba796 100644 --- a/src/Stampeded.Core/MergeQueue/MergeQueueService.cs +++ b/src/Stampeded.Core/MergeQueue/MergeQueueService.cs @@ -1,7 +1,7 @@ using System.Text.Json; using Stampeded.Core.Git; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; using Stampeded.Core.Infra; namespace Stampeded.Core.MergeQueue; @@ -40,7 +40,7 @@ public sealed record MergeQueueDriveResult(string Status, IReadOnlyList<(int Pr, /// /// Who this client calls itself in the queue. Left out, it is asked of /// gh once and remembered. -public sealed class MergeQueueService(GitService git, GitHubService gitHub, string? identity = null) +public sealed class MergeQueueService(GitService git, IPullRequestHost host, string? identity = null) { /// /// Where the queue lives on the remote. Outside refs/heads and refs/tags on purpose: no @@ -310,7 +310,7 @@ public async Task DriveOnceAsync( MergeState state; try { - state = await gitHub.GetMergeStateAsync(entry.Pr, ct); + state = await host.GetMergeStateAsync(entry.Pr, ct); } catch (ToolFailedException ex) { @@ -345,7 +345,7 @@ public async Task DriveOnceAsync( try { progress?.Report(new MergeQueueProgress(entry.Pr, $"merging ({entry.Method})", Working: true)); - await gitHub.MergePrAsync(entry.Pr, entry.Method, entry.DeleteBranch, ct); + await host.MergePrAsync(entry.Pr, entry.Method, entry.DeleteBranch, ct); CliLog.Write("mergequeue", $"merged #{entry.Pr} by {entry.Method}"); await UpdateAsync(doc => ( doc with { @@ -386,11 +386,11 @@ void Pass(int pr, string reason) /// public async Task NudgeDrainerAsync(CancellationToken ct = default) { - if (!await gitHub.HasMergeQueueWorkflowAsync(ct)) + if (!await host.HasMergeQueueWorkflowAsync(ct)) return false; try { - await gitHub.DispatchMergeQueueAsync(ct); + await host.DispatchMergeQueueAsync(ct); } catch (ToolFailedException ex) { @@ -401,7 +401,7 @@ public async Task NudgeDrainerAsync(CancellationToken ct = default) /// Whether a workflow on GitHub empties this queue. public Task HasDrainerAsync(CancellationToken ct = default) - => gitHub.HasMergeQueueWorkflowAsync(ct); + => host.HasMergeQueueWorkflowAsync(ct); /// Who this client is, in the queue's own words. The GitHub login says which /// person and the machine name says which of their windows; without the login - offline, or @@ -413,7 +413,7 @@ public async Task HolderAsync(CancellationToken ct = default) string login; try { - login = await gitHub.GetViewerLoginAsync(ct); + login = await host.GetViewerLoginAsync(ct); } catch (ToolFailedException) { diff --git a/src/Stampeded.Core/PullRequests/IPullRequestHost.cs b/src/Stampeded.Core/PullRequests/IPullRequestHost.cs new file mode 100644 index 0000000..34f3bf6 --- /dev/null +++ b/src/Stampeded.Core/PullRequests/IPullRequestHost.cs @@ -0,0 +1,65 @@ +namespace Stampeded.Core.PullRequests; + +/// +/// Every pull-request fact a review needs, from whichever host the repository is on. GitHub's +/// vocabulary is the vocabulary of the model - APPROVE / REQUEST_CHANGES / COMMENT, APPROVED / +/// CHANGES_REQUESTED, LEFT / RIGHT, MERGEABLE / BLOCKED - because the panes and the pure +/// functions under them already speak it; another host maps onto it in its own implementation +/// and nowhere else. +/// +public interface IPullRequestHost +{ + /// "GitHub" / "Azure DevOps", for every status line, dialog and menu that names it. + string Name { get; } + + /// Whether the host takes an approval from the pull request's own author. GitHub + /// refuses it, Azure DevOps does not. + bool AcceptsOwnApproval { get; } + + /// The refspec that fetches pull request 's head from + /// origin into refs/stampeded/pr/N. + Task PrHeadRefspecAsync(int number, CancellationToken ct = default); + + /// Where a browser would show the pull request, or null when the repository is not + /// on this host - a review of a local branch in a clone with no such remote. + Task PrUrlAsync(int number, CancellationToken ct = default); + + /// Where a browser would show one commit, or null as above. + Task CommitUrlAsync(string sha, CancellationToken ct = default); + + Task GetViewerLoginAsync(CancellationToken ct = default); + Task GetDefaultBranchAsync(CancellationToken ct = default); + Task> ListOpenPrsAsync(CancellationToken ct = default); + Task GetPrAsync(int number, CancellationToken ct = default); + Task> GetChecksAsync(int number, CancellationToken ct = default); + Task GetMergeStateAsync(int number, CancellationToken ct = default); + Task GetIssueTitleAsync(int number, CancellationToken ct = default); + Task GetIssueUrlPrefixAsync(CancellationToken ct = default); + Task GetMergeMethodsAsync(CancellationToken ct = default); + Task MergePrAsync(int number, string method, bool deleteBranch = false, CancellationToken ct = default); + Task MarkReadyForReviewAsync(int number, CancellationToken ct = default); + + /// Log lines of the failed steps of one run: GitHub Actions' run id, Azure DevOps' + /// build id, as carries it. + Task GetFailedLogAsync(long runId, CancellationToken ct = default); + + Task> GetReviewCommentsAsync(int number, CancellationToken ct = default); + Task> GetReviewsAsync(int number, CancellationToken ct = default); + Task> GetThreadResolutionsAsync(int number, CancellationToken ct = default); + Task SetThreadResolvedAsync(string threadId, bool resolved, CancellationToken ct = default); + + /// Rebases the pull request's branch onto its target on the server. A host without + /// such an API throws saying so. + Task UpdateBranchAsync(int number, CancellationToken ct = default); + + Task SubmitReviewAsync(int number, ReviewSubmission submission, CancellationToken ct = default); + Task ReplyToCommentAsync(int number, long commentId, string body, CancellationToken ct = default); + + /// Whether something on the host empties the merge queue without a reader's window + /// being open - GitHub's drainer workflow. A host with no such thing answers false, and the + /// queue is drained by whoever has the window open, as it is on a repository without one. + Task HasMergeQueueWorkflowAsync(CancellationToken ct = default); + + /// Tells that drainer there is something to do. Does nothing where there is none. + Task DispatchMergeQueueAsync(CancellationToken ct = default); +} diff --git a/src/Stampeded.Core/GitHub/IssueLinks.cs b/src/Stampeded.Core/PullRequests/IssueLinks.cs similarity index 97% rename from src/Stampeded.Core/GitHub/IssueLinks.cs rename to src/Stampeded.Core/PullRequests/IssueLinks.cs index cfd92a1..ddb891c 100644 --- a/src/Stampeded.Core/GitHub/IssueLinks.cs +++ b/src/Stampeded.Core/PullRequests/IssueLinks.cs @@ -1,6 +1,6 @@ using System.Text.RegularExpressions; -namespace Stampeded.Core.GitHub; +namespace Stampeded.Core.PullRequests; /// /// Turns "#1234" into a link, the way GitHub renders it everywhere its own text appears. diff --git a/src/Stampeded.Core/GitHub/PrCache.cs b/src/Stampeded.Core/PullRequests/PrCache.cs similarity index 98% rename from src/Stampeded.Core/GitHub/PrCache.cs rename to src/Stampeded.Core/PullRequests/PrCache.cs index f9b7c73..29c947f 100644 --- a/src/Stampeded.Core/GitHub/PrCache.cs +++ b/src/Stampeded.Core/PullRequests/PrCache.cs @@ -3,7 +3,7 @@ using Stampeded.Core.Infra; -namespace Stampeded.Core.GitHub; +namespace Stampeded.Core.PullRequests; /// /// What GitHub said about a pull request the last time it could be reached: enough to open the diff --git a/src/Stampeded.Core/PullRequests/PullRequestHosts.cs b/src/Stampeded.Core/PullRequests/PullRequestHosts.cs new file mode 100644 index 0000000..f102ba6 --- /dev/null +++ b/src/Stampeded.Core/PullRequests/PullRequestHosts.cs @@ -0,0 +1,42 @@ +using Stampeded.Core.AzureDevOps; +using Stampeded.Core.GitHub; +using Stampeded.Core.Infra; + +namespace Stampeded.Core.PullRequests; + +/// Decides which host a checkout's pull requests live on, once per workspace. +public static class PullRequestHosts +{ + /// + /// The host for a checkout, from origin's URL. Anything origin's URL does not name as Azure + /// DevOps is GitHub on purpose: gh also serves GitHub Enterprise hosts, which nothing here + /// can enumerate, and a clone with no origin at all behaves as it always did. + /// STAMPEDED_PR_HOST=github|azdo overrides the decision. + /// + public static async Task ForAsync(string repoPath, CancellationToken ct = default) + { + string origin = await OriginUrlAsync(repoPath, ct); + string? forced = Environment.GetEnvironmentVariable("STAMPEDED_PR_HOST"); + bool azdo = AzureDevOpsUrl.TryParse(origin, out string org, out string project, out string repo, out _); + if (forced is { Length: > 0 }) + { + CliLog.Write("host", $"STAMPEDED_PR_HOST={forced}"); + azdo = forced.Equals("azdo", StringComparison.OrdinalIgnoreCase); + } + if (!azdo) + { + CliLog.Write("host", "origin is GitHub"); + return new GitHubService(repoPath); + } + CliLog.Write("host", $"origin is Azure DevOps ({org}/{project}/{repo})"); + return new AzureDevOpsService(repoPath, org, project, repo); + } + + static async Task OriginUrlAsync(string repoPath, CancellationToken ct) + { + // Exit 1 is git saying the key is not set: a clone that was never pushed anywhere, or + // a review of local work. That is an answer, not a failure. + return (await ExternalTool.RunAsync("git", ["config", "--get", "remote.origin.url"], repoPath, ct, + okExitCodes: [1])).Trim(); + } +} diff --git a/src/Stampeded.Core/PullRequests/PullRequestModels.cs b/src/Stampeded.Core/PullRequests/PullRequestModels.cs new file mode 100644 index 0000000..98d42ea --- /dev/null +++ b/src/Stampeded.Core/PullRequests/PullRequestModels.cs @@ -0,0 +1,359 @@ +using System.Text.Json.Serialization; + +namespace Stampeded.Core.PullRequests; + +public sealed record PrAuthor(string Login); + +/// One reviewer's last word on a pull request, as `latestReviews` hands it over. +public sealed record PrLatestReview(PrAuthor? Author, string? State); + +/// The account a repository belongs to, as `headRepositoryOwner` hands it over. +public sealed record PrRepoOwner(string Login); + +/// Someone a review has been asked of. A team has no login, which is why this is +/// not a : `reviewRequests` holds both. +public sealed record PrReviewRequest(string? Login); + +public sealed record PrSummary( + int Number, + string Title, + PrAuthor? Author, + string HeadRefName, + string BaseRefName, + bool IsDraft, + DateTimeOffset UpdatedAt, + System.Text.Json.JsonElement? StatusCheckRollup = null, + string? HeadRefOid = null, + string? ReviewDecision = null, + int Additions = 0, + int Deletions = 0, + int ChangedFiles = 0, + IReadOnlyList? LatestReviews = null, + IReadOnlyList? ReviewRequests = null, + PrRepoOwner? HeadRepositoryOwner = null) +{ + /// The login gh is authenticated as, stamped on after the list is read: only + /// that tells "approved" apart from "approved by the reader". + public string? ViewerLogin { get; init; } + + /// The owner origin belongs to, stamped on after the list is read: a head branch + /// name means nothing without it, because a pull request lists the branch as it is named + /// in the repository it lives in, which for a fork is not this one. + public string? OriginOwner { get; init; } + + /// The head branch is in somebody's fork, so no branch of this clone is that + /// branch however alike the two are named - "master" from a fork is not the master that + /// is checked out here. Owner alone decides it: a fork cannot sit beside its original + /// under the same account. + public bool HeadIsFork => OriginOwner is { Length: > 0 } origin + && HeadRepositoryOwner is { Login.Length: > 0 } head + && !string.Equals(head.Login, origin, StringComparison.OrdinalIgnoreCase); + + /// "fail" / "pending" / "green" / "none", folded from the check rollup. + public string ChecksBucket => CheckRollup.Bucket(StatusCheckRollup); + + public bool ChecksFailed => ChecksBucket == "fail"; + public bool ChecksPending => ChecksBucket == "pending"; + public bool ChecksGreen => ChecksBucket == "green"; + + public bool IsApproved => ReviewDecision == "APPROVED"; + public bool ChangesRequested => ReviewDecision == "CHANGES_REQUESTED"; + + /// The reader's own last review approved this. A pull request can be approved + /// without their vote, and voted on without the pull request being approved, so this is + /// read from the reviews rather than from the decision. + public bool ApprovedByMe => ViewerLogin is { Length: > 0 } me + && LatestReviews?.Any(r => r.State == "APPROVED" + && string.Equals(r.Author?.Login, me, StringComparison.OrdinalIgnoreCase)) == true; + + /// A review has been asked of the reader by name. Only of them: a request sent + /// to a team they are in is a request nobody in particular has to answer, and gh names the + /// team rather than its members. + public bool ReviewRequestedFromMe => ViewerLogin is { Length: > 0 } me + && ReviewRequests?.Any(r => string.Equals(r.Login, me, StringComparison.OrdinalIgnoreCase)) == true; + + /// Approved, but not by the reader - so the two badges never both show. + public bool ApprovedByOthers => IsApproved && !ApprovedByMe; + + public string NumberDisplay => $"#{Number}"; + + /// The size of the change, as GitHub counts it. Kept to the line totals: this + /// shares a line with the branches, and the file count is in the tooltip. + public string AddedDisplay => $"+{Additions}"; + + public string RemovedDisplay => $"-{Deletions}"; + + /// The whole of the branch line, for when the column is too narrow to show it. + public string BranchesTip => $"{HeadRefName} -> {BaseRefName}, by {Author?.Login ?? "unknown"}"; + + public string StatsTip => $"{ChangedFiles} changed file(s), {Additions} line(s) added, " + + $"{Deletions} removed, as GitHub counts them"; +} + +public sealed record PrDetail( + int Number, + string Title, + string? Body, + string BaseRefName, + string HeadRefName, + string State, + PrAuthor? Author, + bool IsDraft = false); + +/// One check on a pull request. names the run whose failed log +/// can be fetched - GitHub Actions' run id, Azure DevOps' build id - and is null for a check +/// that reports from somewhere neither can read. +public sealed record CheckRun(string Name, string State, string Bucket, string? Link, string? Workflow, + long? RunId = null); + +/// +/// A pull request's status-check rollup as GitHub hands it over: check runs carry status and +/// conclusion, the older status contexts carry state, and both kinds arrive in one list. Read +/// in one place because the pull request list and the merge state fold it the same way, and +/// two foldings that drift apart would have the same review reported green in one pane and +/// failing in another. +/// +public static class CheckRollup +{ + /// "fail", "pending" or "green" for one entry. Anything not named is green: + /// SUCCESS, but also SKIPPED and NEUTRAL, which are not a check saying no. + public static string Verdict(System.Text.Json.JsonElement item) + { + string? conclusion = item.TryGetProperty("conclusion", out var c) ? c.GetString() : null; + string? state = item.TryGetProperty("state", out var s) ? s.GetString() : null; + return ((conclusion is { Length: > 0 } ? conclusion : state) ?? "").ToUpperInvariant() switch { + "FAILURE" or "ERROR" or "TIMED_OUT" or "STARTUP_FAILURE" or "CANCELLED" or "ACTION_REQUIRED" => "fail", + "" or "PENDING" or "IN_PROGRESS" or "QUEUED" or "EXPECTED" or "WAITING" or "REQUESTED" => "pending", + _ => "green", + }; + } + + /// "fail" / "pending" / "green" / "none" for the whole rollup, worst first. + public static string Bucket(System.Text.Json.JsonElement? rollup) + { + bool pending = false; + foreach (var item in Entries(rollup)) + { + switch (Verdict(item)) + { + case "fail": + return "fail"; + case "pending": + pending = true; + break; + } + } + return pending ? "pending" : Entries(rollup).Any() ? "green" : "none"; + } + + /// The checks with one verdict, named, in the order GitHub listed them. + public static IReadOnlyList Names(System.Text.Json.JsonElement? rollup, string verdict) + => [.. Entries(rollup).Where(item => Verdict(item) == verdict).Select(Name)]; + + static string Name(System.Text.Json.JsonElement item) + => (item.TryGetProperty("name", out var name) ? name.GetString() : null) + ?? (item.TryGetProperty("context", out var context) ? context.GetString() : null) + ?? "(unnamed check)"; + + static IEnumerable Entries(System.Text.Json.JsonElement? rollup) + => rollup is { ValueKind: System.Text.Json.JsonValueKind.Array } array + ? array.EnumerateArray() + : []; +} + +/// +/// Whether GitHub would take a merge of this pull request right now, in its own words: +/// is MERGEABLE / CONFLICTING / UNKNOWN, +/// is CLEAN, UNSTABLE, BLOCKED, BEHIND, DIRTY, DRAFT, HAS_HOOKS or UNKNOWN. +/// +public sealed record MergeState( + string? Mergeable, + string? MergeStateStatus, + string? ReviewDecision = null, + bool IsDraft = false, + string? BaseRefName = null, + System.Text.Json.JsonElement? StatusCheckRollup = null, + string? State = null, + string? HeadRefOid = null) +{ + /// The host these two words came from, for the lines that quote it by name. + public string Host { get; init; } = "GitHub"; + + /// + /// UNSTABLE is a failing or pending check on a pull request GitHub would still merge, so + /// it is the reader's call and not a refusal. BLOCKED, BEHIND and DIRTY are refusals whose + /// remedy is not a merge; UNKNOWN is what GitHub answers without push access, and offering + /// a button that will be rejected is worse than not offering one. + /// + public bool CanMerge => Mergeable == "MERGEABLE" + && MergeStateStatus is "CLEAN" or "UNSTABLE" or "HAS_HOOKS"; + + /// GitHub's own two words, for the line that quotes it rather than reads it. + public string Describe => $"{Mergeable ?? "UNKNOWN"} / {MergeStateStatus ?? "UNKNOWN"}"; + + /// + /// What is actually in the way, in the fewest words that let the reader decide what to do. + /// + /// "MERGEABLE / BLOCKED" is GitHub answering a different question: it names the kind of + /// refusal, not the thing to wait for or fix, and the two most common reasons behind it - + /// a check still running and a review not given - are indistinguishable in it. Both are + /// in the fields alongside, so they are read here and the raw pair is kept for the tooltip. + /// + /// Ordered by what the reader would do about it: what they must fix first, then what they + /// are waiting on, then what somebody else owes them. + /// + public string Summary + { + get + { + string target = BaseRefName is { Length: > 0 } ? BaseRefName : "the target branch"; + string status = MergeStateStatus is { Length: > 0 } ? MergeStateStatus : "UNKNOWN"; + var reasons = new List(); + if (Mergeable == "CONFLICTING" || status == "DIRTY") + reasons.Add($"conflicts with {target}"); + if (IsDraft || status == "DRAFT") + reasons.Add("still a draft"); + if (status == "BEHIND") + reasons.Add($"behind {target}"); + if (CheckRollup.Names(StatusCheckRollup, "fail") is { Count: > 0 } failing) + reasons.Add($"{failing.Count} check{(failing.Count == 1 ? "" : "s")} failing"); + if (CheckRollup.Names(StatusCheckRollup, "pending") is { Count: > 0 } running) + reasons.Add($"{running.Count} check{(running.Count == 1 ? "" : "s")} still running"); + if (ReviewDecision == "CHANGES_REQUESTED") + reasons.Add("changes requested"); + else if (ReviewDecision == "REVIEW_REQUIRED") + reasons.Add("no approving review yet"); + + if (reasons.Count > 0) + // Two at most: a third is detail the tooltip already carries in full. + return string.Join(", ", reasons.Take(2)); + if (status == "BLOCKED") + return "blocked by a rule this account cannot read"; + if (status == "UNKNOWN") + return $"{Host} has not worked it out yet"; + return CanMerge ? "nothing blocks it" : Describe; + } + } + + /// + /// Why the merge would be refused, in as much detail as GitHub gives from here. Its two + /// words say the kind of refusal; what the reader needs is which of the several things + /// behind that word is missing, and GitHub answers that in other fields - the review + /// decision, the checks, the draft flag - which are read here alongside it. + /// + /// BLOCKED is the one it will not always explain: a rule can require a check that has not + /// reported at all, or a code-owner review, and neither shows up in what a reader can see. + /// Saying so is better than listing nothing and looking broken. + /// + public string Explain + { + get + { + string target = BaseRefName is { Length: > 0 } ? BaseRefName : "the target branch"; + // A field GitHub left out is a state it does not know, which is what UNKNOWN means. + string status = MergeStateStatus is { Length: > 0 } ? MergeStateStatus : "UNKNOWN"; + var lines = new List { $"{Host} says: {Describe}." }; + if (Mergeable == "CONFLICTING" || MergeStateStatus == "DIRTY") + { + lines.Add($"The branch conflicts with {target}. Rebase it onto {target}, or merge " + + $"{target} into it, and push."); + } + switch (status) + { + case "BEHIND": + lines.Add($"The branch is behind {target}, and this repository requires it to be " + + "up to date before a merge. Rebase it and push."); + break; + case "DRAFT": + lines.Add("The pull request is a draft. It has to be marked ready for review."); + break; + case "BLOCKED": + lines.Add($"A branch protection rule on {target} refuses it."); + break; + case "UNSTABLE": + lines.Add($"{Host} would take it as it is; a check is failing or has not finished, " + + "and whether that matters is the reader's call."); + break; + case "UNKNOWN": + lines.Add($"{Host} has not worked the state out yet, or this account has no push " + + "access to the repository. Refreshing in a moment usually answers it."); + break; + } + var reasons = new List(); + if (ReviewDecision == "REVIEW_REQUIRED") + reasons.Add("No approving review yet."); + else if (ReviewDecision == "CHANGES_REQUESTED") + reasons.Add("A review has requested changes."); + if (CheckRollup.Names(StatusCheckRollup, "fail") is { Count: > 0 } failing) + reasons.Add($"Checks failing: {string.Join(", ", failing)}."); + if (CheckRollup.Names(StatusCheckRollup, "pending") is { Count: > 0 } running) + reasons.Add($"Checks not finished: {string.Join(", ", running)}."); + if (IsDraft && status != "DRAFT") + reasons.Add("The pull request is a draft."); + if (reasons.Count == 0 && status == "BLOCKED") + { + reasons.Add("Which rule is not visible from here: a required check that has not " + + "reported, a review from a code owner, or a rule this account cannot read."); + } + lines.AddRange(reasons.Select(r => "- " + r)); + if (CanMerge && reasons.Count == 0 && status is "CLEAN" or "HAS_HOOKS") + lines.Add("Nothing blocks it."); + return string.Join("\n", lines); + } + } +} + +/// The merge methods the repository's settings allow. +public sealed record MergeMethods(bool MergeCommitAllowed, bool SquashMergeAllowed, bool RebaseMergeAllowed) +{ + /// The gh flags for the allowed methods, in the order GitHub's own menu lists them. + public IReadOnlyList Allowed + { + get + { + var methods = new List(); + if (MergeCommitAllowed) + methods.Add("merge"); + if (SquashMergeAllowed) + methods.Add("squash"); + if (RebaseMergeAllowed) + methods.Add("rebase"); + return methods; + } + } +} + +public sealed record PostedUser(string Login); + +public sealed record PostedComment( + long Id, + string Body, + string Path, + int? Line, + string? Side, + PostedUser? User, + [property: JsonPropertyName("original_line")] int? OriginalLine, + [property: JsonPropertyName("diff_hunk")] string? DiffHunk, + /// The commit the comment was written against. Still in the object database + /// whenever that head was ever fetched, which is what lets the code it was about be read + /// as it was. + [property: JsonPropertyName("original_commit_id")] string? OriginalCommitId, + [property: JsonPropertyName("html_url")] string? HtmlUrl = null); + +/// One submitted review of a pull request: who, what they said of it, and the head +/// they said it of - which is not always the one on screen. +public sealed record PrReview( + PostedUser? User, + string? State, + [property: JsonPropertyName("commit_id")] string? CommitId, + [property: JsonPropertyName("submitted_at")] DateTimeOffset? SubmittedAt); + +/// Resolution state of one GitHub review thread and the REST ids of its comments. +public sealed record ThreadResolution(string ThreadId, bool IsResolved, IReadOnlyList CommentIds); + +public sealed record ReviewCommentDto(string Path, int Line, string Side, string Body); + +public sealed record ReviewSubmission(string Body, string Event, IReadOnlyList Comments); + +/// The whole payload of a reply: the thread it joins is named by the URL. +public sealed record ReplyBody(string Body); diff --git a/src/Stampeded.Core/PullRequests/ReviewAttribution.cs b/src/Stampeded.Core/PullRequests/ReviewAttribution.cs new file mode 100644 index 0000000..a19edfc --- /dev/null +++ b/src/Stampeded.Core/PullRequests/ReviewAttribution.cs @@ -0,0 +1,40 @@ +namespace Stampeded.Core.PullRequests; + +/// Marks a review as posted by this tool. The mark is markdown, which every host +/// renders, so it belongs to the review rather than to the host that takes it. +public static class ReviewAttribution +{ + /// + /// Marks a review as posted by this tool, once, on the first thing a reader will meet. + /// That is the first line comment - the file view, a thread and a mail notification all + /// show those, and none of them shows the summary the comments were batched into. With no + /// line comments the summary is what carries it instead. + /// + /// An approval or a rejection with nothing written at all is left alone: the mark would be + /// the entire review, which says who ran it and nothing about the change. + /// + public static ReviewSubmission Attributed(ReviewSubmission submission) + { + if (submission.Comments.Count > 0) + { + return submission with { + Comments = [ + submission.Comments[0] with { Body = WithAttribution(submission.Comments[0].Body) }, + .. submission.Comments.Skip(1), + ], + }; + } + return submission.Body.Trim().Length == 0 + ? submission + : submission with { Body = WithAttribution(submission.Body) }; + } + + /// The mark for a pass that is nothing but replies: those are posted one by one + /// rather than as a review, so the review body that would otherwise carry it is never + /// sent, and the first reply is the first thing a reader will meet. + public static string AttributedReply(string body) => WithAttribution(body); + + static string WithAttribution(string body) + => (body.Length > 0 ? body.TrimEnd() + "\n\n" : "") + + "*Reviewed with [Stampeded!](https://github.com/icsharpcode/Stampeded)*"; +} diff --git a/src/Stampeded.Core/Review/ReviewVerdicts.cs b/src/Stampeded.Core/Review/ReviewVerdicts.cs index 4f51eb0..f89c368 100644 --- a/src/Stampeded.Core/Review/ReviewVerdicts.cs +++ b/src/Stampeded.Core/Review/ReviewVerdicts.cs @@ -1,4 +1,4 @@ -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Review; diff --git a/src/Stampeded/App.axaml.cs b/src/Stampeded/App.axaml.cs index 5cae3ea..e4c44b3 100644 --- a/src/Stampeded/App.axaml.cs +++ b/src/Stampeded/App.axaml.cs @@ -5,9 +5,12 @@ using Avalonia.Controls.ApplicationLifetimes; using Avalonia.Markup.Xaml; +using Stampeded.Core.AzureDevOps; using Stampeded.Core.GitHub; using Stampeded.Core.Infra; +using Stampeded.Core.PullRequests; + namespace Stampeded; public class App : Application @@ -36,6 +39,7 @@ public static async Task OpenRepositoryAsync(string path, int? prNumber = null) CliLog.Write("action", $"open repository {path}"); Workspace?.Shutdown(); Program.RepoPath = path; + Program.Host = await PullRequestHosts.ForAsync(path); window.DataContext = new MainViewModel(); if (prNumber is { } pr) await (Workspace?.OpenPrAsync(pr) ?? Task.CompletedTask); @@ -58,7 +62,7 @@ static bool IsRepository(string path) /// internal static (string? Folder, bool Answered) NextFolderAnswer; - static async Task AskWhereToCloneAsync(Window window, string owner, string repo) + static async Task AskWhereToCloneAsync(Window window, string name) { if (NextFolderAnswer.Answered) { @@ -69,7 +73,7 @@ static bool IsRepository(string path) } string projects = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), "Projects"); var options = new Avalonia.Platform.Storage.FolderPickerOpenOptions { - Title = $"Clone {owner}/{repo} into which folder?", + Title = $"Clone {name} into which folder?", AllowMultiple = false, }; if (Directory.Exists(projects)) @@ -78,16 +82,44 @@ static bool IsRepository(string path) return picks.Count == 1 ? picks[0].Path.LocalPath : null; } - /// Opens a GitHub repo/PR URL: an already-cloned repository (origin remote - /// matched against the current and recent repos) is reused; otherwise the folder to clone - /// into is asked for, and gh makes a blobless partial clone there. + /// Opens a repository or pull-request URL of either host: an already-cloned + /// repository (any remote matched against the current and recent repos) is reused; + /// otherwise the folder to clone into is asked for, and a blobless partial clone is made + /// there. public static async Task OpenFromUrlAsync(string input) { CliLog.Write("action", $"open from URL {input}"); - if (!GitHubUrl.TryParse(input, out string owner, out string repo, out int? prNumber)) + int? prNumber; + // Azure DevOps first: its URLs have no scheme-less form GitHub's grammar refuses, and + // GitHub's - which also accepts a bare "owner/repo" - would read "dev.azure.com/org/..." + // as a repository called org owned by dev.azure.com. + string name, folder; + Func remoteMatches; + // The command that makes the clone, given the target directory - which is only known + // once the reader has said where it goes, and which each of the two spells in its own + // place on the line. + Func clone; + if (AzureDevOpsUrl.TryParse(input, out string org, out string project, out string adoRepo, out prNumber)) + { + name = $"{org}/{project}/{adoRepo}"; + folder = adoRepo; + remoteMatches = remotes => AzureDevOpsUrl.AnyRemoteMatches(remotes, org, project, adoRepo); + // az has no clone verb, and git's credential helper answers for the login. + clone = target => ("git", ["clone", "--filter=blob:none", + $"https://dev.azure.com/{Uri.EscapeDataString(org)}/{Uri.EscapeDataString(project)}" + + $"/_git/{Uri.EscapeDataString(adoRepo)}", target]); + } + else if (GitHubUrl.TryParse(input, out string owner, out string repo, out prNumber)) + { + name = $"{owner}/{repo}"; + folder = repo; + remoteMatches = remotes => GitHubUrl.AnyRemoteMatches(remotes, owner, repo); + clone = target => ("gh", ["repo", "clone", $"{owner}/{repo}", target, "--", "--filter=blob:none"]); + } + else { - CliLog.Write("action", $"not a GitHub repository or PR URL: {input}"); - Workspace?.PostStatus($"Not a GitHub repository or PR URL: {input}"); + CliLog.Write("action", $"not a GitHub or Azure DevOps repository or PR URL: {input}"); + Workspace?.PostStatus($"Not a GitHub or Azure DevOps repository or PR URL: {input}"); return; } var candidates = new List { Program.RepoPath }; @@ -100,9 +132,9 @@ public static async Task OpenFromUrlAsync(string input) // a fork of it is the right checkout for a URL naming either. string remotes = await ExternalTool.RunAsync( "git", ["-C", candidate, "config", "--get-regexp", @"^remote\..*\.url"], candidate); - if (GitHubUrl.AnyRemoteMatches(remotes, owner, repo)) + if (remoteMatches(remotes)) { - CliLog.Write("action", $"{owner}/{repo} is checked out at {candidate}"); + CliLog.Write("action", $"{name} is checked out at {candidate}"); await OpenRepositoryAsync(candidate, prNumber); return; } @@ -116,28 +148,29 @@ public static async Task OpenFromUrlAsync(string input) // user's business, not a guess about how their disk is arranged. if ((Current?.ApplicationLifetime as IClassicDesktopStyleApplicationLifetime)?.MainWindow is not { } window) return; - string? parent = await AskWhereToCloneAsync(window, owner, repo); + string? parent = await AskWhereToCloneAsync(window, name); if (parent is null) { - Workspace?.PostStatus($"Opening {owner}/{repo} cancelled: no folder chosen to clone into."); + Workspace?.PostStatus($"Opening {name} cancelled: no folder chosen to clone into."); return; } - string target = Path.Combine(parent, repo); + string target = Path.Combine(parent, folder); if (Directory.Exists(target) && !IsRepository(target)) - target = Path.Combine(parent, $"{owner}-{repo}"); + target = Path.Combine(parent, $"{name.Replace('/', '-')}"); if (!Directory.Exists(target)) { - using var busy = Workspace?.Busy.Begin($"Cloning {owner}/{repo}"); - Workspace?.PostStatus($"Cloning {owner}/{repo} into {target}..."); + using var busy = Workspace?.Busy.Begin($"Cloning {name}"); + Workspace?.PostStatus($"Cloning {name} into {target}..."); try { // Blobless partial clone: fast even for large repos; worktree checkouts // fetch missing blobs on demand. - await ExternalTool.RunAsync("gh", ["repo", "clone", $"{owner}/{repo}", target, "--", "--filter=blob:none"], parent); + var (tool, args) = clone(target); + await ExternalTool.RunAsync(tool, args, parent); } catch (ToolFailedException ex) { - CliLog.Write("action", $"clone of {owner}/{repo} failed: {ex.Message}"); + CliLog.Write("action", $"clone of {name} failed: {ex.Message}"); Workspace?.PostStatus($"Clone failed: {ExternalTool.Explain(ex)}"); return; } diff --git a/src/Stampeded/Documents/CommentThreads.cs b/src/Stampeded/Documents/CommentThreads.cs index af100dc..79f06e3 100644 --- a/src/Stampeded/Documents/CommentThreads.cs +++ b/src/Stampeded/Documents/CommentThreads.cs @@ -144,7 +144,7 @@ public Avalonia.Controls.Control Build(string key, ThreadData thread) if (comment.Url is { Length: > 0 } commentUrl) { var github = new Avalonia.Controls.Button { - Content = "GitHub", + Content = App.Workspace?.HostName ?? "GitHub", FontSize = 10, Padding = new Avalonia.Thickness(5, 1), Cursor = new Cursor(StandardCursorType.Hand), @@ -186,7 +186,7 @@ public Avalonia.Controls.Control Build(string key, ThreadData thread) // engine directly: a ScrollViewer inside an editor inline object would nest // scroll regions into every visual line and wreck scrolling performance. var rendered = ThreadMarkdownEngine.Transform( - Core.GitHub.IssueLinks.Autolink(comment.Body, App.Workspace?.IssueUrlPrefix)); + Core.PullRequests.IssueLinks.Autolink(comment.Body, App.Workspace?.IssueUrlPrefix)); // A remark about `Foo` written in bold is bold and about `Foo`; the renderer draws // one of the two and the markers of the other. Controls.MarkdownEmphasis.Repair(rendered); diff --git a/src/Stampeded/Documents/OverviewDocumentView.axaml b/src/Stampeded/Documents/OverviewDocumentView.axaml index cf4f158..7ef7100 100644 --- a/src/Stampeded/Documents/OverviewDocumentView.axaml +++ b/src/Stampeded/Documents/OverviewDocumentView.axaml @@ -145,7 +145,7 @@ @@ -157,7 +157,7 @@ @@ -50,11 +50,12 @@ - + + host to ask. --> @@ -162,12 +162,12 @@ - + + ToolTip.Tip="{Binding HostName, StringFormat='Take this pull request out of draft. {0} then asks for the reviews its rules require.'}" /> - + - @@ -259,7 +259,7 @@ - + @@ -360,7 +360,7 @@ ToolTip.Tip="Rebase this branch onto the default base. A branch no checkout has is rebased in a throwaway worktree; one that a checkout has is rebased there, so that checkout moves with it" /> - @@ -391,7 +391,7 @@ + ToolTip.Tip="The repository's default branch on the remote, and what everything else is reviewed and rebased against" /> - diff --git a/src/Stampeded/Documents/StartDocumentView.axaml.cs b/src/Stampeded/Documents/StartDocumentView.axaml.cs index 17c6f81..7004f2c 100644 --- a/src/Stampeded/Documents/StartDocumentView.axaml.cs +++ b/src/Stampeded/Documents/StartDocumentView.axaml.cs @@ -7,6 +7,8 @@ using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; + namespace Stampeded.Documents; public partial class StartDocumentView : UserControl @@ -214,10 +216,10 @@ void OnRowPrReview(object? sender, RoutedEventArgs e) vm.OpenPr(pr); } - void OnRowPrGitHub(object? sender, RoutedEventArgs e) + void OnRowPrHost(object? sender, RoutedEventArgs e) { if (Vm is { } vm && RowOf(sender) is { } pr) - vm.OpenPrOnGitHub(pr); + vm.OpenPrOnHost(pr); } void OnRowBranchReview(object? sender, RoutedEventArgs e) @@ -262,10 +264,10 @@ void OnRowPrPull(object? sender, RoutedEventArgs e) vm.PullPrBranch(pr); } - void OnRowBranchPrGitHub(object? sender, RoutedEventArgs e) + void OnRowBranchPrHost(object? sender, RoutedEventArgs e) { if (Vm is { } vm && RowOf(sender) is { } row) - vm.OpenBranchPrOnGitHub(row); + vm.OpenBranchPrOnHost(row); } void OnRowStashBranch(object? sender, RoutedEventArgs e) @@ -332,16 +334,16 @@ void OnPrEnqueue(object? sender, RoutedEventArgs e) vm.EnqueuePr(pr); } - void OnPrOpenOnGitHub(object? sender, RoutedEventArgs e) + void OnPrOpenOnHost(object? sender, RoutedEventArgs e) { if (Vm is { } vm && PrListBox.SelectedItem is PrSummary pr) - vm.OpenPrOnGitHub(pr); + vm.OpenPrOnHost(pr); } - void OnBranchPrOnGitHub(object? sender, RoutedEventArgs e) + void OnBranchPrOnHost(object? sender, RoutedEventArgs e) { if (Vm is { } vm && BranchList.SelectedItem is BranchRow row) - vm.OpenBranchPrOnGitHub(row); + vm.OpenBranchPrOnHost(row); } void OnRecentDoubleTapped(object? sender, TappedEventArgs e) @@ -372,7 +374,7 @@ async Task PromptUrlAsync() if (TopLevel.GetTopLevel(this) is not Window owner) return; string? url = await new TextPromptWindow("Open from URL", - "GitHub repository or pull request URL (also accepts owner/repo). A repository not cloned yet is cloned via gh into ~/Projects.", + "GitHub or Azure DevOps repository or pull request URL (also accepts owner/repo). A repository not cloned yet is cloned into a folder you pick.", "Open", "https://github.com/owner/repo/pull/123").ShowDialog(owner); if (!string.IsNullOrWhiteSpace(url)) await App.OpenFromUrlAsync(url); diff --git a/src/Stampeded/Documents/StartDocumentViewModel.cs b/src/Stampeded/Documents/StartDocumentViewModel.cs index 6aa2e97..bbf03ea 100644 --- a/src/Stampeded/Documents/StartDocumentViewModel.cs +++ b/src/Stampeded/Documents/StartDocumentViewModel.cs @@ -8,7 +8,7 @@ using Dock.Model.Mvvm.Controls; using Stampeded.Core.Git; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; using Stampeded.Core.Infra; using Stampeded.Panes; @@ -178,6 +178,9 @@ public sealed partial class StartState : ObservableObject public class StartDocumentViewModel : Document { readonly ReviewWorkspace workspace; + /// Whose pull request it is - "GitHub", "Azure DevOps" - for the headers and + /// tooltips that name the host. + public string HostName => workspace.HostName; bool openOverviewWhenReady; public StartState State { get; } = new(); @@ -699,7 +702,7 @@ async Task ReadyAsync() State.Status = $"Marking #{pr.Number} ready for review..."; try { - await workspace.GitHub.MarkReadyForReviewAsync(pr.Number); + await workspace.Host.MarkReadyForReviewAsync(pr.Number); CliLog.Write("action", $"marked #{pr.Number} ready for review"); State.Status = $"#{pr.Number} is ready for review."; await PrList.LoadAsync(); @@ -954,13 +957,13 @@ public void RebasePr(BranchRow row) public void OpenRecent(string path) => App.OpenRepositoryAsync(path).HandleExceptions(); - public void OpenPrOnGitHub(PrSummary pr) - => workspace.OpenOnGitHubAsync(pr.Number).HandleExceptions(); + public void OpenPrOnHost(PrSummary pr) + => workspace.OpenPrOnHostAsync(pr.Number).HandleExceptions(); - public void OpenBranchPrOnGitHub(BranchRow row) + public void OpenBranchPrOnHost(BranchRow row) { if (row.PrNumber is { } number) - workspace.OpenOnGitHubAsync(number).HandleExceptions(); + workspace.OpenPrOnHostAsync(number).HandleExceptions(); } void BeginPreparation() diff --git a/src/Stampeded/MainViewModel.cs b/src/Stampeded/MainViewModel.cs index 1e2fe9c..bcc2ff0 100644 --- a/src/Stampeded/MainViewModel.cs +++ b/src/Stampeded/MainViewModel.cs @@ -126,6 +126,10 @@ partial void OnZoomChanged(double value) ZoomPreference.Save(value); } + /// Whose pull request it is - "GitHub", "Azure DevOps" - for the menu items that + /// name the host. + public string HostName => App.Workspace?.HostName ?? "GitHub"; + public MainViewModel() { // Before anything reads the list. Both views that show it - this menu and the start @@ -136,7 +140,7 @@ public MainViewModel() ZoomState.Set(Zoom); RecentRepos.Record(Program.RepoPath); Recent = new(RecentRepos.Load()); - var workspace = new ReviewWorkspace(Program.RepoPath); + var workspace = new ReviewWorkspace(Program.RepoPath, Program.Host); Busy = workspace.Busy; App.Workspace = workspace; var factory = new StampededDockFactory(workspace); diff --git a/src/Stampeded/MainWindow.axaml b/src/Stampeded/MainWindow.axaml index 46abdb1..8785497 100644 --- a/src/Stampeded/MainWindow.axaml +++ b/src/Stampeded/MainWindow.axaml @@ -146,7 +146,7 @@ - + diff --git a/src/Stampeded/MainWindow.axaml.cs b/src/Stampeded/MainWindow.axaml.cs index 9b7849f..184c0d4 100644 --- a/src/Stampeded/MainWindow.axaml.cs +++ b/src/Stampeded/MainWindow.axaml.cs @@ -264,7 +264,7 @@ async Task GoToAsync() async Task PromptUrlAsync() { string? url = await new TextPromptWindow("Open from URL", - "GitHub repository or pull request URL (also accepts owner/repo). A repository not cloned yet is cloned via gh into ~/Projects.", + "GitHub or Azure DevOps repository or pull request URL (also accepts owner/repo). A repository not cloned yet is cloned into a folder you pick.", "Open", "https://github.com/owner/repo/pull/123").ShowDialog(this); if (!string.IsNullOrWhiteSpace(url)) await App.OpenFromUrlAsync(url); @@ -392,10 +392,10 @@ static void UsePassBaseline(PassBaselineKind kind) void OnContinueFromPrepare(object? s, RoutedEventArgs e) => App.Workspace?.StartPage?.ContinueNow(); - void OnOpenOnGitHub(object? s, EventArgs e) + void OnOpenOnHost(object? s, EventArgs e) { if (App.Workspace is { CurrentPr: { } pr } ws) - ws.OpenOnGitHubAsync(pr.Number).HandleExceptions(); + ws.OpenPrOnHostAsync(pr.Number).HandleExceptions(); } void OnShowSemanticLog(object? s, EventArgs e) diff --git a/src/Stampeded/Panes/ChecksPaneView.axaml b/src/Stampeded/Panes/ChecksPaneView.axaml index 830ac58..7592f7c 100644 --- a/src/Stampeded/Panes/ChecksPaneView.axaml +++ b/src/Stampeded/Panes/ChecksPaneView.axaml @@ -10,7 +10,7 @@ diff --git a/src/Stampeded/Panes/ChecksPaneViewModel.cs b/src/Stampeded/Panes/ChecksPaneViewModel.cs index 4ca6360..57d4ec4 100644 --- a/src/Stampeded/Panes/ChecksPaneViewModel.cs +++ b/src/Stampeded/Panes/ChecksPaneViewModel.cs @@ -1,11 +1,10 @@ using System.Collections.ObjectModel; -using System.Text.RegularExpressions; using CommunityToolkit.Mvvm.ComponentModel; using Dock.Model.Mvvm.Controls; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; using Stampeded.Core.Infra; namespace Stampeded.Panes; @@ -33,12 +32,12 @@ public sealed record CheckRow(CheckRun Check) /// CI check runs for the open PR's head; double-click a failed check to open its /// failed-step log as a document. /// -public partial class ChecksPaneViewModel : Tool +public class ChecksPaneViewModel : Tool { - [GeneratedRegex(@"/actions/runs/(\d+)")] - private static partial Regex RunIdFromLink(); - readonly ReviewWorkspace workspace; + /// Whose pull request it is - "GitHub", "Azure DevOps" - for the headers and + /// tooltips that name the host. + public string HostName => workspace.HostName; public ObservableCollection Items { get; } = []; public ChecksState State { get; } = new(); @@ -63,7 +62,7 @@ public async Task LoadAsync() State.Status = $"Loading checks for #{pr.Number}..."; try { - var checks = await workspace.GitHub.GetChecksAsync(pr.Number); + var checks = await workspace.Host.GetChecksAsync(pr.Number); workspace.SetChecks(checks); foreach (var check in checks.OrderBy(c => c.Bucket is "fail" or "cancel" ? 0 : c.Bucket == "pending" ? 1 : 2)) rows.Add(new CheckRow(check)); @@ -84,9 +83,10 @@ public async Task LoadAsync() public void Open(CheckRow row) { - if (row.Check.Link is not { } link || RunIdFromLink().Match(link) is not { Success: true } match) + // A check the host cannot name a run for - one reported by something neither GitHub + // Actions nor Azure Pipelines - has no log to fetch, and opens nothing. + if (row.Check.RunId is not { } runId) return; - long runId = long.Parse(match.Groups[1].Value); State.Status = $"Fetching failed log of run {runId}..."; OpenLogAsync(runId, row.Check.Name).HandleExceptions(); } @@ -95,7 +95,7 @@ async Task OpenLogAsync(long runId, string name) { try { - string log = await workspace.GitHub.GetFailedLogAsync(runId); + string log = await workspace.Host.GetFailedLogAsync(runId); if (string.IsNullOrWhiteSpace(log)) log = "(no failed steps in this run)"; workspace.OpenTextDocument($"cilog:{runId}", $"{name} (failed log)", log); diff --git a/src/Stampeded/Panes/CommentsPaneView.axaml b/src/Stampeded/Panes/CommentsPaneView.axaml index 091a66b..f3d9746 100644 --- a/src/Stampeded/Panes/CommentsPaneView.axaml +++ b/src/Stampeded/Panes/CommentsPaneView.axaml @@ -8,7 +8,7 @@ @@ -30,7 +30,7 @@ @@ -40,7 +40,7 @@ - + @@ -64,11 +64,12 @@ - + - @@ -98,7 +98,7 @@ pane where it is read should not be the one place that does not say it. The title follows it while there is room, and the whole line opens the pull request. --> diff --git a/src/Stampeded/Panes/ExplorerPaneView.axaml.cs b/src/Stampeded/Panes/ExplorerPaneView.axaml.cs index c298e06..99643d2 100644 --- a/src/Stampeded/Panes/ExplorerPaneView.axaml.cs +++ b/src/Stampeded/Panes/ExplorerPaneView.axaml.cs @@ -48,9 +48,9 @@ void OnPassBaselineDropdown(object? sender, Avalonia.Interactivity.RoutedEventAr void OnOpenVsCode(object? sender, Avalonia.Interactivity.RoutedEventArgs e) => Vm?.OpenInVsCode(); - void OnOpenOnGitHub(object? sender, Avalonia.Interactivity.RoutedEventArgs e) => Vm?.OpenPrOnGitHub(); + void OnOpenOnHost(object? sender, Avalonia.Interactivity.RoutedEventArgs e) => Vm?.OpenPrOnHost(); - void OnOpenCommitOnGitHub(object? sender, Avalonia.Interactivity.RoutedEventArgs e) => Vm?.OpenCommitOnGitHub(); + void OnOpenCommitOnHost(object? sender, Avalonia.Interactivity.RoutedEventArgs e) => Vm?.OpenCommitOnHost(); void OnOpenReview(object? sender, Avalonia.Interactivity.RoutedEventArgs e) => Vm?.OpenReview(); diff --git a/src/Stampeded/Panes/ExplorerPaneViewModel.cs b/src/Stampeded/Panes/ExplorerPaneViewModel.cs index 72723af..e71e74d 100644 --- a/src/Stampeded/Panes/ExplorerPaneViewModel.cs +++ b/src/Stampeded/Panes/ExplorerPaneViewModel.cs @@ -13,6 +13,9 @@ namespace Stampeded.Panes; public partial class ExplorerPaneViewModel : Tool { readonly ReviewWorkspace workspace; + /// Whose pull request it is - "GitHub", "Azure DevOps" - for the headers and + /// tooltips that name the host. + public string HostName => workspace.HostName; public PrFilesPaneViewModel Files { get; } public FileBrowserPaneViewModel Browser { get; } @@ -149,16 +152,16 @@ void UpdateCommitScope() // deciding about the change, not to reading it. public void OpenInVsCode() => workspace.OpenInVsCodeAsync(oldSide: false).HandleExceptions(); - public void OpenPrOnGitHub() + public void OpenPrOnHost() { if (workspace.CurrentPr is { } pr) - workspace.OpenOnGitHubAsync(pr.Number).HandleExceptions(); + workspace.OpenPrOnHostAsync(pr.Number).HandleExceptions(); } - public void OpenCommitOnGitHub() + public void OpenCommitOnHost() { if (ShortSha.Length > 0) - workspace.OpenCommitOnGitHubAsync(ShortSha).HandleExceptions(); + workspace.OpenCommitOnHostAsync(ShortSha).HandleExceptions(); } public void OpenReview() => workspace.OpenReviewDocument(); diff --git a/src/Stampeded/Panes/MergeQueuePaneViewModel.cs b/src/Stampeded/Panes/MergeQueuePaneViewModel.cs index cf9bd4f..e92b9a0 100644 --- a/src/Stampeded/Panes/MergeQueuePaneViewModel.cs +++ b/src/Stampeded/Panes/MergeQueuePaneViewModel.cs @@ -243,8 +243,8 @@ public async Task EnqueueAsync(int number, string title, string method, bool del using var scope = workspace.Busy.Begin($"Queueing #{number}"); try { - Note(number, "asking GitHub what it points at", working: true); - var state = await workspace.GitHub.GetMergeStateAsync(number); + Note(number, $"asking {workspace.HostName} what it points at", working: true); + var state = await workspace.Host.GetMergeStateAsync(number); // A draft is not up for merging, and queueing one only puts something in front of // everybody that can never reach the front. Ready for Review is the thing to press // first, and saying so is more use than queueing it and reporting a block every turn. @@ -256,7 +256,7 @@ public async Task EnqueueAsync(int number, string title, string method, bool del } if (state.HeadRefOid is not { Length: > 0 } head) { - Give(number, $"GitHub did not say what #{number} points at; it cannot be queued."); + Give(number, $"{workspace.HostName} did not say what #{number} points at; it cannot be queued."); return; } Note(number, "publishing to the remote", working: true); @@ -307,7 +307,7 @@ public async Task BreakLockAsync() $"{held.Holder} took the lock for #{held.Pr} {Ago(held.At)} ago and it has not run " + "out yet, so that window may still be merging.\n\n" + "Clearing it lets this window - or any other - start on the queue as well. " - + "GitHub refuses a second merge of one pull request, so the worst case is a " + + $"{workspace.HostName} refuses a second merge of one pull request, so the worst case is a " + "failed attempt rather than a double merge.", "Clear lock").ShowDialog(owner); if (!go) diff --git a/src/Stampeded/Panes/PrListPaneView.axaml b/src/Stampeded/Panes/PrListPaneView.axaml index d527a2d..9aa99c2 100644 --- a/src/Stampeded/Panes/PrListPaneView.axaml +++ b/src/Stampeded/Panes/PrListPaneView.axaml @@ -1,6 +1,6 @@ @@ -20,11 +20,11 @@ - + - + diff --git a/src/Stampeded/Panes/PrListPaneView.axaml.cs b/src/Stampeded/Panes/PrListPaneView.axaml.cs index c0717e4..d2011d6 100644 --- a/src/Stampeded/Panes/PrListPaneView.axaml.cs +++ b/src/Stampeded/Panes/PrListPaneView.axaml.cs @@ -4,6 +4,8 @@ using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; + namespace Stampeded.Panes; public partial class PrListPaneView : UserControl @@ -29,10 +31,10 @@ void OpenSelected() vm.Open(pr); } - void OnOpenOnGitHubClicked(object? sender, RoutedEventArgs e) + void OnOpenOnHostClicked(object? sender, RoutedEventArgs e) { if (PullRequestList.SelectedItem is PrSummary pr) - App.Workspace?.OpenOnGitHubAsync(pr.Number).HandleExceptions(); + App.Workspace?.OpenPrOnHostAsync(pr.Number).HandleExceptions(); } void OnRefreshClicked(object? sender, RoutedEventArgs e) diff --git a/src/Stampeded/Panes/PrListPaneViewModel.cs b/src/Stampeded/Panes/PrListPaneViewModel.cs index 7cf44fc..0c11b11 100644 --- a/src/Stampeded/Panes/PrListPaneViewModel.cs +++ b/src/Stampeded/Panes/PrListPaneViewModel.cs @@ -4,7 +4,7 @@ using Dock.Model.Mvvm.Controls; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; using Stampeded.Core.Infra; namespace Stampeded.Panes; @@ -26,6 +26,9 @@ public sealed partial class PrListState : ObservableObject public class PrListPaneViewModel : Tool { readonly ReviewWorkspace workspace; + /// Whose pull request it is - "GitHub", "Azure DevOps" - for the headers and + /// tooltips that name the host. + public string HostName => workspace.HostName; public ObservableCollection Items { get; } = []; public PrListState State { get; } = new(); @@ -76,12 +79,12 @@ public async Task LoadAsync() State.Status = $"Not a git repository: {workspace.RepoPath}"; return; } - var prs = await workspace.GitHub.ListOpenPrsAsync(); + var prs = await workspace.Host.ListOpenPrsAsync(); string? originOwner = await workspace.Git.GetOriginOwnerAsync(); string viewer = ""; try { - viewer = await workspace.GitHub.GetViewerLoginAsync(); + viewer = await workspace.Host.GetViewerLoginAsync(); } catch (ToolFailedException) { diff --git a/src/Stampeded/Program.cs b/src/Stampeded/Program.cs index b901efb..957afc2 100644 --- a/src/Stampeded/Program.cs +++ b/src/Stampeded/Program.cs @@ -1,5 +1,7 @@ using Avalonia; +using Stampeded.Core.PullRequests; + namespace Stampeded; internal static class Program @@ -8,6 +10,13 @@ internal static class Program /// changed at runtime by "Open Repository". public static string RepoPath { get; set; } = Environment.CurrentDirectory; + /// + /// Which host the repository's pull requests live on. A property of the repository, like + /// beside it: decided from origin's URL here, before any window + /// exists, and again whenever another repository is opened. + /// + public static IPullRequestHost Host { get; set; } = null!; + /// PR to open right after startup (--pr N), for scripted/diagnostic runs. public static int? AutoOpenPr { get; private set; } @@ -34,6 +43,9 @@ public static void Main(string[] args) .FirstOrDefault(); if (repoArg is not null) RepoPath = Path.GetFullPath(repoArg); + // Waited for rather than awaited: there is no dispatcher to deadlock against yet, and + // the first window is built from the answer. + Host = PullRequestHosts.ForAsync(RepoPath).GetAwaiter().GetResult(); BuildAvaloniaApp().StartWithClassicDesktopLifetime(args); } diff --git a/src/Stampeded/ReviewComments.cs b/src/Stampeded/ReviewComments.cs index a1d8b73..42bc7fb 100644 --- a/src/Stampeded/ReviewComments.cs +++ b/src/Stampeded/ReviewComments.cs @@ -1,4 +1,4 @@ -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; using Stampeded.Core.Infra; using Stampeded.Core.Review; @@ -210,13 +210,13 @@ public async Task LoadPostedAsync(int number, CancellationToken ct) // opened online keeps what it read, for the next time it cannot be. var raw = workspace.Offline ? workspace.SnapshotComments ?? [] - : await workspace.GitHub.GetReviewCommentsAsync(number, ct); + : await workspace.Host.GetReviewCommentsAsync(number, ct); if (!workspace.Offline) workspace.KeepComments(raw); Dictionary resolutionByComment = []; try { - foreach (var thread in await workspace.GitHub.GetThreadResolutionsAsync(number, ct)) + foreach (var thread in await workspace.Host.GetThreadResolutionsAsync(number, ct)) { foreach (long id in thread.CommentIds) resolutionByComment[id] = (thread.ThreadId, thread.IsResolved); @@ -347,7 +347,7 @@ public async Task SetThreadResolvedAsync(string threadId, bool resolved) return; try { - await workspace.GitHub.SetThreadResolvedAsync(threadId, resolved); + await workspace.Host.SetThreadResolvedAsync(threadId, resolved); await LoadPostedAsync(pr.Number, CancellationToken.None); } catch (ToolFailedException ex) @@ -356,18 +356,21 @@ public async Task SetThreadResolvedAsync(string threadId, bool resolved) } } - /// Whether the open review is of the user's own pull request. GitHub rejects - /// APPROVE and REQUEST_CHANGES on those, so only a plain comment review can be - /// submitted. False when nothing is open, or when gh cannot say who it is - the - /// submission itself is the real gate, this only keeps the UI from offering what would - /// certainly fail. - public async Task IsOwnPullRequestAsync() + /// Whether a verdict would be refused because the pull request is the reader's + /// own. GitHub rejects APPROVE and REQUEST_CHANGES on those, so only a plain comment + /// review can be submitted; Azure DevOps takes an author's own vote and this is always + /// false there. False as well when nothing is open, or when the host cannot say who the + /// reader is - the submission itself is the real gate, this only keeps the UI from + /// offering what would certainly fail. + public async Task OwnPullRequestBlocksVerdictAsync() { + if (workspace.Host.AcceptsOwnApproval) + return false; if (workspace.CurrentPr?.Author?.Login is not { Length: > 0 } author) return false; try { - return string.Equals(author, await workspace.GitHub.GetViewerLoginAsync(), StringComparison.OrdinalIgnoreCase); + return string.Equals(author, await workspace.Host.GetViewerLoginAsync(), StringComparison.OrdinalIgnoreCase); } catch (ToolFailedException) { @@ -385,11 +388,11 @@ public async Task SubmitCheckedAsync(string eventType, string body) if (workspace.Offline) { return $"Offline: this review was opened from a snapshot taken {workspace.OfflineSince:g}, and a " - + $"verdict has to go to GitHub. Reload (F5) when there is a connection; your " + + $"verdict has to go to {workspace.HostName}. Reload (F5) when there is a connection; your " + $"{Drafts.Count} draft(s) are kept."; } // A line comment names a path and a line of the pull request's own head. Read against a - // branch that has moved past it, the lines on screen are lines GitHub does not have, and + // branch that has moved past it, the lines on screen are lines the host does not have, and // a comment posted from here would land on whatever text now sits at that number - or be // refused for being outside the diff. A reply names a thread instead and is unaffected. if (workspace.LocalHead) @@ -400,7 +403,7 @@ public async Task SubmitCheckedAsync(string eventType, string body) return $"This review is reading the local branch, which is ahead of what #{workspace.CurrentPr?.Number} " + $"shows ({workspace.PrHeadSha?[..9]}). " + (placed > 0 - ? $"{placed} draft(s) sit on lines GitHub does not have; push the branch, then submit. " + ? $"{placed} draft(s) sit on lines {workspace.HostName} does not have; push the branch, then submit. " : "A verdict is given on the pushed head; push the branch, then submit. ") + "Replies to existing threads can be submitted as a comment review from here."; } @@ -414,9 +417,10 @@ public async Task SubmitCheckedAsync(string eventType, string body) return $"Approval blocked by the review guide - incomplete: {gate.Detail} (override in the Guide pane)"; // The buttons are disabled for these on your own pull request, but the check that // disables them is asynchronous, so a submission can still get here first. - if (eventType is "APPROVE" or "REQUEST_CHANGES" && await IsOwnPullRequestAsync()) + if (eventType is "APPROVE" or "REQUEST_CHANGES" && await OwnPullRequestBlocksVerdictAsync()) { - return $"GitHub does not accept {(eventType == "APPROVE" ? "an approval" : "a change request")} " + return $"{workspace.HostName} does not accept " + + $"{(eventType == "APPROVE" ? "an approval" : "a change request")} " + "on your own pull request. Submit it as a comment instead; the drafts are kept."; } // Drafts are matched against the files in scope and the lines of the head on screen, so @@ -496,12 +500,12 @@ public async Task SubmitCheckedAsync(string eventType, string body) // carries the mark the review body would have. bool reviewSubmitted = payload.Count > 0 || body.Trim().Length > 0 || replies.Count == 0; if (reviewSubmitted) - await workspace.GitHub.SubmitReviewAsync(pr.Number, new ReviewSubmission(body, eventType, payload)); + await workspace.Host.SubmitReviewAsync(pr.Number, new ReviewSubmission(body, eventType, payload)); for (int i = 0; i < replies.Count; i++) { var (inReplyTo, replyBody, id) = replies[i]; - await workspace.GitHub.ReplyToCommentAsync(pr.Number, inReplyTo, - !reviewSubmitted && i == 0 ? GitHubService.AttributedReply(replyBody) : replyBody); + await workspace.Host.ReplyToCommentAsync(pr.Number, inReplyTo, + !reviewSubmitted && i == 0 ? ReviewAttribution.AttributedReply(replyBody) : replyBody); submitted.Add(id); } foreach (var id in submitted) diff --git a/src/Stampeded/ReviewWorkspace.cs b/src/Stampeded/ReviewWorkspace.cs index 4f906b8..7870b6a 100644 --- a/src/Stampeded/ReviewWorkspace.cs +++ b/src/Stampeded/ReviewWorkspace.cs @@ -3,7 +3,7 @@ using Stampeded.Core.Decompilation; using Stampeded.Core.Diff; using Stampeded.Core.Git; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; using Stampeded.Core.MergeQueue; using Stampeded.Core.Infra; using Stampeded.Core.Lsp; @@ -29,15 +29,22 @@ public sealed record ReferenceItem(string RelPath, int Line, string Preview, boo /// workspace over the head worktree, and the review-progress store. Orchestrates git/gh /// access, document opening and cross-document navigation. /// -public sealed class ReviewWorkspace(string repoPath) +public sealed class ReviewWorkspace(string repoPath, IPullRequestHost host) { public string RepoPath { get; } = repoPath; public GitService Git { get; } = new(repoPath); - public GitHubService GitHub { get; } = new(repoPath); + + /// Whichever host this repository's pull requests live on, decided once from + /// origin's URL. Nothing above here knows which one answered. + public IPullRequestHost Host { get; } = host; + + /// The host's name, for the headers and tooltips that say whose pull request it is. + public string HostName => Host.Name; + public WorktreeManager Worktrees { get; } = new(repoPath); /// The merge queue this repository's readers share, wherever they are. - public MergeQueueService MergeQueue { get; } = new(new GitService(repoPath), new GitHubService(repoPath)); + public MergeQueueService MergeQueue { get; } = new(new GitService(repoPath), host); /// File content at any revision, without a checkout: what the base side of a /// review is read from. @@ -60,7 +67,7 @@ public sealed class ReviewWorkspace(string repoPath) public PrDetail? CurrentPr { get; private set; } - /// Where "#1234" in any text of this review points; null off GitHub. + /// Where "#1234" in any text of this review points; null off the host. public string? IssueUrlPrefix { get; private set; } /// The refs a local range review was opened with, null for a pull request one. @@ -104,13 +111,13 @@ async Task LoadReviewersAsync(int number, CancellationToken ct) { try { - Reviewers = ReviewVerdicts.Latest(await GitHub.GetReviewsAsync(number, ct)); + Reviewers = ReviewVerdicts.Latest(await Host.GetReviewsAsync(number, ct)); } catch (ToolFailedException ex) { // Who has approved is worth knowing, and not knowing it is worth saying: an empty // list would read as nobody having reviewed. - CliLog.Write("gh", $"reviews unavailable: {ex.Message}"); + CliLog.Write("host", $"reviews unavailable: {ex.Message}"); Reviewers = null; } ReviewersChanged?.Invoke(); @@ -269,7 +276,7 @@ public void SetCoverage(IReadOnlyDictionary /// True while the open review was read from the snapshot of its last online pass instead of - /// from GitHub. Everything git knows is exact - the commits were fetched then and have not + /// from the host. Everything git knows is exact - the commits were fetched then and have not /// moved - and everything GitHub alone knows is as old as . /// public bool Offline { get; private set; } @@ -374,8 +381,8 @@ public async Task OpenLocalRangeAsync(string baseRef, string headRef, int? prNum { try { - detail = await GitHub.GetPrAsync(number, ct); - prHead = await Git.FetchPrHeadAsync(number, ct); + detail = await Host.GetPrAsync(number, ct); + prHead = await Git.FetchPrHeadAsync(await Host.PrHeadRefspecAsync(number, ct), number, ct); await Git.FetchBranchAsync(detail.BaseRefName, ct); // The pull request's own target, not the repository's default branch: a branch // that targets a release branch is not a diff against master. @@ -385,7 +392,7 @@ public async Task OpenLocalRangeAsync(string baseRef, string headRef, int? prNum { // The branch is here either way, and reading it is the point. Losing the // discussion is worth a line; failing the whole open over it is not. - CliLog.Write("gh", $"PR #{number} not attached to this branch review: {ex.Message}"); + CliLog.Write("host", $"PR #{number} not attached to this branch review: {ex.Message}"); detail = null; prHead = null; } @@ -457,8 +464,8 @@ public async Task OpenPrAsync(int number) snapshot = null; try { - detail = await GitHub.GetPrAsync(number, ct); - headSha = await Git.FetchPrHeadAsync(number, ct); + detail = await Host.GetPrAsync(number, ct); + headSha = await Git.FetchPrHeadAsync(await Host.PrHeadRefspecAsync(number, ct), number, ct); await Git.FetchBranchAsync(detail.BaseRefName, ct); baseSha = await Git.GetMergeBaseAsync($"origin/{detail.BaseRefName}", headSha, ct); } @@ -540,7 +547,7 @@ public async Task OpenPrAsync(int number) async Task LoadIssueUrlPrefixAsync(CancellationToken ct) { - IssueUrlPrefix = await GitHub.GetIssueUrlPrefixAsync(ct); + IssueUrlPrefix = await Host.GetIssueUrlPrefixAsync(ct); // The description and the comment threads are rendered before this returns. ReviewChanged?.Invoke(); Comments.Rerender(); @@ -1207,7 +1214,7 @@ public async Task RebasePrAsync(int number) using var busy = Busy.Begin($"Rebasing #{number}"); try { - await GitHub.UpdateBranchAsync(number); + await Host.UpdateBranchAsync(number); StatusMessage?.Invoke($"#{number} rebased onto its target branch."); } catch (ToolFailedException ex) @@ -1225,7 +1232,7 @@ public async Task RebaseCurrentPrOnTargetAsync() using var busy = Busy.Begin($"Rebasing #{pr.Number} onto {pr.BaseRefName}"); try { - await GitHub.UpdateBranchAsync(pr.Number); + await Host.UpdateBranchAsync(pr.Number); StatusMessage?.Invoke($"#{pr.Number} rebased onto {pr.BaseRefName}; reloading the review..."); // The API is asynchronous server-side; give the new head a moment to exist. await Task.Delay(TimeSpan.FromSeconds(3)); @@ -1393,13 +1400,30 @@ public Task OpenUrlAsync(string url) return ExternalTool.RunAsync(tool, args, RepoPath); } - /// Opens a commit on GitHub via gh. - public Task OpenCommitOnGitHubAsync(string sha) - => ExternalTool.RunAsync("gh", ["browse", sha], RepoPath); + /// Opens a commit on the host in the browser. + public Task OpenCommitOnHostAsync(string sha) + => OpenOnHostAsync(Host.CommitUrlAsync(sha)); + + /// Opens a pull request on the host in the browser. + public Task OpenPrOnHostAsync(int number) + => OpenOnHostAsync(Host.PrUrlAsync(number)); - /// Opens a PR in the browser via gh. - public Task OpenOnGitHubAsync(int number) - => ExternalTool.RunAsync("gh", ["pr", "view", number.ToString(), "--web"], RepoPath); + /// Sends one of the host's own addresses to the platform opener, and says so when + /// there is none - a clone whose origin is on neither host has no page to show. + async Task OpenOnHostAsync(Task address) + { + try + { + if (await address is not { Length: > 0 } url) + return $"This repository is not on {HostName}."; + await OpenUrlAsync(url); + return ""; + } + catch (ToolFailedException ex) + { + return $"Could not open {HostName}: {ExternalTool.Explain(ex)}"; + } + } /// Opens the head (or base) worktree in VS Code, optionally at a file:line, /// for full IDE debugging of the reviewed revision. The source clone's .vscode is @@ -2665,7 +2689,7 @@ public async Task GetDefaultBranchAsync() return known; try { - return defaultBranch = await GitHub.GetDefaultBranchAsync(); + return defaultBranch = await Host.GetDefaultBranchAsync(); } catch (ToolFailedException) { @@ -2693,19 +2717,19 @@ public async Task MarkReadyForReviewAsync() if (Offline) { return $"Offline: this review was opened from a snapshot taken {OfflineSince:g}. " - + "Reload (F5) before changing anything on GitHub."; + + "Reload (F5) before changing anything on the host."; } try { using var busy = Busy.Begin($"Marking #{pr.Number} ready"); - await GitHub.MarkReadyForReviewAsync(pr.Number); + await Host.MarkReadyForReviewAsync(pr.Number); // What the review holds has to stop saying draft at the same moment, or everything // that reads it from here - the queue's Add button among them - stays wrong until // the review is reloaded. CurrentPr = pr with { IsDraft = false }; PrStateChanged?.Invoke(); CliLog.Write("action", $"marked #{pr.Number} ready for review"); - return $"#{pr.Number} is ready for review; GitHub has asked for the reviews its rules require."; + return $"#{pr.Number} is ready for review; {HostName} has asked for the reviews its rules require."; } catch (ToolFailedException ex) { @@ -2727,27 +2751,27 @@ public async Task MergeCurrentPrAsync(string method) return $"Offline: this review was opened from a snapshot taken {OfflineSince:g}. " + "Whether it would merge is not something a snapshot can say; reload (F5) first."; } - Core.GitHub.MergeState state; + Core.PullRequests.MergeState state; try { - state = await GitHub.GetMergeStateAsync(pr.Number); + state = await Host.GetMergeStateAsync(pr.Number); } catch (ToolFailedException ex) { return $"Could not read the merge state: {ex.Message}"; } if (!state.CanMerge) - return $"GitHub will not merge #{pr.Number} right now: {state.Summary}."; + return $"{HostName} will not merge #{pr.Number} right now: {state.Summary}."; if (MainWindowOrNull() is not { } owner) return ""; var dialog = new ConfirmWindow("Merge pull request", $"#{pr.Number} {pr.Title}\n\n" + $"{pr.HeadRefName} -> {pr.BaseRefName}, by {method}.\n\n" + (LocalHead - ? $"This merges {PrHeadSha![..9]}, what GitHub has - not the local branch you have " + ? $"This merges {PrHeadSha![..9]}, what {HostName} has - not the local branch you have " + "been reading, which is ahead of it.\n\n" : "") - + "This merges on GitHub, for everyone. It cannot be undone from here.", + + $"This merges on {HostName}, for everyone. It cannot be undone from here.", $"Merge ({method})", $"Delete {pr.HeadRefName} after merging", DeleteBranchPreference.Load(), @@ -2763,7 +2787,7 @@ public async Task MergeCurrentPrAsync(string method) try { using var busy = Busy.Begin($"Merging #{pr.Number}"); - await GitHub.MergePrAsync(pr.Number, method, deleteBranch); + await Host.MergePrAsync(pr.Number, method, deleteBranch); CliLog.Write("action", $"merged #{pr.Number} by {method}{(deleteBranch ? ", deleting " + pr.HeadRefName : "")}"); return $"#{pr.Number} merged into {pr.BaseRefName} by {method}" diff --git a/tests/Stampeded.Core.Tests/AzureDevOpsIdTests.cs b/tests/Stampeded.Core.Tests/AzureDevOpsIdTests.cs new file mode 100644 index 0000000..cd07f9b --- /dev/null +++ b/tests/Stampeded.Core.Tests/AzureDevOpsIdTests.cs @@ -0,0 +1,29 @@ +using NUnit.Framework; + +using Stampeded.Core.AzureDevOps; + +namespace Stampeded.Core.Tests; + +/// +/// Azure DevOps numbers a thread's comments from 1 again in every thread, so neither number +/// alone identifies a comment - and the review carries exactly one number per comment. These +/// pin that the two survive being packed into it and taken back out. +/// +public class AzureDevOpsIdTests +{ + [TestCase(1, 1)] + [TestCase(42, 7)] + [TestCase(999_999, 999_999)] + public void PacksAndSplits(int thread, int comment) + { + var (t, c) = AzureDevOpsService.SplitId(AzureDevOpsService.PackId(thread, comment)); + Assert.Multiple(() => { + Assert.That(t, Is.EqualTo(thread)); + Assert.That(c, Is.EqualTo(comment)); + }); + } + + [Test] + public void DifferentThreadsWithTheSameCommentNumberStayApart() + => Assert.That(AzureDevOpsService.PackId(3, 1), Is.Not.EqualTo(AzureDevOpsService.PackId(4, 1))); +} diff --git a/tests/Stampeded.Core.Tests/AzureDevOpsUrlTests.cs b/tests/Stampeded.Core.Tests/AzureDevOpsUrlTests.cs new file mode 100644 index 0000000..89d2c01 --- /dev/null +++ b/tests/Stampeded.Core.Tests/AzureDevOpsUrlTests.cs @@ -0,0 +1,55 @@ +using NUnit.Framework; + +using Stampeded.Core.AzureDevOps; + +namespace Stampeded.Core.Tests; + +public class AzureDevOpsUrlTests +{ + [TestCase("https://dev.azure.com/contoso/Widgets/_git/widgets-api", "contoso", "Widgets", "widgets-api", null)] + [TestCase("https://dev.azure.com/contoso/Widgets/_git/widgets-api/pullrequest/417", "contoso", "Widgets", "widgets-api", 417)] + // What the portal's clone button writes: the organization again, in front of the host. + [TestCase("https://contoso@dev.azure.com/contoso/Widgets/_git/widgets-api", "contoso", "Widgets", "widgets-api", null)] + [TestCase("https://contoso.visualstudio.com/Widgets/_git/widgets-api", "contoso", "Widgets", "widgets-api", null)] + [TestCase("https://contoso.visualstudio.com/DefaultCollection/Widgets/_git/widgets-api/pullrequest/8", "contoso", "Widgets", "widgets-api", 8)] + [TestCase("git@ssh.dev.azure.com:v3/contoso/Widgets/widgets-api", "contoso", "Widgets", "widgets-api", null)] + [TestCase("https://dev.azure.com/contoso/Widgets/_git/widgets-api.git", "contoso", "Widgets", "widgets-api", null)] + // A project and a repository may both have spaces in them, and a URL carries them encoded. + [TestCase("https://dev.azure.com/contoso/My%20Project/_git/My%20Repo", "contoso", "My Project", "My Repo", null)] + // What a browser hands over beyond the address: the tab that was open, a discussion anchor. + [TestCase("https://dev.azure.com/contoso/Widgets/_git/widgets-api/pullrequest/417?_a=files", "contoso", "Widgets", "widgets-api", 417)] + [TestCase("https://dev.azure.com/contoso/Widgets/_git/widgets-api/pullrequest/417#1234", "contoso", "Widgets", "widgets-api", 417)] + public void Parses(string input, string org, string project, string repo, int? pr) + { + Assert.That(AzureDevOpsUrl.TryParse(input, out string o, out string p, out string r, out int? number), Is.True); + Assert.Multiple(() => { + Assert.That(o, Is.EqualTo(org)); + Assert.That(p, Is.EqualTo(project)); + Assert.That(r, Is.EqualTo(repo)); + Assert.That(number, Is.EqualTo(pr)); + }); + } + + // GitHub's URLs must not be read as Azure DevOps ones: the host decides which service a + // review talks to, and a URL read by both would settle it by the order they are tried. + [TestCase("https://github.com/icsharpcode/ILSpy/pull/3933")] + [TestCase("git@github.com:icsharpcode/ILSpy.git")] + [TestCase("icsharpcode/ILSpy")] + [TestCase("")] + public void RefusesWhatIsNotAzureDevOps(string input) + => Assert.That(AzureDevOpsUrl.TryParse(input, out _, out _, out _, out _), Is.False); + + [Test] + public void MatchesARemoteOfTheCheckout() + { + string config = """ + remote.origin.url https://contoso@dev.azure.com/contoso/Widgets/_git/widgets-api + remote.upstream.url git@github.com:icsharpcode/ILSpy.git + """; + Assert.Multiple(() => { + Assert.That(AzureDevOpsUrl.AnyRemoteMatches(config, "contoso", "Widgets", "widgets-api"), Is.True); + // The same organization and project, another repository: not this checkout. + Assert.That(AzureDevOpsUrl.AnyRemoteMatches(config, "contoso", "Widgets", "widgets-web"), Is.False); + }); + } +} diff --git a/tests/Stampeded.Core.Tests/CheckRunIdTests.cs b/tests/Stampeded.Core.Tests/CheckRunIdTests.cs new file mode 100644 index 0000000..95b0437 --- /dev/null +++ b/tests/Stampeded.Core.Tests/CheckRunIdTests.cs @@ -0,0 +1,25 @@ +using NUnit.Framework; + +using Stampeded.Core.GitHub; + +namespace Stampeded.Core.Tests; + +/// +/// A check row opens its failed log by run id, and the only place that id appears is the link +/// `gh pr checks` hands over - which points at the job, not the run. Getting it wrong makes +/// double-clicking a failed check do nothing at all, silently. +/// +public class CheckRunIdTests +{ + [TestCase("https://github.com/icsharpcode/Stampeded/actions/runs/33841144458/job/100923522142", 33841144458L)] + [TestCase("https://github.com/icsharpcode/Stampeded/actions/runs/33841144458", 33841144458L)] + public void ReadsTheRunOutOfAnActionsLink(string link, long runId) + => Assert.That(GitHubService.RunIdOf(link), Is.EqualTo(runId)); + + // A check reported by anything but Actions links somewhere gh cannot fetch logs from. + [TestCase("https://dev.azure.com/contoso/Widgets/_build/results?buildId=71")] + [TestCase("")] + [TestCase(null)] + public void AnswersNothingForWhatIsNotAnActionsRun(string? link) + => Assert.That(GitHubService.RunIdOf(link), Is.Null); +} diff --git a/tests/Stampeded.Core.Tests/ChecksBucketTests.cs b/tests/Stampeded.Core.Tests/ChecksBucketTests.cs index f88cfeb..f61a0ce 100644 --- a/tests/Stampeded.Core.Tests/ChecksBucketTests.cs +++ b/tests/Stampeded.Core.Tests/ChecksBucketTests.cs @@ -2,7 +2,7 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Tests; diff --git a/tests/Stampeded.Core.Tests/GitWorkingTreeTests.cs b/tests/Stampeded.Core.Tests/GitWorkingTreeTests.cs new file mode 100644 index 0000000..0c6efda --- /dev/null +++ b/tests/Stampeded.Core.Tests/GitWorkingTreeTests.cs @@ -0,0 +1,79 @@ +using NUnit.Framework; + +using Stampeded.Core.Git; +using Stampeded.Core.Infra; + +namespace Stampeded.Core.Tests; + +/// +/// What a checkout contributes to a review beyond its last commit. Untracked files are the +/// case worth pinning: build output, a scratch file and a local config sit next to the work +/// in every real clone, and none of them is part of the change being read. +/// +public class GitWorkingTreeTests +{ + string repo = ""; + readonly List temporaryDirectories = []; + + [SetUp] + public async Task CreateRepository() + { + repo = NewDirectory(); + await Git("init", "--quiet", "--initial-branch=main"); + await Git("config", "user.name", "Test"); + await Git("config", "user.email", "test@example.com"); + await Write("tracked.txt", "one\n"); + await Git("add", "tracked.txt"); + await Git("commit", "--quiet", "-m", "base"); + } + + [TearDown] + public void RemoveTemporaryDirectories() + { + foreach (var dir in temporaryDirectories) + { + TempDirectory.Delete(dir); + } + temporaryDirectories.Clear(); + } + + [Test] + public async Task DiffsTrackedChangesAndNotUntrackedFiles() + { + await Write("tracked.txt", "two\n"); + await Write("untracked.txt", "scratch\n"); + + var files = await new GitService(repo).DiffWorkingTreeAsync(repo, "HEAD"); + + Assert.That(files.Select(f => f.Path), Is.EqualTo(new[] { "tracked.txt" })); + } + + [Test] + public async Task ACheckoutWithOnlyUntrackedFilesIsNotDirty() + { + await Write("untracked.txt", "scratch\n"); + + Assert.That(await new GitService(repo).IsDirtyAsync(repo), Is.False); + } + + [Test] + public async Task ACheckoutWithAModifiedFileIsDirty() + { + await Write("tracked.txt", "two\n"); + + Assert.That(await new GitService(repo).IsDirtyAsync(repo), Is.True); + } + + string NewDirectory() + { + string dir = Path.Combine(Path.GetTempPath(), "stampeded-test-" + Guid.NewGuid().ToString("N")[..8]); + Directory.CreateDirectory(dir); + temporaryDirectories.Add(dir); + return dir; + } + + Task Write(string fileName, string content) + => File.WriteAllTextAsync(Path.Combine(repo, fileName), content); + + Task Git(params string[] args) => ExternalTool.RunAsync("git", args, repo); +} diff --git a/tests/Stampeded.Core.Tests/IssueLinkTests.cs b/tests/Stampeded.Core.Tests/IssueLinkTests.cs index 8c3d778..200716b 100644 --- a/tests/Stampeded.Core.Tests/IssueLinkTests.cs +++ b/tests/Stampeded.Core.Tests/IssueLinkTests.cs @@ -1,6 +1,6 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Tests; diff --git a/tests/Stampeded.Core.Tests/MergeStateExplainTests.cs b/tests/Stampeded.Core.Tests/MergeStateExplainTests.cs index dda9708..03c3160 100644 --- a/tests/Stampeded.Core.Tests/MergeStateExplainTests.cs +++ b/tests/Stampeded.Core.Tests/MergeStateExplainTests.cs @@ -2,7 +2,7 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Tests; diff --git a/tests/Stampeded.Core.Tests/MergeStateSummaryTests.cs b/tests/Stampeded.Core.Tests/MergeStateSummaryTests.cs index e1787dd..f87a20e 100644 --- a/tests/Stampeded.Core.Tests/MergeStateSummaryTests.cs +++ b/tests/Stampeded.Core.Tests/MergeStateSummaryTests.cs @@ -2,7 +2,7 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Tests; diff --git a/tests/Stampeded.Core.Tests/PrApprovalTests.cs b/tests/Stampeded.Core.Tests/PrApprovalTests.cs index 35070b7..8ad1c9a 100644 --- a/tests/Stampeded.Core.Tests/PrApprovalTests.cs +++ b/tests/Stampeded.Core.Tests/PrApprovalTests.cs @@ -1,6 +1,6 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Tests; diff --git a/tests/Stampeded.Core.Tests/PrSummaryForkTests.cs b/tests/Stampeded.Core.Tests/PrSummaryForkTests.cs index c3db6bb..53a032e 100644 --- a/tests/Stampeded.Core.Tests/PrSummaryForkTests.cs +++ b/tests/Stampeded.Core.Tests/PrSummaryForkTests.cs @@ -1,6 +1,6 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Tests; diff --git a/tests/Stampeded.Core.Tests/ReviewAttributionTests.cs b/tests/Stampeded.Core.Tests/ReviewAttributionTests.cs index 7dec20f..e448056 100644 --- a/tests/Stampeded.Core.Tests/ReviewAttributionTests.cs +++ b/tests/Stampeded.Core.Tests/ReviewAttributionTests.cs @@ -1,6 +1,6 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; namespace Stampeded.Core.Tests; @@ -13,7 +13,7 @@ public class ReviewAttributionTests [Test] public void MarksOnlyTheFirstCommentOfAReview() { - var submitted = GitHubService.Attributed( + var submitted = ReviewAttribution.Attributed( new ReviewSubmission("", "COMMENT", [Comment("first"), Comment("second"), Comment("third")])); Assert.That(submitted.Comments[0].Body, Is.EqualTo("first\n\n" + Mark)); @@ -26,7 +26,7 @@ public void MarksOnlyTheFirstCommentOfAReview() [Test] public void MarksTheSummaryWhenAReviewHasNoLineComments() { - var withSummary = GitHubService.Attributed(new ReviewSubmission("Looks good.", "APPROVE", [])); + var withSummary = ReviewAttribution.Attributed(new ReviewSubmission("Looks good.", "APPROVE", [])); Assert.That(withSummary.Body, Is.EqualTo("Looks good.\n\n" + Mark)); } @@ -35,8 +35,8 @@ public void MarksTheSummaryWhenAReviewHasNoLineComments() public void LeavesAVerdictWithNothingWrittenUnmarked() { // The mark would be the whole review: who ran it, and nothing about the change. - Assert.That(GitHubService.Attributed(new ReviewSubmission("", "APPROVE", [])).Body, Is.EqualTo("")); - Assert.That(GitHubService.Attributed(new ReviewSubmission(" \n ", "REQUEST_CHANGES", [])).Body, + Assert.That(ReviewAttribution.Attributed(new ReviewSubmission("", "APPROVE", [])).Body, Is.EqualTo("")); + Assert.That(ReviewAttribution.Attributed(new ReviewSubmission(" \n ", "REQUEST_CHANGES", [])).Body, Is.EqualTo(" \n ")); } } diff --git a/tests/Stampeded.Core.Tests/ReviewVerdictTests.cs b/tests/Stampeded.Core.Tests/ReviewVerdictTests.cs index 915eafc..9b0fcbc 100644 --- a/tests/Stampeded.Core.Tests/ReviewVerdictTests.cs +++ b/tests/Stampeded.Core.Tests/ReviewVerdictTests.cs @@ -1,6 +1,6 @@ using NUnit.Framework; -using Stampeded.Core.GitHub; +using Stampeded.Core.PullRequests; using Stampeded.Core.Review; namespace Stampeded.Core.Tests;