feat(worker): approve in a company repo verifies in the worktree, pushes and opens a draft PR
This commit is contained in:
@@ -81,7 +81,7 @@ public record PlanningMergeConflictStateDto(string PlanningTaskId, string Subtas
|
||||
|
||||
public record PendingQuestionDto(string TaskId, string QuestionId, string Question);
|
||||
|
||||
public record MergeResultDto(string Status, IReadOnlyList<string> ConflictFiles, string? ErrorMessage);
|
||||
public record MergeResultDto(string Status, IReadOnlyList<string> ConflictFiles, string? ErrorMessage, string? PullRequestUrl = null);
|
||||
|
||||
public record BaseDirtyWarningDto(int ModifiedCount, int UntrackedCount);
|
||||
|
||||
@@ -93,7 +93,7 @@ public record MergePreviewDto(
|
||||
|
||||
public record MergeTargetsDto(
|
||||
string DefaultBranch, IReadOnlyList<string> LocalBranches, string DefaultCommitMessage,
|
||||
bool TargetDirty = false);
|
||||
bool TargetDirty = false, bool IsCompanyRepo = false);
|
||||
|
||||
public record MergeConflictDocumentsDto(string TaskId, IReadOnlyList<ConflictDocumentDto> Files);
|
||||
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
using System.Text;
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Git;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.Config;
|
||||
using ClaudeDo.Worker.Git;
|
||||
using ClaudeDo.Worker.Hub;
|
||||
using ClaudeDo.Worker.Lifecycle;
|
||||
using ClaudeDo.Worker.Runner;
|
||||
using ClaudeDo.Worker.Tickets;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
|
||||
namespace ClaudeDo.Worker.AzureDevOps;
|
||||
|
||||
/// <summary>
|
||||
/// The company-repo end of a review: push the unit branch and find-or-create its draft PR on Azure DevOps.
|
||||
/// Never touches task status or worktree state — the task stays WaitingForReview until the ADO merge.
|
||||
/// </summary>
|
||||
public sealed class PullRequestPublisher
|
||||
{
|
||||
/// <summary>Web front of the Bandel ticket system; the API base URL (an IP) is not browsable.</summary>
|
||||
public const string TicketWebBase = "https://ticket.fb-tuning.local";
|
||||
|
||||
private readonly GitService _git;
|
||||
private readonly AzureDevOpsClient _azdo;
|
||||
private readonly IDbContextFactory<ClaudeDoDbContext> _dbFactory;
|
||||
private readonly HubBroadcaster _broadcaster;
|
||||
private readonly WorkerConfig _cfg;
|
||||
private readonly ILogger<PullRequestPublisher> _logger;
|
||||
private readonly TicketClientFactory? _tickets;
|
||||
|
||||
public PullRequestPublisher(
|
||||
GitService git, AzureDevOpsClient azdo, IDbContextFactory<ClaudeDoDbContext> dbFactory,
|
||||
HubBroadcaster broadcaster, WorkerConfig cfg, ILogger<PullRequestPublisher> logger,
|
||||
TicketClientFactory? tickets = null)
|
||||
{
|
||||
_git = git;
|
||||
_azdo = azdo;
|
||||
_dbFactory = dbFactory;
|
||||
_broadcaster = broadcaster;
|
||||
_cfg = cfg;
|
||||
_logger = logger;
|
||||
_tickets = tickets;
|
||||
}
|
||||
|
||||
/// <summary>Live check of origin against <c>company_remote_pattern</c>.</summary>
|
||||
public Task<bool> IsCompanyRepoAsync(string workingDir, CancellationToken ct) =>
|
||||
CompanyRepo.IsCompanyAsync(_git, workingDir, _cfg.CompanyRemotePattern, ct);
|
||||
|
||||
/// <summary>Why a PR can't be published from <paramref name="workingDir"/>, or null — checked before the verify run so a missing PAT doesn't cost a build.</summary>
|
||||
public async Task<string?> CheckReadyAsync(string workingDir, CancellationToken ct)
|
||||
{
|
||||
if (string.IsNullOrEmpty(_azdo.Pat))
|
||||
return "Kein Azure-DevOps-PAT hinterlegt (Einstellungen → Externe Dienste) — nichts gepusht.";
|
||||
var remote = await _git.GetRemoteUrlAsync(workingDir, "origin", ct);
|
||||
return AzureDevOpsClient.ParseRemote(remote) is null
|
||||
? $"origin ist kein https-Remote eines Azure DevOps Servers: {remote}"
|
||||
: null;
|
||||
}
|
||||
|
||||
/// <summary>Push + draft PR; <paramref name="verifyCommand"/> is the command that passed (null = none ran).</summary>
|
||||
public async Task<MergeResult> PublishAsync(
|
||||
TaskEntity task, ListEntity list, WorktreeEntity wt, string? verifyCommand, CancellationToken ct)
|
||||
{
|
||||
var workingDir = list.WorkingDir!;
|
||||
if (await CheckReadyAsync(workingDir, ct) is { } notReady) return Blocked(notReady);
|
||||
var pat = _azdo.Pat!;
|
||||
var repo = AzureDevOpsClient.ParseRemote(await _git.GetRemoteUrlAsync(workingDir, "origin", ct))!;
|
||||
|
||||
var (pushExit, pushErr) = await _git.PushBranchAsync(wt.Path, wt.BranchName, AzureDevOpsClient.GitAuthEnv(pat), ct);
|
||||
if (pushExit != 0) return Blocked($"push rejected: {pushErr}");
|
||||
|
||||
string url;
|
||||
bool created;
|
||||
try
|
||||
{
|
||||
var target = await ResolveTargetAsync(task.ListId, repo, ct);
|
||||
if (target == wt.BranchName)
|
||||
return Blocked($"gepusht, PR fehlgeschlagen: Quell- und Ziel-Branch sind beide '{target}'.");
|
||||
var ticketId = TicketStatusMap.ParseBandelTicketId(task.TicketRef);
|
||||
var title = BuildTitle(task, list, ticketId);
|
||||
var description = BuildDescription(task, ticketId, await ResolveProjectIdAsync(ticketId, ct), verifyCommand);
|
||||
(url, created) = await _azdo.EnsureDraftPullRequestAsync(repo, wt.BranchName, target, title, description, ct);
|
||||
}
|
||||
catch (AzureDevOpsException ex)
|
||||
{
|
||||
return Blocked($"gepusht, PR fehlgeschlagen: {ex.Message}");
|
||||
}
|
||||
|
||||
using (var ctx = _dbFactory.CreateDbContext())
|
||||
await new WorktreeRepository(ctx).SetPullRequestUrlAsync(task.Id, url, ct);
|
||||
await _broadcaster.WorktreeUpdated(task.Id);
|
||||
_logger.LogInformation("Task {task_id}: branch {branch} pushed, draft PR {pr_url} ({pr_state})",
|
||||
task.Id, wt.BranchName, url, created ? "created" : "reused");
|
||||
|
||||
return new MergeResult(created ? TaskMergeService.StatusPrCreated : TaskMergeService.StatusPrUpdated,
|
||||
Array.Empty<string>(), null, url);
|
||||
}
|
||||
|
||||
/// <summary><c><type>(<list-slug>): <title>, Resolves #<ticket></c>.</summary>
|
||||
public static string BuildTitle(TaskEntity task, ListEntity list, int? ticketId)
|
||||
{
|
||||
var type = string.IsNullOrWhiteSpace(task.CommitType) ? CommitTypeRegistry.DefaultType : task.CommitType.Trim();
|
||||
var slug = CommitMessageBuilder.ToSlug(list.Name);
|
||||
var scope = slug.Length == 0 ? "" : $"({slug})";
|
||||
var title = $"{type}{scope}: {task.Title.Trim()}";
|
||||
return ticketId is null ? title : $"{title}, Resolves #{ticketId}";
|
||||
}
|
||||
|
||||
/// <summary>Ticket link (plain <c>Resolves #id</c> without a project id), task description, and only what really ran.</summary>
|
||||
public static string BuildDescription(TaskEntity task, int? ticketId, int? projectId, string? verifyCommand)
|
||||
{
|
||||
var sb = new StringBuilder();
|
||||
if (ticketId is { } id)
|
||||
{
|
||||
sb.Append(projectId is { } pid
|
||||
? $"Resolves [#{id}]({TicketWebBase}/board/project/{pid}/ticket/{id})"
|
||||
: $"Resolves #{id}");
|
||||
sb.Append("\n\n");
|
||||
}
|
||||
if (!string.IsNullOrWhiteSpace(task.Description))
|
||||
sb.Append(task.Description.Trim()).Append("\n\n");
|
||||
sb.Append(verifyCommand is null ? "Geprüft: nichts automatisch" : $"Geprüft: `{verifyCommand}` grün");
|
||||
return sb.ToString();
|
||||
}
|
||||
|
||||
private async Task<string> ResolveTargetAsync(string listId, AzdoRepo repo, CancellationToken ct)
|
||||
{
|
||||
using var ctx = _dbFactory.CreateDbContext();
|
||||
var baseBranch = await ctx.ListConfigs.AsNoTracking()
|
||||
.Where(c => c.ListId == listId)
|
||||
.Select(c => c.BaseBranch)
|
||||
.FirstOrDefaultAsync(ct);
|
||||
return baseBranch.NullIfBlank() ?? await _azdo.GetDefaultBranchAsync(repo, ct);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The project an item lives in: a ticket's parent is its project, a task's parent is a ticket — so walk
|
||||
/// up until <c>parentTicketID</c> is null (live-checked 2026-10-01). Null when unknown or unreachable.
|
||||
/// </summary>
|
||||
private async Task<int?> ResolveProjectIdAsync(int? ticketId, CancellationToken ct)
|
||||
{
|
||||
if (ticketId is null || _tickets?.Create() is not { } client) return null;
|
||||
try
|
||||
{
|
||||
var id = ticketId.Value;
|
||||
for (var hop = 0; hop < 3; hop++)
|
||||
{
|
||||
var detail = await client.GetTicketAsync(id, ct);
|
||||
if (detail.ParentTicketId is not { } parent) return hop == 0 ? null : id;
|
||||
id = parent;
|
||||
}
|
||||
}
|
||||
catch (TicketApiException ex)
|
||||
{
|
||||
_logger.LogWarning("Project of ticket {ticket_id} unknown, PR links no ticket: {reason}", ticketId, ex.Message);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private static MergeResult Blocked(string reason) =>
|
||||
new(TaskMergeService.StatusBlocked, Array.Empty<string>(), reason);
|
||||
}
|
||||
+8
-1
@@ -636,7 +636,10 @@ public sealed class ExternalMcpService
|
||||
"means the merge stopped on conflicts, with the files listed. emptyChildren (parent approve only) lists " +
|
||||
"the Done children about to be unit-merged whose own review range contributed nothing (e.g. a child that " +
|
||||
"reported CLAUDEDO_BLOCKED and committed no code) — check it before trusting that every child actually " +
|
||||
"delivered something." + McpToolDocs.LeanTaskRef + McpToolDocs.TaskNumberHint + McpToolDocs.ProgressHint)]
|
||||
"delivered something. In a company repo (origin on the Azure DevOps Server) approve does not merge: it runs " +
|
||||
"the verify command in the task's worktree (skipVerify is ignored), pushes the branch and opens or reuses a " +
|
||||
"draft PR — mergeStatus 'pr_created'/'pr_updated', mergeMessage = the PR URL, the task stays WaitingForReview " +
|
||||
"and needs no verify_merges." + McpToolDocs.LeanTaskRef + McpToolDocs.TaskNumberHint + McpToolDocs.ProgressHint)]
|
||||
public async Task<ReviewTaskResult> ReviewTask(
|
||||
string taskId,
|
||||
[Description("'approve', 'reject_rerun', 'reject_park' or 'cancel'.")]
|
||||
@@ -736,6 +739,10 @@ public sealed class ExternalMcpService
|
||||
mergeMessage = "merged; verify deferred — the task stays WaitingForReview until " +
|
||||
"verify_merges (with its id) passes";
|
||||
}
|
||||
else if (r.Status is TaskMergeService.StatusPrCreated or TaskMergeService.StatusPrUpdated)
|
||||
{
|
||||
mergeMessage = r.PullRequestUrl;
|
||||
}
|
||||
else if (r.Status == TaskMergeService.StatusConflict)
|
||||
{
|
||||
if (leaveConflictsInTree)
|
||||
|
||||
@@ -502,7 +502,7 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub
|
||||
=> HubGuard(async () =>
|
||||
{
|
||||
var t = await _mergeService.GetTargetsAsync(taskId, CancellationToken.None);
|
||||
return new MergeTargetsDto(t.DefaultBranch, t.LocalBranches, t.DefaultCommitMessage, t.TargetDirty);
|
||||
return new MergeTargetsDto(t.DefaultBranch, t.LocalBranches, t.DefaultCommitMessage, t.TargetDirty, t.IsCompanyRepo);
|
||||
});
|
||||
|
||||
public Task<MergePreviewDto> PreviewMerge(string taskId, string targetBranch)
|
||||
@@ -714,7 +714,7 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub
|
||||
var r = await _mergeService.ApproveAndMergeAsync(taskId, targetBranch ?? "", CancellationToken.None, autostash);
|
||||
if (r.Status == TaskMergeService.StatusBlocked)
|
||||
throw new HubException(r.ErrorMessage ?? "approve failed");
|
||||
return new MergeResultDto(r.Status, r.ConflictFiles, r.ErrorMessage);
|
||||
return new MergeResultDto(r.Status, r.ConflictFiles, r.ErrorMessage, r.PullRequestUrl);
|
||||
});
|
||||
|
||||
public async Task RejectReviewToQueue(string taskId, string feedback)
|
||||
|
||||
@@ -3,6 +3,7 @@ using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Git;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.AzureDevOps;
|
||||
using ClaudeDo.Worker.Hub;
|
||||
using ClaudeDo.Worker.Runner;
|
||||
using ClaudeDo.Worker.State;
|
||||
@@ -15,13 +16,15 @@ namespace ClaudeDo.Worker.Lifecycle;
|
||||
public sealed record MergeResult(
|
||||
string Status,
|
||||
IReadOnlyList<string> ConflictFiles,
|
||||
string? ErrorMessage);
|
||||
string? ErrorMessage,
|
||||
string? PullRequestUrl = null);
|
||||
|
||||
public sealed record MergeTargets(
|
||||
string DefaultBranch,
|
||||
IReadOnlyList<string> LocalBranches,
|
||||
string DefaultCommitMessage,
|
||||
bool TargetDirty = false);
|
||||
bool TargetDirty = false,
|
||||
bool IsCompanyRepo = false);
|
||||
|
||||
// VerifyExitCode/VerifyDurationMs/VerifyOutputTail are only populated when the caller asked for
|
||||
// a verify run (runVerify=true) AND the list has a verify command configured AND the preview came
|
||||
@@ -76,6 +79,11 @@ public sealed class TaskMergeService
|
||||
// autostash: true) without matching on prose.
|
||||
public const string StatusDirtyTarget = "dirty_target";
|
||||
|
||||
// Company repo: approve pushed the unit branch and opened (or reused) its draft PR; the task stays
|
||||
// WaitingForReview, nothing landed locally.
|
||||
public const string StatusPrCreated = "pr_created";
|
||||
public const string StatusPrUpdated = "pr_updated";
|
||||
|
||||
public const string StatusReverted = "reverted";
|
||||
public const string StatusConflictAborted = "conflict_aborted";
|
||||
|
||||
@@ -118,6 +126,7 @@ public sealed class TaskMergeService
|
||||
private readonly IVerifyCommandRunner _verify;
|
||||
private readonly ILogger<TaskMergeService> _logger;
|
||||
private readonly UnitBranchService? _unitBranch;
|
||||
private readonly PullRequestPublisher? _pullRequests;
|
||||
|
||||
public TaskMergeService(
|
||||
IDbContextFactory<ClaudeDoDbContext> dbFactory,
|
||||
@@ -126,7 +135,8 @@ public sealed class TaskMergeService
|
||||
ITaskStateService state,
|
||||
IVerifyCommandRunner verify,
|
||||
ILogger<TaskMergeService> logger,
|
||||
UnitBranchService? unitBranch = null)
|
||||
UnitBranchService? unitBranch = null,
|
||||
PullRequestPublisher? pullRequests = null)
|
||||
{
|
||||
_dbFactory = dbFactory;
|
||||
_git = git;
|
||||
@@ -135,6 +145,7 @@ public sealed class TaskMergeService
|
||||
_verify = verify;
|
||||
_logger = logger;
|
||||
_unitBranch = unitBranch;
|
||||
_pullRequests = pullRequests;
|
||||
}
|
||||
|
||||
private async Task<(TaskEntity Task, ListEntity List, WorktreeEntity? Worktree, string? VerifyCommand)> LoadMergeContextAsync(
|
||||
@@ -881,7 +892,8 @@ public sealed class TaskMergeService
|
||||
var current = await ResolveTargetAsync(list, null, ct);
|
||||
var branches = await _git.ListLocalBranchesAsync(list.WorkingDir, ct);
|
||||
var dirty = await _git.HasChangesAsync(list.WorkingDir, includeUntracked: false, ct);
|
||||
return new MergeTargets(current, branches, defaultMessage, dirty);
|
||||
var company = _pullRequests is not null && await _pullRequests.IsCompanyRepoAsync(list.WorkingDir, ct);
|
||||
return new MergeTargets(current, branches, defaultMessage, dirty, company);
|
||||
}
|
||||
|
||||
/// <summary>Leeres Ziel → <c>list_config.base_branch</c>, sonst der ausgecheckte Branch des Main-Checkouts.</summary>
|
||||
@@ -1042,6 +1054,11 @@ public sealed class TaskMergeService
|
||||
}
|
||||
}
|
||||
|
||||
if (wt is { State: WorktreeState.Active } && _pullRequests is not null
|
||||
&& !string.IsNullOrWhiteSpace(list.WorkingDir)
|
||||
&& await _pullRequests.IsCompanyRepoAsync(list.WorkingDir, ct))
|
||||
return await ApproveCompanyAsync(task, list, wt, verifyCommand, ct, progress);
|
||||
|
||||
if (wt is not null && wt.State == WorktreeState.Kept)
|
||||
{
|
||||
// Kept means either RevertMergeAsync undid a previous merge, or the user manually
|
||||
@@ -1125,6 +1142,33 @@ public sealed class TaskMergeService
|
||||
return await MergeAsync(taskId, target, removeWorktree: true, commitMessage: "", leaveConflictsInTree, ct, progress, skipVerify, autostash);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Company repo: verify in the unit root's worktree (nothing lands on a local target, so there is no
|
||||
/// merge to defer the verify to — <c>skipVerify</c> does not apply), then push + draft PR. Red verify
|
||||
/// pushes nothing. Status, worktree state and ticket stay as they are.
|
||||
/// </summary>
|
||||
private async Task<MergeResult> ApproveCompanyAsync(
|
||||
TaskEntity task, ListEntity list, WorktreeEntity wt, string? verifyCommand, CancellationToken ct,
|
||||
IProgress<ProgressNotificationValue>? progress)
|
||||
{
|
||||
if (await _pullRequests!.CheckReadyAsync(list.WorkingDir!, ct) is { } notReady)
|
||||
return Blocked(notReady);
|
||||
|
||||
var ranVerify = !string.IsNullOrWhiteSpace(verifyCommand);
|
||||
if (ranVerify)
|
||||
{
|
||||
await _broadcaster.OperationProgress(task.Id, PhaseVerifying, 0, 0);
|
||||
var failed = await RunVerifyGateAsync(
|
||||
verifyCommand, wt.Path, ct, progress,
|
||||
elapsed => _ = _broadcaster.OperationProgress(task.Id, PhaseVerifying, (int)elapsed.TotalSeconds, 0));
|
||||
if (failed is not null) return failed;
|
||||
}
|
||||
|
||||
return await ProgressReporter.RunAsync(
|
||||
_pullRequests.PublishAsync(task, list, wt, ranVerify ? verifyCommand : null, ct),
|
||||
ProgressReportInterval, progress, "push + draft PR running");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Redoes a merge that <see cref="RevertMergeAsync"/> undid (or that predates it, if the branch
|
||||
/// was already merged before its worktree reached <see cref="WorktreeState.Kept"/>), by
|
||||
|
||||
@@ -257,6 +257,7 @@ builder.Services.AddSingleton<TicketStatusSync>();
|
||||
builder.Services.AddSingleton<TicketImportService>();
|
||||
builder.Services.AddHttpClient("azdo", c => c.Timeout = TimeSpan.FromSeconds(30));
|
||||
builder.Services.AddSingleton<AzureDevOpsClient>();
|
||||
builder.Services.AddSingleton<PullRequestPublisher>();
|
||||
|
||||
if (cfg.OnlineInbox.Enabled)
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user