From e790acd28015402b0b722d310bc3a176a6e2eb24 Mon Sep 17 00:00:00 2001 From: Adam Frisby Date: Sat, 10 Oct 2026 03:58:46 +0000 Subject: [PATCH] Re-drive ownership: any-of trailer proof over current-base-tip range, naming offending SHAs Ownership proof demanded every configured trailer key on every exclusive commit, but writers stamp different subsets (agents write Prompt-Revision + Co-Authored-By only), so provably CodeyBox-owned branches were refused. The proof now accepts any exact-matched trailer from the instance vocabulary (defaults cover all five keys), evaluated over .. with the base tip resolved live, filtered by the CommitAttribution config (fully-disabled attribution requires confirmOwnership), and refusals name the offending commit SHAs in both the re-drive gate and the push-path diverged error. Co-Authored-By must equal the canonical identity exactly. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox --- docs/concepts/work-items.md | 2 +- docs/reference/configuration.md | 4 +- src/CodeyBox.Api/Program.cs | 3 +- src/CodeyBox.Core/CodeyBoxTrailers.cs | 8 +- src/CodeyBox.Core/IUpstreamRemote.cs | 29 +++ src/CodeyBox.Core/UpstreamBranchOwnership.cs | 168 ++++++++++++++---- .../UpstreamOwnedBranchDivergedException.cs | 15 ++ .../UpstreamRedriveService.cs | 83 +++++++-- .../GitHubUpstreamRemote.cs | 126 ++++++++++--- .../BranchOwnershipPolicyTests.cs | 102 ++++++++++- .../GitHubOwnedBranchPushTests.cs | 16 +- .../GitHubUpstreamTypedConflictTests.cs | 12 +- .../UpstreamRedriveRecoveryTests.cs | 160 ++++++++++++++++- 13 files changed, 629 insertions(+), 99 deletions(-) diff --git a/docs/concepts/work-items.md b/docs/concepts/work-items.md index 9402abac8..f13f387ae 100644 --- a/docs/concepts/work-items.md +++ b/docs/concepts/work-items.md @@ -178,7 +178,7 @@ POST /workitems/{id}/redrive-upstream `GET` lists settled items (`Failed`, `MergeConflictResolutionFailed`, `Merged`) whose own branch has a stale or diverged remote: items with an open PR on their branch (matched by recorded PR number or, for legacy items with no recorded number, by head branch), plus `upstream_blocked` items whose branch has a live remote tip but no open PR. Each entry carries the PR head, the last-pushed SHA, and the reason. `POST` records the open PR's currently observed head as the operator-authorized lease base (invoking it asserts no third party pushed to the branch) and resumes the upstream phase under the normal lease rules — a concurrent third-party push still fails the lease and parks. Returns `202` when armed, `404` when the item is gone, `409` when the state/branch/PR preconditions do not hold or the retry is refused. A startup scan logs the same candidates once at boot; nothing is pushed without the explicit `POST`. -Two recovery paths cover items the recorded-number join would miss. When the item has no recorded PR number, the open PR is discovered by head branch (`codeybox/`), recorded on the item, and used; when no open PR exists at all, the push is re-driven alone and the pipeline opens a fresh PR. When the branch has no recorded push, the remote tip must prove itself: the re-drive (and the push path itself) accepts the tip only when every commit exclusive of the base carries the configured CodeyBox trailers (`CodeyBox:UpstreamBranchOwnership`), otherwise it keeps refusing with a message that names this endpoint and the exact flag. Pass `{"confirmOwnership": true}` in the `POST` body to assert the tip is CodeyBox's own history explicitly — the assertion is logged and audited. +Two recovery paths cover items the recorded-number join would miss. When the item has no recorded PR number, the open PR is discovered by head branch (`codeybox/`), recorded on the item, and used; when no open PR exists at all, the push is re-driven alone and the pipeline opens a fresh PR. When the branch has no recorded push, the remote tip must prove itself: the re-drive (and the push path itself) accepts the tip only when every commit exclusive of the current base tip carries any accepted CodeyBox trailer (`CodeyBox:UpstreamBranchOwnership`) — commits already reachable from the base never count against the branch — otherwise it keeps refusing with a message that names the offending commit SHAs, this endpoint, and the exact flag. Pass `{"confirmOwnership": true}` in the `POST` body to assert the tip is CodeyBox's own history explicitly — the assertion is logged and audited. ### AbandonedAfterRecoveryAttempts diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 3c76daad1..6c0d88de1 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -626,14 +626,14 @@ Proves a stale CodeyBox-owned branch tip is CodeyBox's own history when no prior ```json "UpstreamBranchOwnership": { - "RequiredTrailerKeys": ["CodeyBox-WorkItem", "Co-Authored-By"], + "RequiredTrailerKeys": ["Co-Authored-By", "CodeyBox-WorkItem", "CodeyBox-Agent", "CodeyBox-Prompt-Revision", "CodeyBox-Mechanical-Fixer"], "MaxCommitsToVerify": 100 } ``` | Key | Default | Description | |-----|---------|-------------| -| `RequiredTrailerKeys` | `["CodeyBox-WorkItem", "Co-Authored-By"]` | Trailer keys that must each appear in the trailer block of every commit exclusive of the base. `Co-Authored-By` additionally requires its value to name CodeyBox. An empty list never proves ownership. | +| `RequiredTrailerKeys` | `["Co-Authored-By", "CodeyBox-WorkItem", "CodeyBox-Agent", "CodeyBox-Prompt-Revision", "CodeyBox-Mechanical-Fixer"]` | Trailer keys accepted as ownership proof, any-of: each commit exclusive of the current base tip counts as CodeyBox's own history when its trailer block carries ANY one of these keys (matched exactly). Any-of because each writer stamps a different subset (agents write `CodeyBox-Prompt-Revision` + `Co-Authored-By`; the orchestrator adds `CodeyBox-WorkItem` + `CodeyBox-Agent` / `CodeyBox-Mechanical-Fixer`). `Co-Authored-By` additionally requires its value to equal the canonical CodeyBox identity exactly. Keys from a family disabled by `CodeyBox:CommitAttribution` cannot prove ownership; when attribution disables every family (or the list is empty), nothing proves ownership and the re-drive requires `{"confirmOwnership": true}`. A refusal names the offending commit SHAs. | | `MaxCommitsToVerify` | `100` | Upper bound on the exclusive commits examined, enforced before buffering. A larger exclusive set is unverifiable and refused unless the operator passes `{"confirmOwnership": true}` on the re-drive. | ## `WorkerPoolHealthWatchdog` diff --git a/src/CodeyBox.Api/Program.cs b/src/CodeyBox.Api/Program.cs index d20a3857d..d0e8e0942 100644 --- a/src/CodeyBox.Api/Program.cs +++ b/src/CodeyBox.Api/Program.cs @@ -4875,7 +4875,8 @@ static Func DotnetTestRunOptionsAccessor(IServiceProvider sp) sp.GetRequiredService(), sp.GetRequiredService(), sp.GetRequiredService>(), - sp.GetRequiredService())); + sp.GetRequiredService(), + sp.GetRequiredService())); builder.Services.AddHostedService(sp => new StaleUpstreamRedriveDetector( sp.GetRequiredService(), sp.GetRequiredService>())); diff --git a/src/CodeyBox.Core/CodeyBoxTrailers.cs b/src/CodeyBox.Core/CodeyBoxTrailers.cs index 6280e4f3e..4943cf0ac 100644 --- a/src/CodeyBox.Core/CodeyBoxTrailers.cs +++ b/src/CodeyBox.Core/CodeyBoxTrailers.cs @@ -32,7 +32,13 @@ namespace CodeyBox.Core; /// public static class CodeyBoxTrailers { - public const string CoAuthoredBy = "Co-Authored-By: CodeyBox "; + /// + /// Canonical value of the terminal co-author trailer. Compared by exact + /// ordinal equality wherever ownership or attribution is decided — a + /// foreign Co-Authored-By value must never satisfy a CodeyBox check. + /// + public const string CoAuthoredByValue = "CodeyBox "; + public const string CoAuthoredBy = "Co-Authored-By: " + CoAuthoredByValue; public const string WorkItemTrailerKey = "CodeyBox-WorkItem"; public const string AgentTrailerKey = "CodeyBox-Agent"; public const string MechanicalFixerTrailerKey = "CodeyBox-Mechanical-Fixer"; diff --git a/src/CodeyBox.Core/IUpstreamRemote.cs b/src/CodeyBox.Core/IUpstreamRemote.cs index 6dca58a77..7c5e35f71 100644 --- a/src/CodeyBox.Core/IUpstreamRemote.cs +++ b/src/CodeyBox.Core/IUpstreamRemote.cs @@ -238,6 +238,27 @@ Task> ListOpenPullRequestsAsync( string baseBranch, string head, int maxCommits, CancellationToken ct = default) => Task.FromResult?>(null); + /// + /// Lists the commits reachable from (a branch + /// name or commit sha) but not from (a + /// branch name or commit sha — pass the resolved current base tip sha to + /// pin the range to <base tip>..<remote tip>), + /// oldest first, with each commit's sha and full message. Same + /// ownership-proof use, bounds, and null-when-unsupported contract as + /// , plus the shas so a + /// refusal can name exactly which commits blocked the proof. The default + /// derives from with unknown + /// shas; forges with a real commit API override this instead. + /// + async Task?> ListBranchCommitsAsync( + string baseRevision, string head, int maxCommits, CancellationToken ct = default) + { + var messages = await ListBranchCommitMessagesAsync(baseRevision, head, maxCommits, ct).ConfigureAwait(false); + return messages is null + ? null + : messages.Select(m => new OwnedBranchCommit(string.Empty, m)).ToList(); + } + /// /// Forge capability seam for audit check-run publication. Returns whether /// this remote can publish structured audit findings as forge check runs. @@ -278,6 +299,14 @@ public sealed record UpstreamPullRequestState( PullRequestStatus Status, string? MergeCommitSha); +/// +/// One commit in a branch's exclusive range (<base>..<head>), +/// oldest first. is the full commit sha (empty when the +/// forge did not report one); is the full commit +/// message, bounded by the listing implementation. +/// +public sealed record OwnedBranchCommit(string Sha, string Message); + /// /// Snapshot of an open pull request as seen by an /// at a point in time. Fields mirror the subset of the forge PR object the diff --git a/src/CodeyBox.Core/UpstreamBranchOwnership.cs b/src/CodeyBox.Core/UpstreamBranchOwnership.cs index 4faf90ff8..b17eb983e 100644 --- a/src/CodeyBox.Core/UpstreamBranchOwnership.cs +++ b/src/CodeyBox.Core/UpstreamBranchOwnership.cs @@ -9,16 +9,26 @@ namespace CodeyBox.Core; public sealed class UpstreamBranchOwnershipOptions { /// - /// Trailer keys that must each appear on every commit reachable from the - /// remote tip but not from the base for the tip to count as CodeyBox's - /// own history. Matched as Key: value trailer lines (key compared - /// ordinal-ignore-case); Co-Authored-By additionally requires its - /// value to name CodeyBox so a foreign Co-Authored-By: attacker - /// line cannot pass. Defaults cover the trailers every CodeyBox-emitted - /// commit carries (see ). + /// Trailer keys accepted as ownership proof, any-of: a commit reachable + /// from the remote tip but not from the current base tip counts as + /// CodeyBox's own history when its trailer block carries ANY one of + /// these keys (matched exactly; Co-Authored-By additionally + /// requires its value to be exactly the canonical CodeyBox identity so + /// a foreign Co-Authored-By: attacker line cannot pass). Any-of + /// because the writers stamp different sets: agents write + /// CodeyBox-Prompt-Revision + Co-Authored-By, the + /// orchestrator writes CodeyBox-WorkItem + CodeyBox-Agent / + /// CodeyBox-Mechanical-Fixer + Co-Authored-By (see + /// ). Requiring every key on every commit + /// refused branches that are provably CodeyBox's own. Defaults cover + /// every trailer key this instance writes (see + /// ); an + /// empty list never proves ownership. Attribution filtering applies on + /// top (see ): + /// keys the instance no longer writes cannot prove anything it wrote. /// public string[] RequiredTrailerKeys { get; set; } = - [CodeyBoxTrailers.WorkItemTrailerKey, "Co-Authored-By"]; + [.. BranchOwnershipPolicy.DefaultRequiredTrailerKeys]; /// /// Upper bound on the exclusive commits examined for ownership proof. @@ -62,68 +72,160 @@ public void Replace(UpstreamBranchOwnershipOptions next) /// /// Pure ownership proof over commit messages: a remote tip counts as /// CodeyBox's own history only when EVERY commit reachable from it but not -/// from the base carries the CodeyBox trailers. A single unmarked commit -/// fails the whole tip — a third party's commit anywhere in the exclusive -/// range must never be rewritten. +/// from the current base tip carries ANY accepted CodeyBox trailer in its +/// trailer block. A single unmarked commit fails the whole tip — a third +/// party's commit anywhere in the exclusive range must never be rewritten. /// public static class BranchOwnershipPolicy { + /// + /// Every trailer key this instance writes on CodeyBox-emitted commits + /// (see ). A commit carrying any one of + /// these (subject to attribution filtering) is CodeyBox's own. + /// + public static readonly string[] KnownCodeyBoxTrailerKeys = + [ + "Co-Authored-By", + CodeyBoxTrailers.WorkItemTrailerKey, + CodeyBoxTrailers.AgentTrailerKey, + CodeyBoxTrailers.PromptRevisionTrailerKey, + CodeyBoxTrailers.MechanicalFixerTrailerKey, + ]; + /// /// Default required trailer keys when no configured options are /// available. Single source of truth for the fallback; the configured /// wins - /// whenever options are present. + /// whenever options are present. Covers every trailer key this instance + /// writes (see ) because each + /// writer stamps a different subset. /// public static readonly string[] DefaultRequiredTrailerKeys = - [CodeyBoxTrailers.WorkItemTrailerKey, "Co-Authored-By"]; + [.. KnownCodeyBoxTrailerKeys]; /// - /// True when carries every key in - /// as a Key: value trailer - /// line. Keys compare ordinal-ignore-case; values must be non-empty, and - /// a Co-Authored-By value must name CodeyBox (ordinal) so a + /// True when carries ANY key in + /// as a Key: value trailer + /// line. Keys compare ordinally (exact); values must be non-empty, and a + /// Co-Authored-By value must exactly equal the canonical CodeyBox + /// identity () so a /// foreign co-author line cannot satisfy the check. /// - public static bool IsOwnedCommit(string? commitMessage, IReadOnlyList requiredTrailerKeys) + public static bool IsOwnedCommit(string? commitMessage, IReadOnlyList acceptedTrailerKeys) { - if (string.IsNullOrWhiteSpace(commitMessage) || requiredTrailerKeys.Count == 0) + if (string.IsNullOrWhiteSpace(commitMessage) || acceptedTrailerKeys.Count == 0) return false; - foreach (var key in requiredTrailerKeys) + foreach (var key in acceptedTrailerKeys) { if (string.IsNullOrWhiteSpace(key)) - return false; - if (!HasTrailer(commitMessage, key.Trim())) - return false; + continue; + if (HasTrailer(commitMessage, key.Trim())) + return true; } - return true; + return false; } /// - /// True when is non-empty and - /// every entry satisfies . An empty range is - /// NOT ownership (nothing proves CodeyBox wrote the tip); callers with a + /// True when is non-empty and every + /// entry satisfies . An empty range is NOT + /// ownership (nothing proves CodeyBox wrote the tip); callers with a /// vacuous range decide separately. /// public static bool AreAllCommitsOwned( IReadOnlyList exclusiveCommitMessages, - IReadOnlyList requiredTrailerKeys) + IReadOnlyList acceptedTrailerKeys) { if (exclusiveCommitMessages.Count == 0) return false; foreach (var message in exclusiveCommitMessages) { - if (!IsOwnedCommit(message, requiredTrailerKeys)) + if (!IsOwnedCommit(message, acceptedTrailerKeys)) return false; } return true; } + /// + /// Shas of the commits in that carry + /// no accepted trailer, oldest first. Empty when the whole range is + /// CodeyBox's own. Callers name these shas in the refusal so the operator + /// can inspect exactly which commits blocked the proof. + /// + public static IReadOnlyList FindUnownedCommitShas( + IReadOnlyList exclusiveCommits, + IReadOnlyList acceptedTrailerKeys) + { + var unowned = new List(); + foreach (var commit in exclusiveCommits) + { + if (!IsOwnedCommit(commit.Message, acceptedTrailerKeys)) + unowned.Add(commit.Sha); + } + return unowned; + } + + /// + /// Human-readable summary of for refusal + /// messages: up to shas named, the rest + /// counted. Entries are forge-reported shas already normalized to hex by + /// the listing implementation; blanks are never emitted, only counted. + /// + public static string DescribeUnownedCommits(IReadOnlyList unownedShas, int maxToName = 10) + { + ArgumentNullException.ThrowIfNull(unownedShas); + if (maxToName < 1) + maxToName = 1; + var named = unownedShas.Where(s => !string.IsNullOrWhiteSpace(s)).Take(maxToName).ToList(); + if (named.Count == 0) + return "unidentified commits"; + var remaining = unownedShas.Count - named.Count; + var summary = string.Join(", ", named); + if (remaining > 0) + summary += $" (and {remaining} more)"; + return summary; + } + + /// + /// Narrows the configured ownership keys to what this instance can still + /// have written under : a disabled + /// Co-Authored-By family cannot prove anything (its line is + /// stripped from everything CodeyBox writes), and likewise for disabled + /// CodeyBox-* trailers. An empty result proves nothing — callers + /// must require explicit ownership confirmation. Configured keys compare + /// ordinal-ignore-case here (operator-written config); commit trailer + /// lines still match exactly (see ). + /// + public static string[] EffectiveAcceptedKeys( + IReadOnlyList configuredKeys, + CommitAttribution attribution) + { + ArgumentNullException.ThrowIfNull(configuredKeys); + ArgumentNullException.ThrowIfNull(attribution); + var accepted = new List(configuredKeys.Count); + foreach (var key in configuredKeys) + { + if (string.IsNullOrWhiteSpace(key)) + continue; + var trimmed = key.Trim(); + if (!attribution.IncludeCoAuthoredBy + && trimmed.Equals("Co-Authored-By", StringComparison.OrdinalIgnoreCase)) + continue; + if (!attribution.IncludeCodeyBoxTrailers + && trimmed.StartsWith("CodeyBox-", StringComparison.OrdinalIgnoreCase)) + continue; + accepted.Add(trimmed); + } + return [.. accepted]; + } + private static bool HasTrailer(string message, string key) { // Trailer block = contiguous trailing lines of `Token: value` form // (git trailer semantics: only the end-of-message block counts, so a // quoted trailer line in the body middle cannot satisfy the check). - // The walk stops at the first blank or non-trailer line. + // The walk stops at the first blank or non-trailer line. Keys match + // exactly (ordinal): CodeyBox always writes canonical-case keys, and + // a near-miss must not pass as proof of authorship. var lines = message.Split('\n'); for (var i = lines.Length - 1; i >= 0; i--) { @@ -139,10 +241,10 @@ private static bool HasTrailer(string message, string key) var value = line[(colon + 1)..].Trim(); if (value.Length == 0) continue; - if (!name.Equals(key, StringComparison.OrdinalIgnoreCase)) + if (!name.Equals(key, StringComparison.Ordinal)) continue; - if (string.Equals(key, "Co-Authored-By", StringComparison.OrdinalIgnoreCase) - && !value.Contains("CodeyBox", StringComparison.Ordinal)) + if (string.Equals(key, "Co-Authored-By", StringComparison.Ordinal) + && !value.Equals(CodeyBoxTrailers.CoAuthoredByValue, StringComparison.Ordinal)) continue; return true; } diff --git a/src/CodeyBox.Core/UpstreamOwnedBranchDivergedException.cs b/src/CodeyBox.Core/UpstreamOwnedBranchDivergedException.cs index d1f95bb56..d974ba2b9 100644 --- a/src/CodeyBox.Core/UpstreamOwnedBranchDivergedException.cs +++ b/src/CodeyBox.Core/UpstreamOwnedBranchDivergedException.cs @@ -26,9 +26,24 @@ public sealed class UpstreamOwnedBranchDivergedException : InvalidOperationExcep public const string ConfirmOwnershipFlag = "confirmOwnership"; public UpstreamOwnedBranchDivergedException(string branch, string remoteSha, string localSha) + : this(branch, remoteSha, localSha, unownedCommitShas: null) + { + } + + /// + /// Same refusal, naming the that + /// blocked the trailer proof so the operator can inspect exactly which + /// commits look third-party. Null (unverifiable history) keeps the base + /// message; an empty list is owned and should never reach this path. + /// + public UpstreamOwnedBranchDivergedException( + string branch, string remoteSha, string localSha, IReadOnlyList? unownedCommitShas) : base($"work branch '{branch}' is not a fast-forward of its previous upstream push " + $"(remote {remoteSha}, local {localSha}); this is not a merge conflict — the work was already recomposed on a fresh base. " + "CodeyBox has no recorded prior push for this branch, so it cannot prove the remote tip is its own history. " + + (unownedCommitShas is { Count: > 0 } + ? $"Commit(s) without a CodeyBox trailer: {BranchOwnershipPolicy.DescribeUnownedCommits(unownedCommitShas)}. " + : string.Empty) + $"Re-drive the upstream step with {RedriveEndpointTemplate} after confirming no third party pushed to the branch. " + "When every commit on the remote tip carries the CodeyBox trailers the rewrite is accepted automatically; " + $"otherwise the re-drive must pass {{\"{ConfirmOwnershipFlag}\": true}} to assert the remote tip is CodeyBox's own history.") diff --git a/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs b/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs index 6c549ba16..7535cdfef 100644 --- a/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs +++ b/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs @@ -30,6 +30,7 @@ public sealed class UpstreamRedriveService private readonly WorkItemRetrier _retrier; private readonly ILogger _log; private readonly UpstreamBranchOwnershipSnapshot _ownership; + private readonly CommitAttributionPolicy? _attributionPolicy; public UpstreamRedriveService( IWorkItemStore store, @@ -37,7 +38,8 @@ public UpstreamRedriveService( IUpstreamRemoteFactory upstreamFactory, WorkItemRetrier retrier, ILogger log, - UpstreamBranchOwnershipSnapshot? ownership = null) + UpstreamBranchOwnershipSnapshot? ownership = null, + CommitAttributionPolicy? attributionPolicy = null) { _store = store; _projects = projects; @@ -45,6 +47,7 @@ public UpstreamRedriveService( _retrier = retrier; _log = log; _ownership = ownership ?? new UpstreamBranchOwnershipSnapshot(new UpstreamBranchOwnershipOptions()); + _attributionPolicy = attributionPolicy; } public sealed record UpstreamRedriveCandidate( @@ -213,8 +216,8 @@ public Task RedriveAsync(WorkItemId id, CancellationToken /// tip, never a blind overwrite. /// /// Ownership proof for branches with no recorded push: the remote tip is - /// accepted only when every commit exclusive of the base carries the - /// configured CodeyBox trailers, or when + /// accepted only when every commit exclusive of the current base tip + /// carries any accepted CodeyBox trailer, or when /// is true (the operator asserts the tip is CodeyBox's own history; logged /// and audited). Otherwise the re-drive is refused. /// @@ -366,10 +369,13 @@ public async Task RedriveAsync( /// /// Ownership gate for a tip with no recorded push and no explicit - /// operator confirmation. Returns null when the tip is accepted (all - /// exclusive commits carry the configured trailers, or the range is - /// empty), otherwise a refusal result naming the working confirmation - /// flag. + /// operator confirmation. The range is pinned to the CURRENT base tip + /// (<base tip>..<remote tip>) so commits already + /// reachable from the base — e.g. operator commits merged into the work + /// branch — never count as third-party work. Returns null when the tip + /// is accepted (every exclusive commit carries any accepted CodeyBox + /// trailer, or the range is empty), otherwise a refusal result naming + /// the offending commit shas and the working confirmation flag. /// private async Task CheckOwnershipGateAsync( IUpstreamRemote upstream, @@ -392,11 +398,25 @@ public async Task RedriveAsync( RedriveConfirmationHint(item.Id), null, null, 0, string.Empty); - IReadOnlyList? messages; + var attribution = _attributionPolicy?.Resolve(project) ?? CommitAttribution.Default; + var accepted = BranchOwnershipPolicy.EffectiveAcceptedKeys( + _ownership.Current.RequiredTrailerKeys ?? [], attribution); + if (accepted.Length == 0) + return new UpstreamRedriveResult( + false, + $"cannot re-drive upstream: branch '{workBranch}' has no recorded push and commit attribution is disabled, " + + $"so no trailer can prove tip {observedTip} is CodeyBox's own history. " + + RedriveConfirmationHint(item.Id), + null, null, 0, string.Empty); + + var baseRevision = await SafeResolveBaseRevisionAsync(upstream, baseBranch, ct).ConfigureAwait(false) + ?? baseBranch; + + IReadOnlyList? commits; try { - messages = await upstream.ListBranchCommitMessagesAsync( - baseBranch, observedTip, Math.Max(1, _ownership.Current.MaxCommitsToVerify), ct).ConfigureAwait(false); + commits = await upstream.ListBranchCommitsAsync( + baseRevision, observedTip, Math.Max(1, _ownership.Current.MaxCommitsToVerify), ct).ConfigureAwait(false); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -407,24 +427,57 @@ public async Task RedriveAsync( _log.LogDebug( "Upstream re-drive ownership check for work item {Id} failed: {Message}", item.Id.ToString(), ex.Message); - messages = null; + commits = null; } - if (messages is null) + if (commits is null) return new UpstreamRedriveResult( false, $"cannot re-drive upstream: branch '{workBranch}' has no recorded push and its tip {observedTip} cannot be proven to be CodeyBox's own history (commit history unreadable). " + RedriveConfirmationHint(item.Id), null, null, 0, string.Empty); - var keys = _ownership.Current.RequiredTrailerKeys ?? []; - if (messages.Count == 0 || BranchOwnershipPolicy.AreAllCommitsOwned(messages, keys)) + if (commits.Count == 0) + return null; + var unowned = BranchOwnershipPolicy.FindUnownedCommitShas(commits, accepted); + if (unowned.Count == 0) return null; return new UpstreamRedriveResult( false, - $"cannot re-drive upstream: remote tip {observedTip} of '{workBranch}' contains commits without the CodeyBox trailers, so it may include third-party work. " + + $"cannot re-drive upstream: remote tip {observedTip} of '{workBranch}' contains {unowned.Count} commit(s) without a CodeyBox trailer " + + $"({BranchOwnershipPolicy.DescribeUnownedCommits(unowned)}), so it may include third-party work. " + RedriveConfirmationHint(item.Id), null, null, 0, string.Empty); } + /// + /// Resolves to its current head sha so the + /// ownership range excludes everything already on the base. Returns null + /// when the tip cannot be read; callers fall back to the branch name, + /// which the forge resolves to the same tip. + /// + private async Task SafeResolveBaseRevisionAsync( + IUpstreamRemote upstream, string baseBranch, CancellationToken ct) + { + try + { + var sha = await upstream.GetBranchHeadShaAsync(baseBranch, ct).ConfigureAwait(false); + if (string.IsNullOrWhiteSpace(sha)) + return null; + Validation.ValidateCommitSha(sha.Trim(), "base branch head sha"); + return sha.Trim().ToLowerInvariant(); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + _log.LogDebug( + "Upstream re-drive could not resolve current tip of base branch {Branch}: {Message}", + baseBranch, ex.Message); + return null; + } + } + private static string RedriveConfirmationHint(WorkItemId id) => $"After confirming no third party pushed to the branch, re-drive with POST /workitems/{id}/redrive-upstream passing {{\"{UpstreamOwnedBranchDivergedException.ConfirmOwnershipFlag}\": true}} to assert the remote tip is CodeyBox's own history."; diff --git a/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs b/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs index 7b03fd4a3..0662c8460 100644 --- a/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs +++ b/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs @@ -507,7 +507,18 @@ public async Task> ListOpenPullRequestsAsync( public async Task?> ListBranchCommitMessagesAsync( string baseBranch, string head, int maxCommits, CancellationToken ct = default) { - Validation.ValidateBranchName(baseBranch, nameof(baseBranch)); + var commits = await ListBranchCommitsAsync(baseBranch, head, maxCommits, ct).ConfigureAwait(false); + return commits?.Select(c => c.Message).ToList(); + } + + /// + public async Task?> ListBranchCommitsAsync( + string baseRevision, string head, int maxCommits, CancellationToken ct = default) + { + if (IsCommitSha(baseRevision)) + Validation.ValidateCommitSha(baseRevision, nameof(baseRevision)); + else + Validation.ValidateBranchName(baseRevision, nameof(baseRevision)); if (string.IsNullOrWhiteSpace(head)) throw new ArgumentException("head must not be empty", nameof(head)); if (IsCommitSha(head)) @@ -520,11 +531,11 @@ public async Task> ListOpenPullRequestsAsync( const int perPage = 100; const int maxMessageBytes = 32768; var pageSize = Math.Min(perPage, maxCommits); - var messages = new List(); + var commits = new List(); for (var page = 1; ; page++) { var url = $"https://api.github.com/repos/{_opts.Owner}/{_opts.Repository}/compare/" + - $"{Uri.EscapeDataString(baseBranch)}...{Uri.EscapeDataString(head)}" + + $"{Uri.EscapeDataString(baseRevision)}...{Uri.EscapeDataString(head)}" + $"?per_page={pageSize}&page={page}"; using var req = await BuildRequestAsync(HttpMethod.Get, url, ct); using var resp = await SendAsync(req, ct); @@ -533,7 +544,7 @@ public async Task> ListOpenPullRequestsAsync( AuditLog.UpstreamApiCallFailed("GET /compare", (int)resp.StatusCode, _opts.Owner, _opts.Repository); _log.LogDebug( "GitHub compare {Base}...{Head} returned {Status}; ownership unverifiable", - SanitizeForLog(baseBranch), SanitizeForLog(head), (int)resp.StatusCode); + SanitizeForLog(baseRevision), SanitizeForLog(head), (int)resp.StatusCode); return null; } var comparison = await resp.Content.ReadFromJsonAsync(ct); @@ -543,7 +554,7 @@ public async Task> ListOpenPullRequestsAsync( { _log.LogDebug( "GitHub compare {Base}...{Head} reports {Total} exclusive commits (cap {Cap}); ownership unverifiable", - SanitizeForLog(baseBranch), SanitizeForLog(head), comparison.TotalCommits, maxCommits); + SanitizeForLog(baseRevision), SanitizeForLog(head), comparison.TotalCommits, maxCommits); return null; } foreach (var commit in comparison.Commits ?? []) @@ -551,38 +562,63 @@ public async Task> ListOpenPullRequestsAsync( var message = commit.Commit?.Message; if (string.IsNullOrWhiteSpace(message)) continue; - messages.Add(TruncateMessageTail(message.Trim(), maxMessageBytes)); + commits.Add(new OwnedBranchCommit( + NormalizeCommitSha(commit.Sha), + TruncateMessageTail(message.Trim(), maxMessageBytes))); } if ((comparison.Commits?.Count ?? 0) < pageSize) break; } - return messages; + return commits; + } + + /// + /// Forge-reported commit shas are untrusted input: keep only plausible + /// hex shas (lowercased) so refusal messages naming offending commits + /// cannot carry terminal escapes or prose. Anything else becomes empty + /// and is omitted from the named list. + /// + private static string NormalizeCommitSha(string? sha) + { + var candidate = (sha ?? string.Empty).Trim().ToLowerInvariant(); + if (candidate.Length is < 40 or > 64) + return string.Empty; + foreach (var c in candidate) + { + if (!Uri.IsHexDigit(c)) + return string.Empty; + } + return candidate; } /// /// Proves a diverged remote tip is CodeyBox's own history by listing the - /// commits exclusive of through the compare - /// API and requiring every one to carry the configured trailers. An empty - /// exclusive set is safe (the tip sits at or behind the base, so nothing - /// foreign can be lost); any unreadable, oversized, or unmarked range is - /// unverified — never assumed. Never throws except on cancellation. + /// commits exclusive of the CURRENT base tip (<base tip>..<remote tip>) + /// through the compare API and requiring every one to carry any accepted + /// CodeyBox trailer. Returns the shas of commits that block the proof + /// (empty = owned; null = unverifiable). An empty exclusive set is + /// safe (the tip sits at or behind the base, so nothing foreign can be + /// lost); any unreadable, oversized, or unmarked range is unverified — + /// never assumed. Never throws except on cancellation. /// - private async Task TryVerifyRemoteTipOwnedAsync( + private async Task?> TryVerifyRemoteTipOwnedAsync( string baseBranch, string remoteSha, CancellationToken ct) { try { var options = _ownership.Current; - var keys = options.RequiredTrailerKeys ?? []; - if (keys.Length == 0) - return false; - var messages = await ListBranchCommitMessagesAsync( - baseBranch, remoteSha, Math.Max(1, options.MaxCommitsToVerify), ct).ConfigureAwait(false); - if (messages is null) - return false; - if (messages.Count == 0) - return true; - return BranchOwnershipPolicy.AreAllCommitsOwned(messages, keys); + var accepted = BranchOwnershipPolicy.EffectiveAcceptedKeys( + options.RequiredTrailerKeys ?? [], _attribution); + if (accepted.Length == 0) + return null; + var baseRevision = await ResolveBaseRevisionAsync(baseBranch, ct).ConfigureAwait(false); + var commits = await ListBranchCommitsAsync( + baseRevision, remoteSha, Math.Max(1, options.MaxCommitsToVerify), ct).ConfigureAwait(false); + if (commits is null) + return null; + if (commits.Count == 0) + return []; + return BranchOwnershipPolicy.FindUnownedCommitShas(commits, accepted); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -593,10 +629,44 @@ private async Task TryVerifyRemoteTipOwnedAsync( _log.LogDebug( "Ownership verification of remote tip {Sha} failed ({Message}); treating as unverified", SanitizeForLog(remoteSha), ex.Message); - return false; + return null; } } + /// + /// Pins the ownership range to the live base tip: resolves + /// to its current head sha so commits + /// already reachable from the base (e.g. operator commits merged into + /// the work branch) never count as third-party work. Falls back to the + /// branch name — which the compare endpoint resolves to the same tip — + /// when the tip cannot be read. + /// + private async Task ResolveBaseRevisionAsync(string baseBranch, CancellationToken ct) + { + try + { + var tip = await GetBranchHeadShaAsync(baseBranch, ct).ConfigureAwait(false); + if (!string.IsNullOrWhiteSpace(tip)) + { + _log.LogDebug( + "Ownership check on base {Branch} pinned to current tip {Sha}", + SanitizeForLog(baseBranch), SanitizeForLog(tip)); + return tip; + } + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + _log.LogDebug( + "Could not resolve current tip of base {Branch} ({Message}); comparing against the branch name", + SanitizeForLog(baseBranch), ex.Message); + } + return baseBranch; + } + private static bool IsCommitSha(string value) { if (value.Length is < 40 or > 64) @@ -1102,14 +1172,15 @@ await _gitHost.PushToUpstreamAsync( { // No recorded prior push to lease from: accept the remote tip as // CodeyBox's own history only when every commit exclusive of the - // base carries the configured CodeyBox trailers. A single + // current base tip carries an accepted CodeyBox trailer. A single // unmarked commit keeps the refusal — a third party's history is // never rewritten without the operator's explicit re-drive // confirmation. Verification failures (unreadable commits, // oversized ranges, transient API errors) are unverified, never // assumed. - if (!await TryVerifyRemoteTipOwnedAsync(request.BaseBranch, remoteSha, ct).ConfigureAwait(false)) - throw new UpstreamOwnedBranchDivergedException(request.WorkBranch, remoteSha, localTip); + var unowned = await TryVerifyRemoteTipOwnedAsync(request.BaseBranch, remoteSha, ct).ConfigureAwait(false); + if (unowned is null || unowned.Count > 0) + throw new UpstreamOwnedBranchDivergedException(request.WorkBranch, remoteSha, localTip, unowned); leaseBase = remoteSha; _log.LogInformation( "Work branch {WorkBranch} remote tip {Sha} proven CodeyBox-owned by commit trailers; rewriting under lease", @@ -2592,6 +2663,7 @@ internal sealed record GitHubMergeResponse( [property: JsonPropertyName("merged")] bool Merged = false); internal sealed record GitHubPullRequestCommitResponse( + [property: JsonPropertyName("sha")] string? Sha, [property: JsonPropertyName("commit")] GitHubPullRequestCommitDetail? Commit); internal sealed record GitHubBranchDetail( diff --git a/tests/CodeyBox.Tests/BranchOwnershipPolicyTests.cs b/tests/CodeyBox.Tests/BranchOwnershipPolicyTests.cs index 6aeb14e96..00260e02f 100644 --- a/tests/CodeyBox.Tests/BranchOwnershipPolicyTests.cs +++ b/tests/CodeyBox.Tests/BranchOwnershipPolicyTests.cs @@ -4,9 +4,9 @@ namespace CodeyBox.Tests; /// /// Pure ownership proof over commit messages: a remote tip counts as -/// CodeyBox's own history only when every exclusive commit carries the -/// configured trailers in its trailer block. Each test pins a value the -/// policy produced so a single-statement production mutation flips it red. +/// CodeyBox's own history only when every exclusive commit carries ANY +/// accepted CodeyBox trailer in its trailer block. Each test pins a value +/// the policy produced so a single-statement production mutation flips it red. /// public sealed class BranchOwnershipPolicyTests { @@ -24,25 +24,50 @@ public void OwnedCommit_WithAllTrailers_IsOwned() } [Fact] - public void Commit_MissingOneTrailer_IsNotOwned() + public void Commit_WithSingleAcceptedTrailer_IsOwned() { - const string message = - "agent work\n\nCo-Authored-By: CodeyBox "; - - Assert.False(BranchOwnershipPolicy.IsOwnedCommit(message, Keys)); + // Writers stamp different sets: agents write Prompt-Revision + + // Co-Authored-By only, so any single accepted key must suffice. + Assert.True(BranchOwnershipPolicy.IsOwnedCommit( + "agent work\n\nCodeyBox-Prompt-Revision: 3\nCo-Authored-By: CodeyBox ", + ["CodeyBox-WorkItem", "Co-Authored-By", "CodeyBox-Prompt-Revision"])); + Assert.True(BranchOwnershipPolicy.IsOwnedCommit( + "agent work\n\nCo-Authored-By: CodeyBox ", + Keys)); + Assert.True(BranchOwnershipPolicy.IsOwnedCommit( + "fixer work\n\nCodeyBox-Mechanical-Fixer: whitespace", + ["CodeyBox-Mechanical-Fixer"])); } [Fact] public void Commit_ForeignCoAuthorValue_IsNotOwned() { + // A foreign Co-Authored-By line as the commit's only trailer claim + // proves nothing, even when it names a plausible value shape. const string message = "third party change\n\n" + - "CodeyBox-WorkItem: 6f2c9a1e3b4d5f6a7b8c9d0e1f2a3b4c\n" + "Co-Authored-By: Mallory "; Assert.False(BranchOwnershipPolicy.IsOwnedCommit(message, Keys)); } + [Fact] + public void Commit_CoAuthorKeyOrValueMismatch_IsNotOwned() + { + // Keys match exactly: a wrong-case key is not this instance's trailer. + Assert.False(BranchOwnershipPolicy.IsOwnedCommit( + "work\n\nco-authored-by: CodeyBox ", + Keys)); + // Values match exactly: the bare name without the canonical identity is not proof. + Assert.False(BranchOwnershipPolicy.IsOwnedCommit( + "work\n\nCo-Authored-By: CodeyBox", + Keys)); + // A foreign value that merely contains the name is not proof either. + Assert.False(BranchOwnershipPolicy.IsOwnedCommit( + "work\n\nCo-Authored-By: Mallory pretending to be CodeyBox ", + Keys)); + } + [Fact] public void Commit_TrailerQuotedInBodyMiddle_IsNotOwned() { @@ -90,4 +115,63 @@ public void DefaultKeys_CoverWorkItemAndCoAuthoredBy() options.RequiredTrailerKeys); Assert.True(options.MaxCommitsToVerify > 0); } + + [Fact] + public void KnownKeys_CoverEveryTrailerThisInstanceWrites() + { + Assert.Contains("Co-Authored-By", BranchOwnershipPolicy.KnownCodeyBoxTrailerKeys); + Assert.Contains(CodeyBoxTrailers.WorkItemTrailerKey, BranchOwnershipPolicy.KnownCodeyBoxTrailerKeys); + Assert.Contains(CodeyBoxTrailers.AgentTrailerKey, BranchOwnershipPolicy.KnownCodeyBoxTrailerKeys); + Assert.Contains(CodeyBoxTrailers.PromptRevisionTrailerKey, BranchOwnershipPolicy.KnownCodeyBoxTrailerKeys); + Assert.Contains(CodeyBoxTrailers.MechanicalFixerTrailerKey, BranchOwnershipPolicy.KnownCodeyBoxTrailerKeys); + } + + [Fact] + public void EffectiveAcceptedKeys_DropsFamiliesDisabledByAttribution() + { + string[] configured = ["CodeyBox-WorkItem", "CodeyBox-Agent", "Co-Authored-By"]; + + var coAuthorOff = BranchOwnershipPolicy.EffectiveAcceptedKeys( + configured, new CommitAttribution(includeCoAuthoredBy: false, includeCodeyBoxTrailers: true, includePullRequestFooter: true)); + Assert.Equal(["CodeyBox-WorkItem", "CodeyBox-Agent"], coAuthorOff); + + var trailersOff = BranchOwnershipPolicy.EffectiveAcceptedKeys( + configured, new CommitAttribution(includeCoAuthoredBy: true, includeCodeyBoxTrailers: false, includePullRequestFooter: true)); + Assert.Equal(["Co-Authored-By"], trailersOff); + + var allOff = BranchOwnershipPolicy.EffectiveAcceptedKeys( + configured, new CommitAttribution(includeCoAuthoredBy: false, includeCodeyBoxTrailers: false, includePullRequestFooter: true)); + Assert.Empty(allOff); + + var allOn = BranchOwnershipPolicy.EffectiveAcceptedKeys( + configured, CommitAttribution.Default); + Assert.Equal(configured, allOn); + } + + [Fact] + public void FindUnownedCommitShas_NamesOnlyTrailerlessCommits() + { + var owned = new OwnedBranchCommit( + new string('a', 40), + "agent work\n\nCo-Authored-By: CodeyBox "); + var foreign = new OwnedBranchCommit(new string('b', 40), "third party change"); + + var unowned = BranchOwnershipPolicy.FindUnownedCommitShas([owned, foreign], Keys); + + Assert.Equal([new string('b', 40)], unowned); + Assert.Empty(BranchOwnershipPolicy.FindUnownedCommitShas([owned, owned], Keys)); + } + + [Fact] + public void DescribeUnownedCommits_NamesShasAndCountsRemainder() + { + var shas = Enumerable.Range(0, 12).Select(i => new string((char)('a' + i), 40)).ToList(); + + var summary = BranchOwnershipPolicy.DescribeUnownedCommits(shas); + + Assert.Contains(shas[0], summary); + Assert.Contains("(and 2 more)", summary); + Assert.Equal("unidentified commits", BranchOwnershipPolicy.DescribeUnownedCommits([])); + Assert.Equal("unidentified commits", BranchOwnershipPolicy.DescribeUnownedCommits(["", " "])); + } } diff --git a/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs b/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs index c6600d826..a4413eb83 100644 --- a/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs +++ b/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs @@ -92,7 +92,8 @@ public async Task DivergedNoRecord_TrailerTip_PushesUnderLeaseAndReusesPr() var (owner, repo) = SplitGithubUrl(setup.GithubUrl); var handler = new FakeHttpMessageHandler(); - handler.Enqueue(Json(new // GET /compare/main...old: the stale tip is CodeyBox's own + handler.Enqueue(Json(new { commit = new { sha = new string('c', 40) } })); // GET /branches/main: current base tip + handler.Enqueue(Json(new // GET /compare/...old: the stale tip is CodeyBox's own { total_commits = 1, commits = new[] { new { sha = oldHead, commit = new { message = trailerMessage } } }, @@ -178,9 +179,11 @@ public async Task DivergedWithoutRecord_ParksDistinctFromMergeConflict() // No ExpectedRemoteHeadSha: the remote tip ("old work", no CodeyBox // trailers) cannot be proven to be CodeyBox's own history, so the // push must refuse — distinctly from a merge conflict, which would - // route into conflict-rework. The compare API is consulted for the - // trailer proof; the refusal leaves the remote intact and issues no - // PR call. + // route into conflict-rework. The base tip is resolved first so the + // compare range excludes everything already on the base; the compare + // API is then consulted for the trailer proof. The refusal leaves + // the remote intact, issues no PR call, and names the offending sha. + handler.Enqueue(Json(new { commit = new { sha = new string('c', 40) } })); // GET /branches/main: current base tip handler.Enqueue(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent( @@ -199,8 +202,9 @@ public async Task DivergedWithoutRecord_ParksDistinctFromMergeConflict() Assert.False(UpstreamPushReconcileConflictException.TryFindIn(ex, out _)); var (_, tip, _) = await TestSupport.RunGit(setup.UpstreamBare, "rev-parse", workBranch); Assert.Equal(oldHead, tip.Trim()); - Assert.Single(handler.Requests); - Assert.Contains("/compare/", handler.Requests[0].RequestUri!.ToString(), StringComparison.Ordinal); + Assert.Equal(2, handler.Requests.Count); + Assert.Contains("/branches/", handler.Requests[0].RequestUri!.ToString(), StringComparison.Ordinal); + Assert.Contains("/compare/", handler.Requests[1].RequestUri!.ToString(), StringComparison.Ordinal); } [Fact] diff --git a/tests/CodeyBox.Tests/GitHubUpstreamTypedConflictTests.cs b/tests/CodeyBox.Tests/GitHubUpstreamTypedConflictTests.cs index 0a67a993d..f1da7c5e7 100644 --- a/tests/CodeyBox.Tests/GitHubUpstreamTypedConflictTests.cs +++ b/tests/CodeyBox.Tests/GitHubUpstreamTypedConflictTests.cs @@ -215,6 +215,15 @@ public async Task RebaseDivergenceWithoutLeaseRecord_ParksDistinctFromConflict() // proven to be CodeyBox's own history, so the trailer proof is // consulted and the push still parks distinctly. handler.Enqueue(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent( + JsonSerializer.Serialize(new + { + commit = new { sha = new string('c', 40) }, + }), + Encoding.UTF8, "application/json"), + }); + handler.Enqueue(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent( JsonSerializer.Serialize(new @@ -230,7 +239,8 @@ public async Task RebaseDivergenceWithoutLeaseRecord_ParksDistinctFromConflict() Assert.False(UpstreamPushReconcileConflictException.TryFindIn(ex, out _)); Assert.DoesNotContain(token, ex.Message); Assert.DoesNotContain(token, ex.Branch); - var compareCall = Assert.Single(handler.Requests); + Assert.Equal(2, handler.Requests.Count); + var compareCall = handler.Requests[1]; Assert.Contains("/compare/", compareCall.RequestUri!.ToString(), StringComparison.Ordinal); // The stale remote tip is untouched. var (_, content, _) = await TestSupport.RunGit(upstreamBare, "show", $"{workBranch}:conflict.txt"); diff --git a/tests/CodeyBox.Tests/UpstreamRedriveRecoveryTests.cs b/tests/CodeyBox.Tests/UpstreamRedriveRecoveryTests.cs index 12aa9eda7..d100bf4a5 100644 --- a/tests/CodeyBox.Tests/UpstreamRedriveRecoveryTests.cs +++ b/tests/CodeyBox.Tests/UpstreamRedriveRecoveryTests.cs @@ -90,6 +90,137 @@ public async Task RedriveAsync_ForeignTip_RefusedUnlessConfirmOwnership() Assert.Equal(headSha, stored.LastPushedWorkBranchSha); } + [Fact] + public async Task RedriveAsync_MergeFromMainPlusTraileredWork_AcceptedWithoutConfirmation() + { + // Regression: branches that merged main and carry trailered work in + // every writer's own subset were refused because the proof demanded + // every configured key on every commit. + using var fixture = CreateFixture(); + var item = NewItem("codeybox/legacy-item", fixture.Project.Id); + item = item.With(WorkItemState.Failed, "upstream push diverged", failureKind: WorkItemFailureKinds.UpstreamBlocked); + await fixture.Store.CreateAsync(item, CancellationToken.None); + await EnsureHostBranchAsync(fixture, item); + var baseTip = new string('1', 40); + var headSha = new string('e', 40); + fixture.Remote.BranchTips["main"] = baseTip; + fixture.Remote.OpenPullRequests.Add(new UpstreamPullRequest + { + Number = 612, + Url = "https://github.com/o/r/pull/612", + HeadBranch = "codeybox/legacy-item", + HeadSha = headSha, + BaseBranch = "main", + HasMergeConflict = true, + }); + var mergeSha = new string('2', 40); + var agentSha = new string('3', 40); + var mechanicalSha = new string('4', 40); + fixture.Remote.BranchCommits = + [ + // Orchestrator merge commit: WorkItem + Agent + Co-Authored-By. + new OwnedBranchCommit( + mergeSha, + "codeybox: merge codeybox/legacy-item\n\n" + + "CodeyBox-WorkItem: 6f2c9a1e3b4d5f6a7b8c9d0e1f2a3b4c\n" + + "CodeyBox-Agent: test-agent\n" + + "Co-Authored-By: CodeyBox "), + // Agent work commit: Prompt-Revision + Co-Authored-By only (no WorkItem). + new OwnedBranchCommit( + agentSha, + "agent work\n\nDo the thing.\n\n" + + "CodeyBox-Prompt-Revision: 7\n" + + "Co-Authored-By: CodeyBox "), + // Mechanical fixer commit: Mechanical-Fixer only. + new OwnedBranchCommit( + mechanicalSha, + "normalize whitespace\n\nCodeyBox-Mechanical-Fixer: whitespace"), + ]; + + var result = await fixture.Redrive.RedriveAsync(item.Id, CancellationToken.None); + + Assert.True(result.Success); + // The range was pinned to the current base tip, not the branch name. + Assert.Equal(baseTip, fixture.Remote.LastBaseRevision); + } + + [Fact] + public async Task RedriveAsync_UntraileredCommitOffBase_RefusedNamingSha() + { + using var fixture = CreateFixture(); + var item = NewItem("codeybox/legacy-item", fixture.Project.Id); + item = item.With(WorkItemState.Failed, "upstream push diverged", failureKind: WorkItemFailureKinds.UpstreamBlocked); + await fixture.Store.CreateAsync(item, CancellationToken.None); + var baseTip = new string('1', 40); + var headSha = new string('f', 40); + fixture.Remote.BranchTips["main"] = baseTip; + fixture.Remote.OpenPullRequests.Add(new UpstreamPullRequest + { + Number = 612, + Url = "https://github.com/o/r/pull/612", + HeadBranch = "codeybox/legacy-item", + HeadSha = headSha, + BaseBranch = "main", + HasMergeConflict = true, + }); + var ownedSha = new string('a', 40); + var foreignSha = new string('b', 40); + fixture.Remote.BranchCommits = + [ + new OwnedBranchCommit( + ownedSha, + "agent work\n\nCodeyBox-Prompt-Revision: 7\nCo-Authored-By: CodeyBox "), + new OwnedBranchCommit(foreignSha, "operator hotfix without trailers"), + ]; + + var refused = await fixture.Redrive.RedriveAsync(item.Id, CancellationToken.None); + + Assert.False(refused.Success); + Assert.Contains(foreignSha, refused.Error); + Assert.DoesNotContain(ownedSha, refused.Error); + Assert.Contains("confirmOwnership", refused.Error); + } + + [Fact] + public async Task RedriveAsync_TrailersDisabledByConfig_RequiresConfirmOwnership() + { + using var fixture = CreateFixture(new CommitAttribution( + includeCoAuthoredBy: false, includeCodeyBoxTrailers: false, includePullRequestFooter: true)); + var item = NewItem("codeybox/legacy-item", fixture.Project.Id); + item = item.With(WorkItemState.Failed, "upstream push diverged", failureKind: WorkItemFailureKinds.UpstreamBlocked); + await fixture.Store.CreateAsync(item, CancellationToken.None); + await EnsureHostBranchAsync(fixture, item); + var baseTip = new string('1', 40); + var headSha = new string('e', 40); + fixture.Remote.BranchTips["main"] = baseTip; + fixture.Remote.OpenPullRequests.Add(new UpstreamPullRequest + { + Number = 612, + Url = "https://github.com/o/r/pull/612", + HeadBranch = "codeybox/legacy-item", + HeadSha = headSha, + BaseBranch = "main", + HasMergeConflict = true, + }); + fixture.Remote.BranchCommits = + [ + new OwnedBranchCommit( + new string('a', 40), + "agent work\n\n" + + "CodeyBox-WorkItem: 6f2c9a1e3b4d5f6a7b8c9d0e1f2a3b4c\n" + + "Co-Authored-By: CodeyBox "), + ]; + + var refused = await fixture.Redrive.RedriveAsync(item.Id, CancellationToken.None); + + Assert.False(refused.Success); + Assert.Contains("confirmOwnership", refused.Error); + + var confirmed = await fixture.Redrive.RedriveAsync(item.Id, confirmOwnership: true, CancellationToken.None); + + Assert.True(confirmed.Success); + } + [Fact] public async Task RedriveAsync_NoPrAnywhere_PushesAloneWithoutRecordingPr() { @@ -194,7 +325,7 @@ private static async Task EnsureHostBranchAsync(RecoveryFixture fixture, WorkIte } } - private RecoveryFixture CreateFixture() + private RecoveryFixture CreateFixture(CommitAttribution? attribution = null) { var workspace = Path.Combine(_workspace, "fx-" + Guid.NewGuid().ToString("N")[..8]); Directory.CreateDirectory(workspace); @@ -216,9 +347,19 @@ private RecoveryFixture CreateFixture() new StubTaskQueue(), git, NullLogger.Instance); + CommitAttributionPolicy? policy = attribution is null + ? null + : new CommitAttributionPolicy(new CommitAttributionSnapshot( + new CommitAttributionOptions + { + CoAuthoredBy = attribution.IncludeCoAuthoredBy, + CodeyBoxTrailers = attribution.IncludeCodeyBoxTrailers, + PullRequestFooter = attribution.IncludePullRequestFooter, + })); var service = new UpstreamRedriveService( store, projects, new StubRemoteFactory(remote), retrier, - NullLogger.Instance); + NullLogger.Instance, + attributionPolicy: policy); return new RecoveryFixture(workspace, store, project, remote, git, service); } @@ -251,7 +392,10 @@ private sealed class StubRecoveryUpstreamRemote : IUpstreamRemote { public List OpenPullRequests { get; } = new(); public IReadOnlyList? BranchCommitMessages { get; set; } + public IReadOnlyList? BranchCommits { get; set; } public string? BranchHeadSha { get; set; } + public Dictionary BranchTips { get; } = new(StringComparer.Ordinal); + public string? LastBaseRevision { get; private set; } public string Name => "stub-recovery-upstream"; public Task PushAsync(string repositoryId, string branch, CancellationToken ct = default) @@ -268,11 +412,21 @@ public Task> ListOpenPullRequestsAsync(string OpenPullRequests.Where(p => p.HeadBranch.StartsWith(branchPrefix, StringComparison.Ordinal)).ToList()); public Task GetBranchHeadShaAsync(string branch, CancellationToken ct = default) - => Task.FromResult(BranchHeadSha); + => Task.FromResult(BranchTips.TryGetValue(branch, out var sha) ? sha : BranchHeadSha); public Task?> ListBranchCommitMessagesAsync( string baseBranch, string head, int maxCommits, CancellationToken ct = default) => Task.FromResult(BranchCommitMessages); + + public Task?> ListBranchCommitsAsync( + string baseRevision, string head, int maxCommits, CancellationToken ct = default) + { + LastBaseRevision = baseRevision; + if (BranchCommits is not null) + return Task.FromResult?>(BranchCommits); + return Task.FromResult?>(BranchCommitMessages? + .Select(m => new OwnedBranchCommit(string.Empty, m)).ToList()); + } } private sealed class StubTaskQueue : ITaskQueue