diff --git a/docs/explore-notes/review-merge.md b/docs/explore-notes/review-merge.md index 482c002a..660e6658 100644 --- a/docs/explore-notes/review-merge.md +++ b/docs/explore-notes/review-merge.md @@ -29,7 +29,7 @@ There is no separate "Merge all" entry. the unit branch (`UnitBranchService.AbsorbChildAsync`, ff-only) instead of merging to the target. - **No active worktree** (sandbox run) → straight to `Done`. - **Company repo** (`PullRequestPublisher.IsCompanyRepoAsync`, origin matches - `company_remote_pattern`, checked live) with an `Active` worktree → `ApproveCompanyAsync` + `company_remote_pattern`, checked live) with an `Active` or `Kept` worktree → `ApproveCompanyAsync` instead of a merge, after the planning-parent gate and the child absorb: missing PAT / non-ADO remote is `blocked` before anything runs, then the verify command in the **worktree** (`skipVerify` ignored — no local merge to defer it to; red = `verify_failed`, nothing pushed), diff --git a/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs b/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs index 5064820d..394e905c 100644 --- a/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs +++ b/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs @@ -45,8 +45,6 @@ public sealed class WorktreeRepository .SetProperty(w => w.MergeCommit, mergeCommit), ct); } - /// Records a successful auto-rebase: the branch's fork point moves to the rebased-onto - /// commit and its head moves to the new tip git rebase produced. /// Stores the draft PR an approve in a company repo created or reused. public async Task SetPullRequestUrlAsync(string taskId, string url, CancellationToken ct = default) { @@ -55,6 +53,8 @@ public sealed class WorktreeRepository .ExecuteUpdateAsync(s => s.SetProperty(w => w.PullRequestUrl, url), ct); } + /// Records a successful auto-rebase: the branch's fork point moves to the rebased-onto + /// commit and its head moves to the new tip git rebase produced. public async Task SetRebasedAsync(string taskId, string baseCommit, string headCommit, CancellationToken ct = default) { await _context.Worktrees diff --git a/src/ClaudeDo.Worker/AzureDevOps/AzureDevOpsClient.cs b/src/ClaudeDo.Worker/AzureDevOps/AzureDevOpsClient.cs index 841d3aea..91c7cafa 100644 --- a/src/ClaudeDo.Worker/AzureDevOps/AzureDevOpsClient.cs +++ b/src/ClaudeDo.Worker/AzureDevOps/AzureDevOpsClient.cs @@ -121,7 +121,7 @@ public sealed class AzureDevOpsClient private static string StripHeads(string branch) => branch.StartsWith("refs/heads/", StringComparison.Ordinal) ? branch["refs/heads/".Length..] : branch; - private async Task SendAsync( + private async Task SendAsync( AzdoRepo repo, HttpMethod method, string path, string? query, string? body, CancellationToken ct) { var pat = Pat; @@ -153,7 +153,11 @@ public sealed class AzureDevOpsClient throw new AzureDevOpsException("Azure DevOps 203: PAT abgelaufen oder ohne Scope Code (Read & Write)."); if (response.IsSuccessStatusCode) { - try { return JsonNode.Parse(payload) ?? new JsonObject(); } + try + { + return JsonNode.Parse(payload) as JsonObject + ?? throw new AzureDevOpsException($"Unerwartete Antwort von Azure DevOps bei {method} {path}."); + } catch (JsonException ex) { throw new AzureDevOpsException($"Unerwartete Antwort von Azure DevOps: {ex.Message}"); } } @@ -174,9 +178,10 @@ public sealed class AzureDevOpsClient { try { - if (JsonNode.Parse(payload)?["message"]?.GetValue() is { Length: > 0 } msg) return msg; + if (JsonNode.Parse(payload) is JsonObject o && o["message"]?.GetValue() is { Length: > 0 } msg) return msg; } - catch (JsonException) { } + // InvalidOperationException: "message" exists but is not a string. + catch (Exception ex) when (ex is JsonException or InvalidOperationException) { } return payload.Length <= 500 ? payload : payload[..500]; } } diff --git a/src/ClaudeDo.Worker/AzureDevOps/PullRequestPublisher.cs b/src/ClaudeDo.Worker/AzureDevOps/PullRequestPublisher.cs index 02277496..105c241d 100644 --- a/src/ClaudeDo.Worker/AzureDevOps/PullRequestPublisher.cs +++ b/src/ClaudeDo.Worker/AzureDevOps/PullRequestPublisher.cs @@ -83,7 +83,8 @@ public sealed class PullRequestPublisher 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) + // InvalidOperationException: a field of the ADO answer has an unexpected JSON type. + catch (Exception ex) when (ex is AzureDevOpsException or InvalidOperationException) { return Blocked($"gepusht, PR fehlgeschlagen: {ex.Message}"); } diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index efa2662b..e460f55b 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -1054,7 +1054,8 @@ public sealed class TaskMergeService } } - if (wt is { State: WorktreeState.Active } && _pullRequests is not null + // Kept (parked or reverted) goes the PR way too: a local merge has no place in a company repo. + if (wt is { State: WorktreeState.Active or WorktreeState.Kept } && _pullRequests is not null && !string.IsNullOrWhiteSpace(list.WorkingDir) && await _pullRequests.IsCompanyRepoAsync(list.WorkingDir, ct)) return await ApproveCompanyAsync(task, list, wt, verifyCommand, ct, progress); @@ -1164,9 +1165,18 @@ public sealed class TaskMergeService if (failed is not null) return failed; } - return await ProgressReporter.RunAsync( + var result = await ProgressReporter.RunAsync( _pullRequests.PublishAsync(task, list, wt, ranVerify ? verifyCommand : null, ct), ProgressReportInterval, progress, "push + draft PR running"); + + // A branch with an open PR is under review again, whatever parked it before. + if (wt.State == WorktreeState.Kept && result.PullRequestUrl is not null) + { + using (var ctx = _dbFactory.CreateDbContext()) + await new WorktreeRepository(ctx).SetStateAsync(task.Id, WorktreeState.Active, ct); + await _broadcaster.WorktreeUpdated(task.Id); + } + return result; } /// diff --git a/src/ClaudeDo.Worker/Runner/WorktreeManager.cs b/src/ClaudeDo.Worker/Runner/WorktreeManager.cs index 3f446b22..84928d69 100644 --- a/src/ClaudeDo.Worker/Runner/WorktreeManager.cs +++ b/src/ClaudeDo.Worker/Runner/WorktreeManager.cs @@ -282,8 +282,17 @@ public sealed class WorktreeManager .Select(c => c.BaseBranch) .FirstOrDefaultAsync(ct)).NullIfBlank(); - if (isCompany && await FetchCompanyBaseAsync(workingDir, baseBranch, ct) is { } fetched) - return fetched; + if (isCompany) + { + try + { + if (await FetchCompanyBaseAsync(workingDir, baseBranch, ct) is { } fetched) return fetched; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Fetching the company base in {working_dir} failed, continuing with the local branch", workingDir); + } + } return baseBranch is null ? await _git.RevParseHeadAsync(workingDir, ct) @@ -304,7 +313,7 @@ public sealed class WorktreeManager if (basis is null && _azdo is not null && pat is not null && AzureDevOpsClient.ParseRemote(remote) is { } repo) { try { basis = await _azdo.GetDefaultBranchAsync(repo, ct); } - catch (AzureDevOpsException ex) { _logger.LogWarning("Default branch from Azure DevOps unavailable: {reason}", ex.Message); } + catch (Exception ex) when (ex is AzureDevOpsException or InvalidOperationException) { _logger.LogWarning("Default branch from Azure DevOps unavailable: {reason}", ex.Message); } } basis ??= await _git.GetRemoteHeadBranchAsync(workingDir, ct); if (basis is null) diff --git a/tests/ClaudeDo.Worker.Tests/AzureDevOps/CompanyApproveTests.cs b/tests/ClaudeDo.Worker.Tests/AzureDevOps/CompanyApproveTests.cs index 50fb250d..2b405613 100644 --- a/tests/ClaudeDo.Worker.Tests/AzureDevOps/CompanyApproveTests.cs +++ b/tests/ClaudeDo.Worker.Tests/AzureDevOps/CompanyApproveTests.cs @@ -158,6 +158,22 @@ public sealed class CompanyApproveTests : IDisposable Assert.StartsWith("Resolves [#412](https://ticket.fb-tuning.local/board/project/393/ticket/412)", description); } + [Fact] + public async Task Approve_KeptWorktree_OpensPrInsteadOfMergingLocally() + { + if (!GitRepoFixture.IsGitAvailable()) return; + await SetUpWorktree(); + using (var ctx = _db.CreateContext()) + await new WorktreeRepository(ctx).SetStateAsync("t1", WorktreeState.Kept); + var mainBefore = GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", "main").Trim(); + + var r = await Merge(Ado()).ApproveAndMergeAsync("t1", "", default); + + Assert.Equal(TaskMergeService.StatusPrCreated, r.Status); + Assert.Equal(mainBefore, GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", "main").Trim()); + Assert.Equal(WorktreeState.Active, (await Rows()).Wt.State); + } + [Fact] public async Task Approve_ReusesActivePr_AsUpdate() {