diff --git a/src/AgentOs.Domain/Sessions/IIssueWorkAgent.cs b/src/AgentOs.Domain/Sessions/IIssueWorkAgent.cs index 8c6c0e2..ed4e77a 100644 --- a/src/AgentOs.Domain/Sessions/IIssueWorkAgent.cs +++ b/src/AgentOs.Domain/Sessions/IIssueWorkAgent.cs @@ -16,7 +16,17 @@ namespace AgentOs.Domain.Sessions; /// Base branch the fix branch is cut from. /// Set for "Ask for changes": the branch of this repo's open pull request. The agent /// checks it out and pushes more commits to it instead of starting a new branch. -public sealed record WorkRepo(Guid SessionRepoId, string Owner, string Repo, string DefaultBranch, string? ExistingBranch = null); +public sealed record WorkRepo(Guid SessionRepoId, string Owner, string Repo, string DefaultBranch, string? ExistingBranch = null) +{ + /// The branch a fresh attempt creates, chosen by the server so it never collides with one an earlier, + /// interrupted attempt left on the remote (that push would need a force-push, which the agent may not do). + /// Null = the default issue-N-ai-fix. + public string? NewBranch { get; init; } + + /// The branch a ticket's first attempt uses. + public static string DefaultBranchName(int issueNumber) => + "issue-" + issueNumber.ToString(System.Globalization.CultureInfo.InvariantCulture) + "-ai-fix"; +} /// Request to the issue-work agent — a ticket plus the repos it should be implemented in. /// Correlating session row for evidence / logging. diff --git a/src/AgentOs.Domain/Workspaces/IPrCreationService.cs b/src/AgentOs.Domain/Workspaces/IPrCreationService.cs index f0911b5..7e79d83 100644 --- a/src/AgentOs.Domain/Workspaces/IPrCreationService.cs +++ b/src/AgentOs.Domain/Workspaces/IPrCreationService.cs @@ -89,6 +89,10 @@ Task GetMergeStateAsync(WorkspaceDescriptor workspace, int number, Task UpdateBranchAsync(WorkspaceDescriptor workspace, int number, CancellationToken ct = default) => Task.FromResult("Updating a branch is not supported for this provider."); + /// True when exists on the remote. Null when it couldn't be checked. + Task BranchExistsAsync(WorkspaceDescriptor workspace, string branch, CancellationToken ct = default) => + Task.FromResult(null); + /// Post a comment on pull request (the Testing agent's evidence). Never throws. Task CommentAsync(WorkspaceDescriptor workspace, int number, string body, CancellationToken ct = default) => Task.FromResult(false); diff --git a/src/AgentOs.Modules.Integration/GitHubPrCreationService.cs b/src/AgentOs.Modules.Integration/GitHubPrCreationService.cs index 5d767be..c72fc09 100644 --- a/src/AgentOs.Modules.Integration/GitHubPrCreationService.cs +++ b/src/AgentOs.Modules.Integration/GitHubPrCreationService.cs @@ -163,6 +163,28 @@ internal static PrMergeState MergeStateOf(bool? mergeable, MergeableState? state } } + /// + public async Task BranchExistsAsync(WorkspaceDescriptor workspace, string branch, CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(workspace); + if (workspace.Kind != SourceProviderKind.GitHub || string.IsNullOrWhiteSpace(branch)) { return null; } + try + { + var client = GitHubClientFactory.Create(workspace.AccessToken, workspace.Host); + await client.Repository.Branch.Get(workspace.Owner, workspace.Repo, branch).ConfigureAwait(false); + return true; + } + catch (NotFoundException) + { + return false; + } + catch (Exception ex) when (ex is ApiException or System.Net.Http.HttpRequestException or TaskCanceledException) + { + _logger.LogWarning(ex, "Couldn't check branch {Branch} on {Owner}/{Repo}", branch, workspace.Owner, workspace.Repo); + return null; + } + } + /// public async Task CommentAsync(WorkspaceDescriptor workspace, int number, string body, CancellationToken ct = default) { diff --git a/src/AgentOs.Modules.Pipeline/Prompts/IssueWorkPrompt.cs b/src/AgentOs.Modules.Pipeline/Prompts/IssueWorkPrompt.cs index 597bfc2..8581cd9 100644 --- a/src/AgentOs.Modules.Pipeline/Prompts/IssueWorkPrompt.cs +++ b/src/AgentOs.Modules.Pipeline/Prompts/IssueWorkPrompt.cs @@ -160,7 +160,11 @@ private static bool IsRevision(IssueWorkRequest req, WorkRepo repo) => !string.IsNullOrWhiteSpace(req.ChangeRequest) && !string.IsNullOrWhiteSpace(repo.ExistingBranch); internal static string BranchName(IssueWorkRequest req, WorkRepo repo) => - IsRevision(req, repo) ? repo.ExistingBranch! : $"issue-{req.IssueNumber.ToString(CultureInfo.InvariantCulture)}-ai-fix"; + IsRevision(req, repo) ? repo.ExistingBranch! : FreshBranch(req, repo); + + // A fresh attempt's branch: the server's pick when an older attempt's branch is still on the remote. + private static string FreshBranch(IssueWorkRequest req, WorkRepo repo) => + string.IsNullOrWhiteSpace(repo.NewBranch) ? WorkRepo.DefaultBranchName(req.IssueNumber) : repo.NewBranch; // Step 3 of the workflow. The prompt's steps start at column 0 with continuation lines indented by 3, so the // inserted text follows the same shape. @@ -173,7 +177,7 @@ private static string BranchStep(IssueWorkRequest req, WorkRepo repo) => IsRevis " Read the requested changes in the user message and address every point.\n") : string.Concat( "3. Create a feature branch off the default branch (", repo.DefaultBranch, "):\n", - " git checkout -b issue-", req.IssueNumber.ToString(CultureInfo.InvariantCulture), "-ai-fix\n"); + " git checkout -b ", FreshBranch(req, repo), "\n"); // The commit type follows the ticket ("feat" for a feature, "fix" for a bug…), like the pull request title. private static string CommitType(IssueWorkRequest req) => AgentOs.Domain.Workspaces.PullRequestText.ConventionalType(req.TicketType); @@ -190,9 +194,9 @@ private static string Instruction(IssueWorkRequest req, WorkRepo repo) => IsRevi """, repo.ExistingBranch, UntrustedText.Wrap("requested-changes", req.ChangeRequest!, maxChars: 16_000)) : string.Format(CultureInfo.InvariantCulture, - "Implement a fix for this ticket in THIS repository. Create branch `issue-{0}-ai-fix`,\n" + "Implement a fix for this ticket in THIS repository. Create branch `{0}`,\n" + "make the minimal necessary changes, ensure the build passes, commit, and push the branch.", - req.IssueNumber); + FreshBranch(req, repo)); private static string CrossServiceNote(IssueWorkRequest req, WorkRepo repo) { diff --git a/src/AgentOs.Web/Services/BoardTasks/BoardTaskService.cs b/src/AgentOs.Web/Services/BoardTasks/BoardTaskService.cs index 5e36ab9..b87fa38 100644 --- a/src/AgentOs.Web/Services/BoardTasks/BoardTaskService.cs +++ b/src/AgentOs.Web/Services/BoardTasks/BoardTaskService.cs @@ -273,6 +273,24 @@ private static string ShortTestName(string test) return shortName.Length > 90 ? shortName[..90] + "…" : shortName; } + // A fresh attempt must not reuse a branch an earlier, interrupted attempt left on the remote: its push would be + // rejected as non-fast-forward and the agent may not force-push (#15 on the lab finished its work and could not push + // it). The first free of issue-N-ai-fix, -2 … -9; null (the default name) when it can't be checked. + internal static async Task FreeBranchAsync(IPrCreationService prs, WorkspaceDescriptor repo, int issueNumber) + { + ArgumentNullException.ThrowIfNull(prs); + if (issueNumber <= 0) { return null; } + var name = WorkRepo.DefaultBranchName(issueNumber); + for (var attempt = 1; attempt <= 9; attempt++) + { + var candidate = attempt == 1 ? name : $"{name}-{attempt.ToString(System.Globalization.CultureInfo.InvariantCulture)}"; + var exists = await prs.BranchExistsAsync(repo, candidate).ConfigureAwait(false); + if (exists is null) { return null; } + if (exists == false) { return attempt == 1 ? null : candidate; } + } + return $"{name}-{DateTimeOffset.UtcNow:yyyyMMddHHmm}"; + } + /// The number of a pull request from its URL (…/pull/42). public static int? PrNumber(string? url) { @@ -308,10 +326,17 @@ void Publish(SessionRunEventKind kind, string message, string? repo = null, stri var issueBody = await ReadTicketBodyAsync(scope, board, tenantId, session, pat).ConfigureAwait(false); // Ask for changes: each repo that already has a pull request continues on its branch. - var workRepos = children - .Select(c => new WorkRepo(c.Id, c.Owner, c.Repo, c.DefaultBranch, - change is not null && !string.IsNullOrEmpty(c.PrUrl) ? c.BranchName : null)) - .ToList(); + var workRepos = new List(children.Count); + foreach (var c in children) + { + var continuing = change is not null && !string.IsNullOrEmpty(c.PrUrl); + workRepos.Add(new WorkRepo(c.Id, c.Owner, c.Repo, c.DefaultBranch, continuing ? c.BranchName : null) + { + NewBranch = continuing ? null + : await FreeBranchAsync(prService, RepoDescriptor(board, tenantId, c.Owner, c.Repo, c.DefaultBranch, pat), + session.IssueNumber ?? 0).ConfigureAwait(false), + }); + } var issueRequest = new IssueWorkRequest( SessionId: session.Id, TenantId: tenantId, diff --git a/tests/AgentOs.Tests/Pipeline/IssueWorkRevisionPromptTests.cs b/tests/AgentOs.Tests/Pipeline/IssueWorkRevisionPromptTests.cs index dcc5235..c47f9e1 100644 --- a/tests/AgentOs.Tests/Pipeline/IssueWorkRevisionPromptTests.cs +++ b/tests/AgentOs.Tests/Pipeline/IssueWorkRevisionPromptTests.cs @@ -51,6 +51,20 @@ public void FollowUp_RequestedChanges_AreFencedAsUntrusted() user.ShouldContain("on branch `issue-4-ai-fix`"); } + // An interrupted earlier attempt left issue-4-ai-fix on the remote: the server picks a free name and the whole prompt + // uses it (pushing to the old one would need a force-push the agent may not do). + [Fact] + public void FreshAttempt_ServerPickedBranch_UsedEverywhere() + { + var repo = NewRepo with { NewBranch = "issue-4-ai-fix-2" }; + foreach (var system in new[] { IssueWorkPrompt.System(Req(repo), repo), IssueWorkPrompt.SystemCli(Req(repo), repo) }) + { + system.ShouldContain("git checkout -b issue-4-ai-fix-2"); + system.ShouldContain("{\"branch\":\"issue-4-ai-fix-2\""); + } + IssueWorkPrompt.User(Req(repo), repo).ShouldContain("Create branch `issue-4-ai-fix-2`"); + } + [Fact] public void FollowUp_WithoutABranch_IsAFirstRun() => IssueWorkPrompt.SystemCli(Req(NewRepo, "change it"), NewRepo).ShouldContain("git checkout -b issue-4-ai-fix"); diff --git a/tests/AgentOs.Tests/Web/FreeBranchTests.cs b/tests/AgentOs.Tests/Web/FreeBranchTests.cs new file mode 100644 index 0000000..8c4a96d --- /dev/null +++ b/tests/AgentOs.Tests/Web/FreeBranchTests.cs @@ -0,0 +1,46 @@ +// A fresh attempt picks a branch an earlier, interrupted attempt didn't leave on the remote (#15 on the lab finished its +// work and could not push: issue-15-ai-fix already existed, and the agent may not force-push). + +using System; +using System.Threading; +using System.Threading.Tasks; +using AgentOs.Domain.Workspaces; +using AgentOs.Web.Services.BoardTasks; +using NSubstitute; +using Shouldly; +using Xunit; + +namespace AgentOs.Tests.Web; + +public sealed class FreeBranchTests +{ + private static readonly WorkspaceDescriptor Repo = new(Guid.NewGuid(), "t", SourceProviderKind.GitHub, "o", "r", null, "main", "pat"); + + private static IPrCreationService Remote(params string[] existing) + { + var prs = Substitute.For(); + prs.BranchExistsAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ci => Task.FromResult(Array.IndexOf(existing, ci.ArgAt(1)) >= 0)); + return prs; + } + + [Fact] + public async Task FreeBranch_NothingOnTheRemote_KeepsTheDefault() => + (await BoardTaskService.FreeBranchAsync(Remote(), Repo, 15)).ShouldBeNull(); + + [Fact] + public async Task FreeBranch_EarlierAttemptsLeftBranches_PicksTheNextFreeOne() => + (await BoardTaskService.FreeBranchAsync(Remote("issue-15-ai-fix", "issue-15-ai-fix-2"), Repo, 15)).ShouldBe("issue-15-ai-fix-3"); + + [Fact] + public async Task FreeBranch_CannotCheck_KeepsTheDefault() + { + var prs = Substitute.For(); + prs.BranchExistsAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(Task.FromResult(null)); + (await BoardTaskService.FreeBranchAsync(prs, Repo, 15)).ShouldBeNull(); + } + + [Fact] + public async Task FreeBranch_NoTicketNumber_KeepsTheDefault() => + (await BoardTaskService.FreeBranchAsync(Remote("issue-0-ai-fix"), Repo, 0)).ShouldBeNull(); +}