From 21f9acd81a8276ff0ac1c70aef8dd868d1b1c81a Mon Sep 17 00:00:00 2001 From: Adam Frisby Date: Mon, 12 Oct 2026 06:41:27 +0000 Subject: [PATCH 1/2] Record every work-branch push; accept recorded/trailered tips before refusing a moved remote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A re-drive push landed on the remote but never updated the item's last-pushed sha, so the next attempt compared the remote at CodeyBox's own new tip against the stale record and misreported it as third-party work (d9951691). Every successful work-branch push is now recorded immediately (latest sha plus bounded history) on all paths — normal push, re-drive retry, lease rewrite, conflict-rework recomposition — including when later PR/merge steps fail, with best-effort confirmation when a completion fails after its push. Before a moved remote is treated as third-party, the push path accepts a tip matching any recorded push or whose exclusive commits all carry this instance's CodeyBox trailers; genuinely foreign tips are still refused without clobbering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> 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/IUpstreamRemote.cs | 29 ++- src/CodeyBox.Core/UpstreamBranchOwnership.cs | 13 ++ src/CodeyBox.Core/WorkItem.cs | 11 + src/CodeyBox.Core/WorkItemPushHistory.cs | 92 ++++++++ .../PipelineRunner.State.cs | 11 +- .../PipelineRunner.UpstreamPush.cs | 184 ++++++++++++++-- .../SqliteWorkItemStore.cs | 31 ++- .../UpstreamRedriveService.cs | 13 +- .../GitHubUpstreamRemote.cs | 68 +++++- .../GitGenericUpstreamRemote.cs | 8 + src/CodeyBox.Upstream/NoopUpstreamRemote.cs | 6 + .../GitHubOwnedBranchPushTests.cs | 107 +++++++++- .../UpstreamLeasePipelineTests.cs | 199 ++++++++++++++++++ .../WorkItemPushHistoryTests.cs | 148 +++++++++++++ 17 files changed, 899 insertions(+), 30 deletions(-) create mode 100644 src/CodeyBox.Core/WorkItemPushHistory.cs create mode 100644 tests/CodeyBox.Tests/WorkItemPushHistoryTests.cs diff --git a/docs/concepts/work-items.md b/docs/concepts/work-items.md index f13f387ae..1645971a1 100644 --- a/docs/concepts/work-items.md +++ b/docs/concepts/work-items.md @@ -180,6 +180,8 @@ POST /workitems/{id}/redrive-upstream 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. +Every successful work-branch push is recorded on the item immediately (latest sha plus a bounded history, `CodeyBox:UpstreamBranchOwnership:MaxPushHistoryEntries`), on all paths — normal push, re-drive retry, lease-guarded rewrite, and conflict-rework recomposition — including when the later PR/merge steps fail. Before a moved remote is treated as third-party work, the push path also accepts a tip equal to any recorded push, or a tip whose exclusive commits all carry this instance's CodeyBox trailers. A genuinely foreign tip (unrecorded sha, unmarked commits) is still refused and the branch is left untouched. + ### AbandonedAfterRecoveryAttempts When the recovery loop has retried an item more than `CodeyBox:DeadWorker:MaxRecoveryAttempts` times (default 10) without it ever completing the recovered phase, the item is transitioned to `AbandonedAfterRecoveryAttempts` with a descriptive `lastError`. Use `POST /workitems/{id}/retry` to resume manually after investigating the root cause. diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 0479f8f4e..6ca26e3fa 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -629,7 +629,8 @@ Proves a stale CodeyBox-owned branch tip is CodeyBox's own history when no prior ```json "UpstreamBranchOwnership": { "RequiredTrailerKeys": ["Co-Authored-By", "CodeyBox-WorkItem", "CodeyBox-Agent", "CodeyBox-Prompt-Revision", "CodeyBox-Mechanical-Fixer"], - "MaxCommitsToVerify": 100 + "MaxCommitsToVerify": 100, + "MaxPushHistoryEntries": 20 } ``` @@ -637,6 +638,7 @@ Proves a stale CodeyBox-owned branch tip is CodeyBox's own history when no prior |-----|---------|-------------| | `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. | +| `MaxPushHistoryEntries` | `20` | Upper bound on the per-item history of pushed work-branch tips. Every successful push is recorded (latest sha plus this bounded history); a diverged remote tip matching ANY retained entry is accepted as CodeyBox's own history without consulting commit trailers — the only proof available when trailers are disabled or the exclusive range is unverifiable. Oldest entries drop first; values below 1 behave as 1. | ## `WorkerPoolHealthWatchdog` diff --git a/src/CodeyBox.Api/Program.cs b/src/CodeyBox.Api/Program.cs index d0e8e0942..68e98d9e3 100644 --- a/src/CodeyBox.Api/Program.cs +++ b/src/CodeyBox.Api/Program.cs @@ -4260,7 +4260,8 @@ static Func DotnetTestRunOptionsAccessor(IServiceProvider sp) providerTransientCorrelation: sp.GetRequiredService(), baseBuildVerifier: sp.GetRequiredService(), baseBrokenConditions: sp.GetRequiredService(), - attributionPolicy: sp.GetRequiredService())); + attributionPolicy: sp.GetRequiredService(), + branchOwnership: sp.GetRequiredService())); builder.Services.AddSingleton(sp => sp.GetRequiredService()); // Isolated base-branch fix-item spawner for NotDiffAttributable audit test // failures. Constructed lazily from the store/queue plus the hot-reloadable diff --git a/src/CodeyBox.Core/IUpstreamRemote.cs b/src/CodeyBox.Core/IUpstreamRemote.cs index 7c5e35f71..0c7e97746 100644 --- a/src/CodeyBox.Core/IUpstreamRemote.cs +++ b/src/CodeyBox.Core/IUpstreamRemote.cs @@ -11,6 +11,16 @@ public interface IUpstreamRemote /// Stable identifier for diagnostics ("noop", "github", "git-generic"). string Name { get; } + /// + /// True when pushes the item's work branch to + /// the upstream (the common case). False for upstreams whose completion + /// pushes a different ref (git-generic lands the base branch) or nothing + /// at all (noop). The orchestrator records a pushed work-branch tip only + /// when this is true, so a recorded sha is always evidence of a real + /// work-branch push — never a guess. + /// + bool PushesWorkBranch => true; + /// /// Pushes the named ref from the host bare repo to the upstream. The /// repository identifier is opaque and must be understood by the host @@ -757,12 +767,27 @@ public sealed record UpstreamCompletionRequest /// as LastPushedWorkBranchSha), or an operator-authorized sha /// observed on the remote during a re-drive. When the remote tip has /// diverged from the new head, the implementation rewrites the owned - /// branch only under a --force-with-lease bound to this sha; a - /// mismatch means a third party pushed and the rewrite is refused. + /// branch only under a --force-with-lease bound to this sha. A + /// moved remote is first checked against + /// and the commit-trailer ownership proof before it is refused as + /// third-party work; only a foreign or unverifiable tip is refused. /// Null when CodeyBox has no record (first push, or legacy rows): a /// diverged remote is then reported distinctly rather than rewritten. /// public string? ExpectedRemoteHeadSha { get; init; } + + /// + /// Every work-branch tip sha CodeyBox has recorded pushing for this item + /// (bounded history, newest last; see + /// WorkItem.PushedWorkBranchShaHistory). When the remote tip + /// matches the lease proceeds as + /// usual; when it has moved, a tip equal to ANY entry here is still + /// accepted as CodeyBox's own history before the implementation falls + /// back to the commit-trailer ownership proof. Null or empty when no + /// history was recorded (first push, legacy rows): only the trailer + /// proof can then authorize a rewrite. + /// + public IReadOnlyList? KnownWorkBranchShas { get; init; } } /// Result of . diff --git a/src/CodeyBox.Core/UpstreamBranchOwnership.cs b/src/CodeyBox.Core/UpstreamBranchOwnership.cs index b17eb983e..139a2cd97 100644 --- a/src/CodeyBox.Core/UpstreamBranchOwnership.cs +++ b/src/CodeyBox.Core/UpstreamBranchOwnership.cs @@ -37,10 +37,23 @@ public sealed class UpstreamBranchOwnershipOptions /// public int MaxCommitsToVerify { get; set; } = 100; + /// + /// Upper bound on the per-item pushed-sha history retained for the lease + /// fallback (see ). A diverged remote + /// tip matching any retained entry is accepted as CodeyBox's own history + /// without consulting commit trailers — the only proof available when + /// trailers are disabled by configuration or the exclusive range is + /// unverifiable. Enforced at record time (oldest entries drop first), so + /// the stored history never grows without bound. Default 20; must be at + /// least 1 (smaller values behave as 1). + /// + public int MaxPushHistoryEntries { get; set; } = 20; + public UpstreamBranchOwnershipOptions Clone() => new() { RequiredTrailerKeys = (string[])RequiredTrailerKeys.Clone(), MaxCommitsToVerify = MaxCommitsToVerify, + MaxPushHistoryEntries = MaxPushHistoryEntries, }; } diff --git a/src/CodeyBox.Core/WorkItem.cs b/src/CodeyBox.Core/WorkItem.cs index 2e44fab70..2de2826f9 100644 --- a/src/CodeyBox.Core/WorkItem.cs +++ b/src/CodeyBox.Core/WorkItem.cs @@ -735,6 +735,17 @@ AgentTurnResumeCheckpoint is not null /// public string? LastPushedWorkBranchSha { get; init; } + /// + /// Bounded history of work-branch tip shas CodeyBox has pushed to the + /// upstream for this item, oldest first (see + /// ). + /// is always the newest entry. A later retry accepts a diverged remote + /// tip that equals ANY entry here — not just the latest — before falling + /// back to the commit-trailer ownership proof. Empty for legacy rows + /// predating push tracking. + /// + public IReadOnlyList PushedWorkBranchShaHistory { get; init; } = []; + /// /// The release this work item belongs to. When set, the orchestrator targets the /// release branch instead of the project's default base branch, and the release state diff --git a/src/CodeyBox.Core/WorkItemPushHistory.cs b/src/CodeyBox.Core/WorkItemPushHistory.cs new file mode 100644 index 000000000..7f1c5d58e --- /dev/null +++ b/src/CodeyBox.Core/WorkItemPushHistory.cs @@ -0,0 +1,92 @@ +namespace CodeyBox.Core; + +/// +/// Pure bookkeeping for the bounded per-item history of pushed work-branch +/// tip shas. Every successful work-branch push (plain or lease-guarded, on +/// any path) is recorded through so a later retry +/// can prove a diverged remote tip is CodeyBox's own history even when the +/// latest record went stale (crash between push and persist, legacy rows). +/// The history also backs the lease check when commit trailers are disabled +/// by configuration and no trailer proof is possible. +/// +public static class WorkItemPushHistory +{ + /// + /// Default cap on retained entries when no configured bound is + /// available. The configured + /// + /// wins whenever options are present. + /// + public const int DefaultMaxEntries = 20; + + /// + /// Guards the persisted-state sink: only plausible commit ids (hex, + /// 40–64 chars after trimming) qualify as pushed shas. Forge-reported + /// tips are untrusted input, and lease decisions branch on the recorded + /// history, so garbage must never enter it. + /// + public static bool IsPlausibleSha(string? sha) + { + if (string.IsNullOrWhiteSpace(sha)) + return false; + var trimmed = sha.Trim(); + if (trimmed.Length is < 40 or > 64) + return false; + foreach (var c in trimmed) + { + if (!Uri.IsHexDigit(c)) + return false; + } + return true; + } + + /// + /// Returns with recorded + /// as the latest push: + /// becomes and the sha is appended to + /// (moved to the end + /// when already present), truncated to the newest + /// entries. Blank or implausible shas + /// (see ) leave the item unchanged — a guess + /// is never persisted. Pure: the input is never mutated. + /// + public static WorkItem RecordPush(WorkItem item, string? sha, int maxEntries = DefaultMaxEntries) + { + ArgumentNullException.ThrowIfNull(item); + if (!IsPlausibleSha(sha)) + return item; + var bound = Math.Max(1, maxEntries); + var trimmed = sha!.Trim(); + var history = item.PushedWorkBranchShaHistory + .Where(IsPlausibleSha) + .Select(s => s.Trim()) + .Where(s => !string.Equals(s, trimmed, StringComparison.OrdinalIgnoreCase)) + .ToList(); + history.Add(trimmed); + while (history.Count > bound) + history.RemoveAt(0); + return item with + { + LastPushedWorkBranchSha = trimmed, + PushedWorkBranchShaHistory = history, + }; + } + + /// + /// True when equals any recorded push for + /// (latest or history), compared ordinally + /// ignoring case. Used by the lease guard before treating a moved remote + /// as third-party work. + /// + public static bool Contains(WorkItem item, string? sha) + { + ArgumentNullException.ThrowIfNull(item); + if (string.IsNullOrWhiteSpace(sha)) + return false; + var trimmed = sha.Trim(); + if (string.Equals(item.LastPushedWorkBranchSha?.Trim(), trimmed, StringComparison.OrdinalIgnoreCase)) + return true; + return item.PushedWorkBranchShaHistory.Any(s => + string.Equals(s?.Trim(), trimmed, StringComparison.OrdinalIgnoreCase)); + } +} diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.State.cs b/src/CodeyBox.Orchestrator/PipelineRunner.State.cs index cd8e680df..90c2f6d63 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.State.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.State.cs @@ -96,6 +96,9 @@ public sealed partial class PipelineRunner // a private snapshot (unchanging defaults) when DI does not supply one. private readonly PipelineTuningSnapshot _pipelineTuning; private readonly CommitAttributionPolicy _attribution; + // Hot-reloadable work-branch ownership knobs. Null when unwired (tests / + // legacy embedders): readers fall back to shipped defaults. + private readonly UpstreamBranchOwnershipSnapshot? _branchOwnership; // Optional stale-base remediation router. When wired AND enabled, a // stale-base conflict discovered at upstream-push time re-dispatches the // item into conflict-rework instead of parking. Null preserves the @@ -615,7 +618,12 @@ public PipelineRunner( // condition, the fix-item dedupe, and the dispatcher hold. BaseBuildVerifier? baseBuildVerifier = null, BaseBrokenConditionTracker? baseBrokenConditions = null, - CommitAttributionPolicy? attributionPolicy = null) + CommitAttributionPolicy? attributionPolicy = null, + // Hot-reloadable work-branch ownership knobs (trailer proof, push + // history bound). Optional: when null the shipped defaults apply, so + // tests and embeddings that don't wire the snapshot keep the + // previous behaviour. + UpstreamBranchOwnershipSnapshot? branchOwnership = null) { _sandboxes = sandboxes; _gitHost = gitHost; @@ -740,6 +748,7 @@ public PipelineRunner( _incrementalRebase = incrementalRebase; _pipelineTuning = pipelineTuning ?? new PipelineTuningSnapshot(new PipelineTuningOptions()); _attribution = attributionPolicy ?? new CommitAttributionPolicy(new CommitAttributionSnapshot(new CommitAttributionOptions())); + _branchOwnership = branchOwnership; _staleBaseReworkRouter = staleBaseReworkRouter; _flakeEscalation = flakeEscalation; _flakeEscalationOptions = flakeEscalationOptions; diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.UpstreamPush.cs b/src/CodeyBox.Orchestrator/PipelineRunner.UpstreamPush.cs index 4575eac45..8b18d3116 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.UpstreamPush.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.UpstreamPush.cs @@ -168,6 +168,11 @@ private async Task RunUpstreamPushPhaseAsync( // the operator instead of being rewritten blind. if (request.ExpectedRemoteHeadSha is null && !string.IsNullOrWhiteSpace(stored?.LastPushedWorkBranchSha)) request = request with { ExpectedRemoteHeadSha = stored.LastPushedWorkBranchSha }; + // Bounded recorded-push history for the same lease decision: + // a remote tip matching ANY entry is still CodeyBox's own + // even when the latest record went stale. + if (stored is not null && stored.PushedWorkBranchShaHistory.Count > 0) + request = request with { KnownWorkBranchShas = stored.PushedWorkBranchShaHistory }; } // Pre-merge CI gate. The forge's textual `mergeable` flag does not @@ -321,6 +326,19 @@ _preMergeVerifier is not null && if (outcome.Notes is not null) _log.LogInformation("Upstream notes: {Notes}", outcome.Notes); + // Record every successful work-branch push immediately — + // before race recovery (which recomposes the branch and + // may itself fail), PR handling, or the next loop + // iteration. A push whose completion later fails must + // already be on record, or the next attempt sees the + // remote at this sha, compares it with the stale record, + // and misreports CodeyBox's own push as third-party work. + // The refreshed request also carries the new tip as the + // lease base for any further attempt in this run. + request = await RecordUpstreamPushAsync( + item.Id, upstream, repoId, workBranch, outcome.PushedWorkBranchSha, + outcome.BranchPushed && !outcome.Skipped, request, ct); + lastIterationRaced = outcome.AutoMergeRaced; if (outcome.AutoMergeRaced) { @@ -428,12 +446,14 @@ await _webhooks.PublishAsync(new WebhookEvent if (outcome.PullRequestNumber is not null || outcome.PullRequestUrl is not null) { var racedPersist = await _store.GetAsync(item.Id, ct) ?? item; - await _store.UpdateAsync(racedPersist with - { - MergedPrNumber = outcome.PullRequestNumber ?? racedPersist.MergedPrNumber, - MergedPrUrl = outcome.PullRequestUrl ?? racedPersist.MergedPrUrl, - LastPushedWorkBranchSha = outcome.PushedWorkBranchSha ?? racedPersist.LastPushedWorkBranchSha, - }, ct); + await _store.UpdateAsync(WorkItemPushHistory.RecordPush( + racedPersist with + { + MergedPrNumber = outcome.PullRequestNumber ?? racedPersist.MergedPrNumber, + MergedPrUrl = outcome.PullRequestUrl ?? racedPersist.MergedPrUrl, + }, + outcome.PushedWorkBranchSha, + PushHistoryBound), ct); } continue; } @@ -497,6 +517,18 @@ await _store.UpdateAsync(racedPersist with } _log.LogWarning("Upstream complete attempt {Attempt} failed: {Error}", attempt, ex.Message); + // The push may still have landed even though the + // completion failed (a PR/merge step threw after the + // branch push, or the push result was ambiguous). + // Confirm and record it BEFORE retrying/parking: + // otherwise the next attempt sees the remote at that sha, + // compares it with the stale record, and misreports + // CodeyBox's own push as third-party work. This runs only + // on the generic path: the typed terminals above + // (lease mismatch, diverged history, reconcile conflict) + // are thrown INSTEAD of pushing, so their contracts + // guarantee nothing landed and no confirm read is needed. + request = await ConfirmUpstreamPushAsync(item.Id, upstream, repoId, workBranch, request, ct); if (attempt < _opts.UpstreamPushMaxAttempts) await Task.Delay(_opts.UpstreamPushBackoff, ct); else @@ -569,13 +601,15 @@ completed.PullRequestUrl is not null || completed.PushedWorkBranchSha is not null) { var preMergePersist = await _store.GetAsync(item.Id, ct) ?? item; - await _store.UpdateAsync(preMergePersist with - { - MergeSha = completed.MergedSha ?? preMergePersist.MergeSha, - MergedPrNumber = completed.PullRequestNumber ?? preMergePersist.MergedPrNumber, - MergedPrUrl = completed.PullRequestUrl ?? preMergePersist.MergedPrUrl, - LastPushedWorkBranchSha = completed.PushedWorkBranchSha ?? preMergePersist.LastPushedWorkBranchSha, - }, ct); + await _store.UpdateAsync(WorkItemPushHistory.RecordPush( + preMergePersist with + { + MergeSha = completed.MergedSha ?? preMergePersist.MergeSha, + MergedPrNumber = completed.PullRequestNumber ?? preMergePersist.MergedPrNumber, + MergedPrUrl = completed.PullRequestUrl ?? preMergePersist.MergedPrUrl, + }, + completed.PushedWorkBranchSha, + PushHistoryBound), ct); } // Verified-delivery gate (sig-pr422-delivery-reconciliation): @@ -830,4 +864,128 @@ private readonly record struct ConflictReworkResult( string? FailureKind = null, AgentKind? Agent = null); + /// + /// Configured bound on retained pushed-sha history entries, read live so + /// hot-reload edits apply to the next push without a restart. Falls back + /// to the shipped default when no ownership snapshot is wired. + /// + private int PushHistoryBound => + _branchOwnership?.Current.MaxPushHistoryEntries ?? WorkItemPushHistory.DefaultMaxEntries; + + /// + /// Records a successful work-branch push on the item (latest sha plus + /// bounded history) and refreshes the in-flight request's lease base, so + /// a later attempt in this run — or the next run — proves the remote tip + /// is CodeyBox's own instead of misreporting it as third-party work. + /// is the tip the remote reports pushing; + /// when it is blank but confirms a + /// work-branch push landed, the local tip is resolved best-effort + /// (covers legacy hosts and forge remotes that track no tip). The sha + /// must look like a real commit id (hex, 40–64 chars) — anything else is + /// untrusted forge output and stays unrecorded. Never throws except on + /// cancellation: every I/O failure keeps the previous record and request. + /// + private async Task RecordUpstreamPushAsync( + WorkItemId itemId, + IUpstreamRemote upstream, + string repoId, + string workBranch, + string? reportedSha, + bool branchPushed, + UpstreamCompletionRequest request, + CancellationToken ct) + { + var sha = reportedSha?.Trim(); + if (string.IsNullOrWhiteSpace(sha) && branchPushed && upstream.PushesWorkBranch) + { + try + { + sha = (await _gitHost.ResolveCommitAsync(repoId, workBranch, ct).ConfigureAwait(false)).Trim(); + } + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) + { + _log.LogDebug(ex, "Could not resolve pushed tip of {WorkBranch}; push stays unrecorded", workBranch); + return request; + } + } + if (string.IsNullOrWhiteSpace(sha) || !WorkItemPushHistory.IsPlausibleSha(sha)) + { + _log.LogDebug("Refusing to record implausible pushed sha for {WorkBranch}; push stays unrecorded", workBranch); + return request; + } + WorkItem? stored; + try + { + stored = await _store.GetAsync(itemId, ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) + { + _log.LogDebug(ex, "Could not read work item {WorkItemId} to record pushed sha", itemId); + return request; + } + if (stored is null) + return request; + var updated = WorkItemPushHistory.RecordPush(stored, sha, PushHistoryBound); + try + { + await _store.UpdateAsync(updated, ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) + { + _log.LogDebug(ex, "Could not persist pushed sha for work item {WorkItemId}", itemId); + return request; + } + _log.LogInformation("Recorded pushed work-branch tip {Sha} for work item {WorkItemId}", sha, itemId); + return request with + { + ExpectedRemoteHeadSha = updated.LastPushedWorkBranchSha, + KnownWorkBranchShas = updated.PushedWorkBranchShaHistory, + }; + } + + /// + /// Best-effort confirmation for a completion that FAILED after its push + /// step: when the upstream tip now equals the local tip CodeyBox + /// attempted to push, that tip demonstrably reached the remote (either + /// by this attempt or an identical earlier one — both CodeyBox's own) and + /// is recorded before the failure is classified. Anything unresolvable + /// or unequal records nothing: a guess is never persisted. + /// + private async Task ConfirmUpstreamPushAsync( + WorkItemId itemId, + IUpstreamRemote upstream, + string repoId, + string workBranch, + UpstreamCompletionRequest request, + CancellationToken ct) + { + if (!upstream.PushesWorkBranch) + return request; + string? remoteTip; + string? localTip; + try + { + remoteTip = await upstream.GetBranchHeadShaAsync(workBranch, ct).ConfigureAwait(false); + localTip = await _gitHost.ResolveCommitAsync(repoId, workBranch, ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) + { + // A timeout here (TaskCanceledException without a requested + // shutdown) must not surface as a cancellation: swallowing it + // lets the in-loop retry classify the ORIGINAL failure instead + // of parking the item as interrupted. + _log.LogDebug(ex, "Could not confirm pushed tip of {WorkBranch} after failed completion", workBranch); + return request; + } + if (string.IsNullOrWhiteSpace(remoteTip) || string.IsNullOrWhiteSpace(localTip)) + return request; + if (!string.Equals(remoteTip.Trim(), localTip.Trim(), StringComparison.OrdinalIgnoreCase)) + return request; + _log.LogInformation( + "Upstream tip of {WorkBranch} equals the attempted local tip after a failed completion; recording {Sha}", + workBranch, remoteTip.Trim()); + return await RecordUpstreamPushAsync( + itemId, upstream, repoId, workBranch, remoteTip.Trim(), branchPushed: true, request, ct).ConfigureAwait(false); + } + } diff --git a/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs b/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs index 470bbe956..03bfa2ba8 100644 --- a/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs +++ b/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs @@ -221,6 +221,10 @@ CREATE INDEX IF NOT EXISTS idx_work_item_audit_progress_attempt // tip is its own previous push before rewriting the owned branch // under --force-with-lease. RunMigration("ALTER TABLE work_items ADD COLUMN last_pushed_work_branch_sha TEXT;"); + // Additive migration: bounded history of work-branch tips CodeyBox + // pushed, so the lease guard can accept any recorded tip (not just + // the latest) before treating a moved remote as third-party work. + RunMigration("ALTER TABLE work_items ADD COLUMN pushed_work_branch_sha_history TEXT;"); // Additive migration: minimum quality-score floor for routing. // Default 95 preserves existing semantics (frontier-adjacent fallback allowed). RunMigration("ALTER TABLE work_items ADD COLUMN min_model_score INTEGER NOT NULL DEFAULT 95;"); @@ -1451,7 +1455,7 @@ INSERT INTO work_items (id, project_id, title, prompt, base_branch, work_branch, work_timeout_ticks, work_timeout_override_ticks, merge_timeout_ticks, push_upstream, state, created_at, updated_at, last_error, upstream_push_attempts, depends_on_json, agent_class_id, queue_position, stuck_retries, started_at, external_id, replay_of_work_item_id, merge_sha, - local_squash_sha, merged_pr_number, merged_pr_url, last_pushed_work_branch_sha, + local_squash_sha, merged_pr_number, merged_pr_url, last_pushed_work_branch_sha, pushed_work_branch_sha_history, min_model_score, cancellation_reason, recovery_attempts, recovery_attempt_source_state, consecutive_infra_recoveries, release_id, preempted_at, preempt_checkpoint, agent_turn_resume_checkpoint_json, agent_turn_recovery_lease_json, suspended_vm_name, suspended_at, agent_log_path, @@ -1472,7 +1476,7 @@ INSERT INTO work_items (id, project_id, title, prompt, base_branch, work_branch, initiator_json) VALUES ($id, $project_id, $title, $prompt, $base, $work, $agent, $agent_instance_id, $wt, $wto, $mt, $pu, $state, $ca, $ua, $err, $att, $deps, $class_id, $qpos, $sretries, $started_at, $external_id, $replay_of, $merge_sha, - $local_squash_sha, $merged_pr_number, $merged_pr_url, $last_pushed_work_branch_sha, + $local_squash_sha, $merged_pr_number, $merged_pr_url, $last_pushed_work_branch_sha, $pushed_work_branch_sha_history, $min_model_score, $cancellation_reason, $recovery_attempts, $recovery_attempt_source_state, $consecutive_infra_recoveries, $release_id, $preempted_at, $preempt_checkpoint, $agent_turn_resume_checkpoint, $agent_turn_recovery_lease, $suspended_vm_name, $suspended_at, $agent_log_path, @@ -1802,6 +1806,7 @@ THEN started_at merged_pr_number = $merged_pr_number, merged_pr_url = $merged_pr_url, last_pushed_work_branch_sha = $last_pushed_work_branch_sha, + pushed_work_branch_sha_history = $pushed_work_branch_sha_history, min_model_score = $min_model_score, cancellation_reason = $cancellation_reason, recovery_attempts = $recovery_attempts, @@ -1909,6 +1914,7 @@ UPDATE work_items SET merged_pr_number = $merged_pr_number, merged_pr_url = $merged_pr_url, last_pushed_work_branch_sha = $last_pushed_work_branch_sha, + pushed_work_branch_sha_history = $pushed_work_branch_sha_history, min_model_score = $min_model_score, cancellation_reason = $cancellation_reason, recovery_attempts = $recovery_attempts, @@ -2018,6 +2024,7 @@ UPDATE work_items SET merged_pr_number = $merged_pr_number, merged_pr_url = $merged_pr_url, last_pushed_work_branch_sha = $last_pushed_work_branch_sha, + pushed_work_branch_sha_history = $pushed_work_branch_sha_history, min_model_score = $min_model_score, cancellation_reason = $cancellation_reason, recovery_attempts = $recovery_attempts, @@ -2497,6 +2504,7 @@ UPDATE work_items SET merged_pr_number = $merged_pr_number, merged_pr_url = $merged_pr_url, last_pushed_work_branch_sha = $last_pushed_work_branch_sha, + pushed_work_branch_sha_history = $pushed_work_branch_sha_history, min_model_score = $min_model_score, cancellation_reason = $cancellation_reason, recovery_attempts = $recovery_attempts, @@ -2941,6 +2949,7 @@ UPDATE work_items SET merged_pr_number = $merged_pr_number, merged_pr_url = $merged_pr_url, last_pushed_work_branch_sha = $last_pushed_work_branch_sha, + pushed_work_branch_sha_history = $pushed_work_branch_sha_history, min_model_score = $min_model_score, cancellation_reason = $cancellation_reason, recovery_attempts = $recovery_attempts, @@ -4609,6 +4618,8 @@ private static void Bind(SqliteCommand cmd, WorkItem item) item.MergedPrNumber.HasValue ? (object)item.MergedPrNumber.Value : DBNull.Value); cmd.Parameters.AddWithValue("$merged_pr_url", (object?)item.MergedPrUrl ?? DBNull.Value); cmd.Parameters.AddWithValue("$last_pushed_work_branch_sha", (object?)item.LastPushedWorkBranchSha ?? DBNull.Value); + cmd.Parameters.AddWithValue("$pushed_work_branch_sha_history", + JsonSerializer.Serialize(item.PushedWorkBranchShaHistory)); cmd.Parameters.AddWithValue("$min_model_score", item.MinModelScore); cmd.Parameters.AddWithValue("$cancellation_reason", item.CancellationReason.HasValue ? (object)item.CancellationReason.Value.ToString() : DBNull.Value); @@ -4765,6 +4776,7 @@ private static readonly IReadOnlyDictionary EmptyKnobs MergedPrNumber = ReadNullableInt32(r, "merged_pr_number"), MergedPrUrl = ReadNullableString(r, "merged_pr_url"), LastPushedWorkBranchSha = ReadNullableString(r, "last_pushed_work_branch_sha"), + PushedWorkBranchShaHistory = ReadPushHistory(r), MinModelScore = ReadInt32OrDefault(r, "min_model_score", defaultValue: 95), CancellationReason = ReadCancellationReason(r), RecoveryAttempts = ReadInt32OrDefault(r, "recovery_attempts", defaultValue: 0), @@ -5046,6 +5058,21 @@ private static int ReadInt32OrDefault(SqliteDataReader r, string column, int def return r.IsDBNull(ord) ? null : r.GetString(ord); } + /// + /// Reads the bounded pushed-sha history. Legacy rows carry NULL and + /// corrupt JSON degrades to empty — either way the lease guard falls + /// back to the latest sha and the trailer proof instead of failing the + /// read. The column itself always exists: the additive migration adds it + /// when the store opens. + /// + private static IReadOnlyList ReadPushHistory(SqliteDataReader r) + { + var raw = ReadNullableString(r, "pushed_work_branch_sha_history"); + if (string.IsNullOrWhiteSpace(raw)) + return []; + return DeserializeStringList(raw); + } + private static AgentKind? ReadNullableAgentKind(SqliteDataReader r, string column) { var raw = ReadNullableString(r, column); diff --git a/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs b/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs index 7535cdfef..d6df1f457 100644 --- a/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs +++ b/src/CodeyBox.Orchestrator/UpstreamRedriveService.cs @@ -352,9 +352,18 @@ public async Task RedriveAsync( }; } if (leaseBase is not null - && !string.Equals(updated.LastPushedWorkBranchSha, leaseBase, StringComparison.OrdinalIgnoreCase)) + && !string.Equals(updated.LastPushedWorkBranchSha, leaseBase, StringComparison.OrdinalIgnoreCase) + && !WorkItemPushHistory.Contains(updated, leaseBase)) { - updated = updated with { LastPushedWorkBranchSha = leaseBase }; + // Record the operator-authorized tip as the lease base AND in the + // bounded push history: the retried run's push receipt extends + // this history, and the lease fallback accepts any entry — so + // the record survives even if a later push lands without being + // persisted. Implausible tips stay unrecorded (see + // WorkItemPushHistory); the retry then proceeds without a lease + // base rather than on poisoned evidence. + updated = WorkItemPushHistory.RecordPush( + updated, leaseBase, _ownership.Current.MaxPushHistoryEntries); } if (!ReferenceEquals(updated, fresh)) await _store.UpdateAsync(updated, ct); diff --git a/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs b/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs index 0662c8460..695fd2a9d 100644 --- a/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs +++ b/src/CodeyBox.Upstream.GitHub/GitHubUpstreamRemote.cs @@ -591,6 +591,31 @@ private static string NormalizeCommitSha(string? sha) return candidate; } + /// + /// True when equals any recorded CodeyBox + /// push in (ordinal, ignoring case and + /// surrounding whitespace). The scan is bounded: at most the first + /// MaxKnownShaScan entries are compared, so a hostile or + /// misconfigured caller cannot turn the lease decision into an + /// unbounded loop. Recorded history is already bounded at record time + /// (see ); this is defense in depth. + /// + private static bool IsKnownPushedSha(IReadOnlyList? knownShas, string remoteSha) + { + const int MaxKnownShaScan = 128; + if (knownShas is null || knownShas.Count == 0) + return false; + var count = 0; + foreach (var known in knownShas) + { + if (count++ >= MaxKnownShaScan) + break; + if (string.Equals(known?.Trim(), remoteSha, StringComparison.OrdinalIgnoreCase)) + return true; + } + return false; + } + /// /// Proves a diverged remote tip is CodeyBox's own history by listing the /// commits exclusive of the CURRENT base tip (<base tip>..<remote tip>) @@ -1168,7 +1193,20 @@ await _gitHost.PushToUpstreamAsync( var expected = request.ExpectedRemoteHeadSha?.Trim(); string leaseBase; - if (string.IsNullOrWhiteSpace(expected)) + if (IsKnownPushedSha(request.KnownWorkBranchShas, remoteSha)) + { + // Bounded-history fallback: the remote tip equals a sha CodeyBox + // recorded pushing for this item. The latest record may have gone + // stale (a later push landed but its completion failed before the + // record was persisted), yet this tip is provably CodeyBox's own — + // no trailer proof needed, and none is possible when trailers are + // disabled by configuration. + leaseBase = remoteSha; + _log.LogInformation( + "Work branch {WorkBranch} remote tip {Sha} matches a recorded CodeyBox push; rewriting under lease", + SanitizeForLog(request.WorkBranch), leaseBase); + } + else if (string.IsNullOrWhiteSpace(expected)) { // No recorded prior push to lease from: accept the remote tip as // CodeyBox's own history only when every commit exclusive of the @@ -1189,9 +1227,31 @@ await _gitHost.PushToUpstreamAsync( else { if (!string.Equals(expected, remoteSha, StringComparison.OrdinalIgnoreCase)) - throw await BuildLeaseMismatchWithActualAsync( - request, expected, remoteSha, upstreamEnv, ct).ConfigureAwait(false); - leaseBase = remoteSha; + { + // The remote moved past the last recorded push. It is still + // CodeyBox's own history when every commit exclusive of the + // current base tip carries this instance's trailers (the same + // ownership proof the no-record path uses) — e.g. a re-drive + // push that landed but was never recorded. Only a tip with + // foreign or unverifiable commits refuses as third-party work. + var unowned = await TryVerifyRemoteTipOwnedAsync(request.BaseBranch, remoteSha, ct).ConfigureAwait(false); + if (unowned is not null && unowned.Count == 0) + { + leaseBase = remoteSha; + _log.LogInformation( + "Work branch {WorkBranch} remote tip {Sha} moved past the last recorded push but every exclusive commit carries CodeyBox trailers; rewriting under lease", + SanitizeForLog(request.WorkBranch), leaseBase); + } + else + { + throw await BuildLeaseMismatchWithActualAsync( + request, expected, remoteSha, upstreamEnv, ct).ConfigureAwait(false); + } + } + else + { + leaseBase = remoteSha; + } } try diff --git a/src/CodeyBox.Upstream/GitGenericUpstreamRemote.cs b/src/CodeyBox.Upstream/GitGenericUpstreamRemote.cs index f84aff072..778bdd3fd 100644 --- a/src/CodeyBox.Upstream/GitGenericUpstreamRemote.cs +++ b/src/CodeyBox.Upstream/GitGenericUpstreamRemote.cs @@ -21,6 +21,14 @@ public GitGenericUpstreamRemote(IGitHost gitHost, GitGenericUpstreamOptions opts public string Name => "git-generic"; + /// + /// Generic completion pushes the base branch (it has no work-branch PR + /// concept), never the item's work branch — so its push is not recorded + /// as a work-branch tip. Recording the base tip as a work-branch push + /// would poison the lease decision with false evidence. + /// + public bool PushesWorkBranch => false; + public async Task PushAsync(string repositoryId, string branch, CancellationToken ct = default) { try diff --git a/src/CodeyBox.Upstream/NoopUpstreamRemote.cs b/src/CodeyBox.Upstream/NoopUpstreamRemote.cs index 6dc34d825..faef32b04 100644 --- a/src/CodeyBox.Upstream/NoopUpstreamRemote.cs +++ b/src/CodeyBox.Upstream/NoopUpstreamRemote.cs @@ -10,6 +10,12 @@ public sealed class NoopUpstreamRemote : IUpstreamRemote { public string Name => "noop"; + /// + /// The noop completion pushes nothing: there is no work-branch tip to + /// record, so push-history tracking stays empty for noop items. + /// + public bool PushesWorkBranch => false; + public Task PushAsync(string repositoryId, string branch, CancellationToken ct = default) => Task.FromResult(new UpstreamPushResult(true, null)); diff --git a/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs b/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs index a4413eb83..7f599d731 100644 --- a/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs +++ b/tests/CodeyBox.Tests/GitHubOwnedBranchPushTests.cs @@ -135,6 +135,91 @@ public async Task DivergedNoRecord_TrailerTip_PushesUnderLeaseAndReusesPr() r.RequestUri!.ToString().Contains("/compare/", StringComparison.Ordinal)); } + [Fact] + public async Task StaleLease_KnownHistoryTip_ProceedsUnderLeaseWithoutTrailerProof() + { + // The remote tip is an older CodeyBox push still in the bounded + // history while the latest record points elsewhere (a later push + // landed without being persisted): the recorded tip is accepted as + // CodeyBox's own without consulting commit trailers — the only + // proof available when trailers are disabled by configuration. + var workBranch = NewBranch(); + using var setup = await SetupAsync(); + var oldHead = await CommitBranchFileAsync(setup.UpstreamBare, workBranch, "old.txt", "old\n", "old work"); + var newHead = await RecomposeHostBranchOnFreshBaseAsync(setup.HostPath, workBranch); + + var (owner, repo) = SplitGithubUrl(setup.GithubUrl); + var handler = new FakeHttpMessageHandler(); + handler.Enqueue(Json(new // POST /pulls → fresh PR (AutoMerge off: no merge call) + { + number = 612, + html_url = $"https://github.com/{owner}/{repo}/pull/612", + })); + var remote = BuildRemote(setup.Host, handler, owner, repo, setup.Token, AutoMerge: false); + + var staleExpected = new string('7', 40); + var outcome = await remote.CompleteAsync( + SampleRequest(setup.RepoId, workBranch, "squash", ExpectedRemoteHeadSha: staleExpected, knownWorkBranchShas: [oldHead]), + CancellationToken.None); + + Assert.Equal(newHead, outcome.PushedWorkBranchSha); + Assert.Equal(612, outcome.PullRequestNumber); + var (_, tip, _) = await TestSupport.RunGit(setup.UpstreamBare, "rev-parse", workBranch); + Assert.Equal(newHead, tip.Trim()); + Assert.Contains(setup.Shim.Invocations, i => i.Contains("--force-with-lease=")); + // History short-circuits the trailer proof: no branch/compare call. + Assert.DoesNotContain(handler.Requests, r => + r.RequestUri!.ToString().Contains("/compare/", StringComparison.Ordinal)); + Assert.DoesNotContain(handler.Requests, r => + r.RequestUri!.ToString().Contains("/branches/", StringComparison.Ordinal)); + } + + [Fact] + public async Task StaleLease_UnrecordedTrailerTip_ProceedsUnderLeaseAfterTrailerProof() + { + // Replay of the d9951691 incident shape: a re-drive push landed on + // the remote but was never recorded, so the latest record is stale + // and the remote tip is in no history — yet every exclusive commit + // carries CodeyBox trailers. The push must proceed under lease + // instead of misreporting CodeyBox's own push as third-party work. + var workBranch = NewBranch(); + using var setup = await SetupAsync(); + const string trailerMessage = + "codeybox: merge codeybox/work\n\n" + + "CodeyBox-WorkItem: 6f2c9a1e3b4d5f6a7b8c9d0e1f2a3b4c\n" + + "Co-Authored-By: CodeyBox "; + var remoteTip = await CommitBranchFileAsync(setup.UpstreamBare, workBranch, "merge.txt", "merge\n", trailerMessage); + var newHead = await RecomposeHostBranchOnFreshBaseAsync(setup.HostPath, workBranch); + + var (owner, repo) = SplitGithubUrl(setup.GithubUrl); + var handler = new FakeHttpMessageHandler(); + handler.Enqueue(Json(new { commit = new { sha = new string('c', 40) } })); // GET /branches/main: current base tip + handler.Enqueue(Json(new // GET /compare/...remote: the stale tip is CodeyBox's own + { + total_commits = 1, + commits = new[] { new { sha = remoteTip, commit = new { message = trailerMessage } } }, + })); + handler.Enqueue(Json(new // POST /pulls → fresh PR (AutoMerge off: no merge call) + { + number = 612, + html_url = $"https://github.com/{owner}/{repo}/pull/612", + })); + var remote = BuildRemote(setup.Host, handler, owner, repo, setup.Token, AutoMerge: false); + + var staleExpected = new string('8', 40); + var outcome = await remote.CompleteAsync( + SampleRequest(setup.RepoId, workBranch, "squash", ExpectedRemoteHeadSha: staleExpected, knownWorkBranchShas: [staleExpected]), + CancellationToken.None); + + Assert.Equal(newHead, outcome.PushedWorkBranchSha); + Assert.Equal(612, outcome.PullRequestNumber); + var (_, tip, _) = await TestSupport.RunGit(setup.UpstreamBare, "rev-parse", workBranch); + Assert.Equal(newHead, tip.Trim()); + Assert.Contains(setup.Shim.Invocations, i => i.Contains("--force-with-lease=")); + Assert.Contains(handler.Requests, r => + r.RequestUri!.ToString().Contains("/compare/", StringComparison.Ordinal)); + } + [Fact] public async Task ThirdPartyPush_LeaseMismatchParksWithoutClobber() { @@ -147,6 +232,15 @@ public async Task ThirdPartyPush_LeaseMismatchParksWithoutClobber() var (owner, repo) = SplitGithubUrl(setup.GithubUrl); var handler = new FakeHttpMessageHandler(); + // A moved remote is trailer-proven before it is refused: the proof + // consults the branch + compare APIs and finds the unmarked + // third-party commit, so the lease still refuses. + handler.Enqueue(Json(new { commit = new { sha = new string('c', 40) } })); // GET /branches/main: current base tip + handler.Enqueue(Json(new // GET /compare/...remote: foreign commit, no trailers + { + total_commits = 1, + commits = new[] { new { sha = thirdPartyHead, commit = new { message = "third party change" } } }, + })); var remote = BuildRemote(setup.Host, handler, owner, repo, setup.Token, AutoMerge: true); var ex = await Assert.ThrowsAsync(() => @@ -156,12 +250,16 @@ public async Task ThirdPartyPush_LeaseMismatchParksWithoutClobber() Assert.Equal(workBranch, ex.Branch); Assert.Equal(oldHead, ex.ExpectedSha); - // Nothing clobbered: the third-party tip is intact and no PR API call - // was ever issued (the push guard fired first). + // Nothing clobbered: the third-party tip is intact and no PR-mutating + // API call was ever issued (only the read-only ownership proof ran + // before the push guard fired). var (_, tip, _) = await TestSupport.RunGit(setup.UpstreamBare, "rev-parse", workBranch); Assert.Equal(thirdPartyHead, tip.Trim()); Assert.NotEqual(newHead, tip.Trim()); - Assert.Empty(handler.Requests); + Assert.Contains(handler.Requests, r => + r.RequestUri!.ToString().Contains("/compare/", StringComparison.Ordinal)); + Assert.DoesNotContain(handler.Requests, r => + r.Method != HttpMethod.Get); } [Fact] @@ -325,7 +423,7 @@ public async Task OwnedBranchEqualToBase_RefusesBeforePush() } private static UpstreamCompletionRequest SampleRequest( - string repoId, string workBranch, string mergeMethod, string? ExpectedRemoteHeadSha) => new() + string repoId, string workBranch, string mergeMethod, string? ExpectedRemoteHeadSha, IReadOnlyList? knownWorkBranchShas = null) => new() { RepositoryId = repoId, WorkItemId = WorkItemId.New(), @@ -337,6 +435,7 @@ private static UpstreamCompletionRequest SampleRequest( Description = "desc", MergeMethod = mergeMethod, ExpectedRemoteHeadSha = ExpectedRemoteHeadSha, + KnownWorkBranchShas = knownWorkBranchShas, }; private static GitHubUpstreamRemote BuildRemote( diff --git a/tests/CodeyBox.Tests/UpstreamLeasePipelineTests.cs b/tests/CodeyBox.Tests/UpstreamLeasePipelineTests.cs index 404ea34b9..3fc73634d 100644 --- a/tests/CodeyBox.Tests/UpstreamLeasePipelineTests.cs +++ b/tests/CodeyBox.Tests/UpstreamLeasePipelineTests.cs @@ -1,3 +1,4 @@ +using System.Net.Http; using CodeyBox.Core; using CodeyBox.Git; using CodeyBox.Orchestrator; @@ -167,6 +168,155 @@ public async Task RedriveAsync_WithOpenPr_RecordsLeaseBaseBeforeResume() Assert.Equal(headSha, stored.LastPushedWorkBranchSha); } + [Fact] + public async Task UpstreamPush_Success_RecordsPushedShaAndHistory() + { + // Every successful work-branch push is recorded immediately: the + // item's last-pushed sha becomes the pushed tip and the bounded + // history carries it, so the next attempt proves the remote tip is + // CodeyBox's own. + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var pushedSha = new string('a', 40); + var remote = new RecordingUpstreamRemote + { + SeedRepoPath = seed, + OutcomeForCall = _ => new UpstreamCompletionOutcome + { + BranchPushed = true, + PushedWorkBranchSha = pushedSha, + PullRequestNumber = 7, + PullRequestUrl = "https://example.invalid/o/r/pull/7", + }, + }; + var factory = new RecordingRemoteFactory(remote); + + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + upstream: new ProjectUpstream + { + Kind = "recording-upstream", + AutoMerge = false, + MergeMethod = "squash", + }, + upstreamFactory: factory, + mergeStrategy: [MergeStrategy.RealMerge]); + remote.BareRepoRoot = tp.GitRoot; + tp.Agent.WorkPlan.Enqueue(new FileWrite("recorded.txt", "recorded\n")); + + var item = NewItem("codeybox/record-push-test"); + await tp.Store.CreateAsync(item); + await tp.Pipeline.RunAsync(item, CancellationToken.None); + + var final = (await tp.Store.GetAsync(item.Id))!; + Assert.Equal(WorkItemState.Done, final.State); + Assert.Equal(1, remote.CompleteCalls); + Assert.Equal(pushedSha, final.LastPushedWorkBranchSha); + Assert.Equal([pushedSha], final.PushedWorkBranchShaHistory); + Assert.Null(remote.Requests[0].ExpectedRemoteHeadSha); + } + + [Fact] + public async Task UpstreamPush_TransientFailure_RetriesWithRefreshedLeaseAndRecordsPush() + { + // A completion that fails AFTER its push landed must not lose the + // push: the tip is confirmed (remote tip equals the attempted local + // tip) and recorded before classification, and the retry carries it + // as the lease base instead of re-hitting a stale record. + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var secondPushSha = new string('b', 40); + var remote = new RecordingUpstreamRemote + { + SeedRepoPath = seed, + ThrowForCall = index => index == 0 + ? new HttpRequestException("connection reset after push") + : null, + OutcomeForCall = _ => new UpstreamCompletionOutcome + { + BranchPushed = true, + PushedWorkBranchSha = secondPushSha, + PullRequestNumber = 8, + PullRequestUrl = "https://example.invalid/o/r/pull/8", + }, + }; + var factory = new RecordingRemoteFactory(remote); + + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + upstream: new ProjectUpstream + { + Kind = "recording-upstream", + AutoMerge = false, + MergeMethod = "squash", + }, + upstreamFactory: factory, + mergeStrategy: [MergeStrategy.RealMerge], + pipelineOptions: new PipelineOptions + { + SandboxImageReference = "ignored", + UpstreamPushMaxAttempts = 2, + UpstreamPushBackoff = TimeSpan.Zero, + }); + remote.BareRepoRoot = tp.GitRoot; + tp.Agent.WorkPlan.Enqueue(new FileWrite("retry-record.txt", "retry\n")); + + var item = NewItem("codeybox/retry-record-test"); + await tp.Store.CreateAsync(item); + await tp.Pipeline.RunAsync(item, CancellationToken.None); + + var final = (await tp.Store.GetAsync(item.Id))!; + Assert.Equal(WorkItemState.Done, final.State); + Assert.Equal(2, remote.CompleteCalls); + // The retry succeeded with a new tip: it becomes the latest record, + // while the confirmed first-attempt tip stays in the history. + Assert.Equal(secondPushSha, final.LastPushedWorkBranchSha); + Assert.Equal(2, final.PushedWorkBranchShaHistory.Count); + Assert.Equal(secondPushSha, final.PushedWorkBranchShaHistory[1]); + // The first attempt carried no lease base; the retry carried the + // confirmed tip recorded from the failed attempt. + Assert.Null(remote.Requests[0].ExpectedRemoteHeadSha); + Assert.Equal(final.PushedWorkBranchShaHistory[0], remote.Requests[1].ExpectedRemoteHeadSha); + Assert.Equal([final.PushedWorkBranchShaHistory[0]], remote.Requests[1].KnownWorkBranchShas); + } + + [Fact] + public async Task RedriveAsync_WithOpenPr_AppendsLeaseBaseToPushHistory() + { + // The operator-authorized tip joins the bounded push history (not + // just the latest record): the retried run's push receipt extends + // this history, and the lease fallback accepts any entry — so the + // record survives even if a later push lands without being + // persisted. + using var fixture = CreateRedriveFixture(); + var oldPush = new string('a', 40); + var item = WorkItemPushHistory.RecordPush( + NewItem("codeybox/history-item", fixture.Project.Id) + .With(WorkItemState.Failed, "refusing to overwrite", failureKind: WorkItemFailureKinds.UpstreamBlocked), + oldPush); + item = item with { MergedPrNumber = 612 }; + await fixture.Store.CreateAsync(item); + var headSha = new string('d', 40); + fixture.Remote.OpenPullRequests.Add(new UpstreamPullRequest + { + Number = 612, + Url = "https://github.com/o/r/pull/612", + HeadBranch = "codeybox/history-item", + HeadSha = headSha, + BaseBranch = "main", + HasMergeConflict = false, + }); + fixture.Remote.BranchCommitMessages = + [ + "agent work\n\nCodeyBox-WorkItem: x\nCo-Authored-By: CodeyBox ", + ]; + + var result = await fixture.Redrive.RedriveAsync(item.Id, CancellationToken.None); + + Assert.False(result.Success); + var stored = (await fixture.Store.GetAsync(item.Id))!; + Assert.Equal(headSha, stored.LastPushedWorkBranchSha); + Assert.Equal([oldPush, headSha], stored.PushedWorkBranchShaHistory); + } + private static WorkItem NewItem(string branch, ProjectId? projectId = null) => new() { Id = WorkItemId.New(), @@ -304,4 +454,53 @@ private sealed class LeaseMismatchRemoteFactory(LeaseMismatchUpstreamRemote remo { public IUpstreamRemote Create(Project project) => remote; } + + private sealed class RecordingUpstreamRemote : IUpstreamRemote + { + public required string SeedRepoPath { get; init; } + public string? BareRepoRoot { get; set; } + public List Requests { get; } = new(); + public int CompleteCalls { get; private set; } + public string? LastRepositoryId { get; private set; } + public string Name => "recording-upstream"; + + public Func? OutcomeForCall { get; set; } + public Func? ThrowForCall { get; set; } + + public Task PushAsync(string repositoryId, string branch, CancellationToken ct = default) + => Task.FromResult(new UpstreamPushResult(true, null)); + + public Task CompleteAsync(UpstreamCompletionRequest request, CancellationToken ct = default) + { + Requests.Add(request); + LastRepositoryId = request.RepositoryId; + var index = CompleteCalls; + CompleteCalls++; + if (ThrowForCall?.Invoke(index) is { } ex) + throw ex; + return Task.FromResult(OutcomeForCall?.Invoke(index) + ?? new UpstreamCompletionOutcome { BranchPushed = true }); + } + + public Task TryMergeUpstreamBranchAsync(string targetBranch, string sourceBranch, CancellationToken ct = default) + => Task.FromResult(true); + + public async Task GetBranchHeadShaAsync(string branch, CancellationToken ct = default) + { + // Reads the host bare repo — the same repo the pipeline resolves + // the local tip from — so a failure after a real push observes + // remote == local and the tip is confirmed, while a diverged + // remote would observe a mismatch and record nothing. + if (BareRepoRoot is null || LastRepositoryId is null) + return null; + var barePath = Path.Combine(BareRepoRoot, LastRepositoryId + ".git"); + var (_, sha, _) = await TestSupport.RunGit(barePath, "rev-parse", "--verify", $"{branch}^{{commit}}"); + return sha.Trim(); + } + } + + private sealed class RecordingRemoteFactory(RecordingUpstreamRemote remote) : IUpstreamRemoteFactory + { + public IUpstreamRemote Create(Project project) => remote; + } } diff --git a/tests/CodeyBox.Tests/WorkItemPushHistoryTests.cs b/tests/CodeyBox.Tests/WorkItemPushHistoryTests.cs new file mode 100644 index 000000000..c519ec745 --- /dev/null +++ b/tests/CodeyBox.Tests/WorkItemPushHistoryTests.cs @@ -0,0 +1,148 @@ +using CodeyBox.Core; +using CodeyBox.Orchestrator; + +namespace CodeyBox.Tests; + +/// +/// Regression for the stale-record lease failure (d9951691): every +/// successful work-branch push must be recorded as the item's last-pushed +/// sha plus bounded history, so the next attempt proves the remote tip is +/// CodeyBox's own instead of misreporting it as third-party work. +/// Covers the pure history bookkeeping and its sqlite persistence +/// (round-trip, legacy NULL rows, corrupt payloads). +/// +public sealed class WorkItemPushHistoryTests : IDisposable +{ + private readonly string _workspace = + Directory.CreateTempSubdirectory("codeybox-push-history-").FullName; + + public void Dispose() => Directory.Delete(_workspace, recursive: true); + + private static WorkItem NewItem() => new() + { + Id = WorkItemId.New(), + ProjectId = new ProjectId("test-project"), + Title = "history test", + Prompt = "do the thing", + BaseBranch = "main", + WorkBranch = "codeybox/history-test", + PushUpstream = true, + }; + + private static string Sha(char c) => new(c, 40); + + [Fact] + public void RecordPush_SetsLatestAndAppendsHistory() + { + var item = NewItem(); + + var updated = WorkItemPushHistory.RecordPush(item, Sha('a')); + + Assert.Equal(Sha('a'), updated.LastPushedWorkBranchSha); + Assert.Equal([Sha('a')], updated.PushedWorkBranchShaHistory); + // Pure: the input is untouched. + Assert.Null(item.LastPushedWorkBranchSha); + Assert.Empty(item.PushedWorkBranchShaHistory); + } + + [Fact] + public void RecordPush_MovesRepeatToEndWithoutDuplicating() + { + var item = WorkItemPushHistory.RecordPush(NewItem(), Sha('a')); + item = WorkItemPushHistory.RecordPush(item, Sha('b')); + + var updated = WorkItemPushHistory.RecordPush(item, Sha('a')); + + Assert.Equal(Sha('a'), updated.LastPushedWorkBranchSha); + Assert.Equal([Sha('b'), Sha('a')], updated.PushedWorkBranchShaHistory); + } + + [Fact] + public void RecordPush_BoundsHistoryOldestDropsFirst() + { + var item = NewItem(); + item = WorkItemPushHistory.RecordPush(item, Sha('a'), maxEntries: 2); + item = WorkItemPushHistory.RecordPush(item, Sha('b'), maxEntries: 2); + + var updated = WorkItemPushHistory.RecordPush(item, Sha('c'), maxEntries: 2); + + Assert.Equal(Sha('c'), updated.LastPushedWorkBranchSha); + Assert.Equal([Sha('b'), Sha('c')], updated.PushedWorkBranchShaHistory); + } + + [Fact] + public void RecordPush_RejectsBlankAndImplausibleShas() + { + var item = NewItem(); + + Assert.Same(item, WorkItemPushHistory.RecordPush(item, null)); + Assert.Same(item, WorkItemPushHistory.RecordPush(item, " ")); + // Not hex / wrong length: untrusted forge output must never enter + // the history lease decisions branch on. + Assert.Same(item, WorkItemPushHistory.RecordPush(item, "not-a-sha")); + Assert.Same(item, WorkItemPushHistory.RecordPush(item, new string('z', 40))); + Assert.Same(item, WorkItemPushHistory.RecordPush(item, Sha('a')[..39])); + } + + [Fact] + public void Contains_MatchesLatestOrHistoryCaseInsensitively() + { + var item = WorkItemPushHistory.RecordPush(NewItem(), Sha('a')); + item = WorkItemPushHistory.RecordPush(item, Sha('b')); + + Assert.True(WorkItemPushHistory.Contains(item, Sha('b'))); + Assert.True(WorkItemPushHistory.Contains(item, Sha('a'))); + Assert.True(WorkItemPushHistory.Contains(item, Sha('a').ToUpperInvariant())); + Assert.False(WorkItemPushHistory.Contains(item, Sha('c'))); + Assert.False(WorkItemPushHistory.Contains(item, null)); + } + + [Fact] + public async Task Store_RoundTripsPushedHistory() + { + var dbPath = Path.Combine(_workspace, "history.db"); + using var store = new SqliteWorkItemStore(dbPath); + var item = WorkItemPushHistory.RecordPush(NewItem(), Sha('a')); + item = WorkItemPushHistory.RecordPush(item, Sha('b')); + await store.CreateAsync(item); + + var stored = (await store.GetAsync(item.Id))!; + + Assert.Equal(Sha('b'), stored.LastPushedWorkBranchSha); + Assert.Equal([Sha('a'), Sha('b')], stored.PushedWorkBranchShaHistory); + + // Updates persist the history too (the push-record path writes via + // UpdateAsync, not just CreateAsync). + await store.UpdateAsync(WorkItemPushHistory.RecordPush(stored, Sha('c'))); + var reread = (await store.GetAsync(item.Id))!; + Assert.Equal(Sha('c'), reread.LastPushedWorkBranchSha); + Assert.Equal([Sha('a'), Sha('b'), Sha('c')], reread.PushedWorkBranchShaHistory); + } + + [Fact] + public async Task Store_LegacyNullHistoryReadsAsEmpty() + { + var dbPath = Path.Combine(_workspace, "legacy.db"); + using var store = new SqliteWorkItemStore(dbPath); + var item = NewItem() with { LastPushedWorkBranchSha = Sha('a') }; + await store.CreateAsync(item); + + // Simulate a row predating the history column: NULL history with a + // recorded latest sha still leases on the latest sha alone. + await store.UpdateAsync(item with { PushedWorkBranchShaHistory = [] }); + using var conn = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={dbPath}"); + await conn.OpenAsync(); + using (var cmd = conn.CreateCommand()) + { + cmd.CommandText = "UPDATE work_items SET pushed_work_branch_sha_history = NULL WHERE id = $id"; + cmd.Parameters.AddWithValue("$id", item.Id.ToString()); + await cmd.ExecuteNonQueryAsync(); + } + + var stored = (await store.GetAsync(item.Id))!; + + Assert.Equal(Sha('a'), stored.LastPushedWorkBranchSha); + Assert.Empty(stored.PushedWorkBranchShaHistory); + Assert.True(WorkItemPushHistory.Contains(stored, Sha('a'))); + } +} From a592945a6555afaf7ea07340400d005e8a891e42 Mon Sep 17 00:00:00 2001 From: Adam Frisby Date: Tue, 13 Oct 2026 08:26:06 +0000 Subject: [PATCH 2/2] Fix stale agent-turn checkpoint discard after branch recompose (#739 follow-up) The stale-checkpoint discard was unreachable for typed refs: the pre-turn commit is the fork point of the tip and the checkpoint source, which is always an ancestor of the source itself, so the ancestry check never reported stale. Detect the moved base directly (current base tip no longer contained in the checkpoint source's history) and OR it into the discard condition; unresolvable lineage still resumes. Also fix the tests' recompose helper to advance the upstream seed repo instead of pushing bare main directly, which the pickup canonical-base-refresh force-fetch clobbers. Fixes the two StaleCheckpointFreshTurnTests failures. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox --- .../PipelineRunner.AgentTurnResume.cs | 42 +++++++++++++++++ .../PipelineRunner.WorkPhase.cs | 24 ++++++++-- .../StaleCheckpointFreshTurnTests.cs | 45 ++++++++++--------- 3 files changed, 88 insertions(+), 23 deletions(-) diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs b/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs index 3e42a9746..e2f5d8644 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs @@ -1270,6 +1270,48 @@ private async Task IsStaleAgentTurnCheckpointAsync( StringComparison.Ordinal); } + /// + /// True when the base branch moved past the checkpoint's source commit: + /// the current base tip is no longer contained in the checkpoint's + /// history, so the checkpoint tree was staged on an abandoned base and a + /// fresh turn from the current tip must replace it. This covers the + /// recompose/rebase case that the fork-point ancestry check above cannot + /// see: the pre-turn commit is the fork point of the tip and the source, + /// which is always an ancestor of the source itself, so that check alone + /// never reports a typed checkpoint stale. Applies only to typed refs + /// (the only shape carrying a source commit); anything unresolvable + /// answers false so an uncertain lineage resumes rather than discards. + /// + private async Task IsCheckpointBaseMovedAsync( + string repoId, + string baseBranch, + string checkpointSourceSha, + CancellationToken ct) + { + var source = checkpointSourceSha.Trim(); + string baseTip; + try + { + baseTip = (await _gitHost.ResolveCommitAsync(repoId, baseBranch, ct).ConfigureAwait(false)).Trim(); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _log.LogDebug(ex, "Could not resolve base branch '{BaseBranch}' tip for checkpoint lineage check; assuming unmoved", baseBranch); + return false; + } + if (string.Equals(baseTip, source, StringComparison.OrdinalIgnoreCase)) + return false; + try + { + return !await _gitHost.IsAncestorAsync(repoId, baseTip, source, ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _log.LogDebug(ex, "Could not check checkpoint lineage against base branch '{BaseBranch}'; assuming unmoved", baseBranch); + return false; + } + } + private const string StaleCheckpointLeaseRefusalMessage = "Cannot discard a retained-sandbox recovery lease for a stale durable agent-turn checkpoint; retry the retained agent turn or cancel the item so provider cleanup remains authoritative."; diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs b/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs index a06607845..f0139d8d5 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs @@ -324,8 +324,24 @@ await ConvertRetainedSandboxToCheckpointAsync( preemptCheckpoint = item.PreemptCheckpoint!; checkpointBranch = ValidatePreemptCheckpoint(item, preemptCheckpoint); await PipelineAgentExecutor.Run(sandbox, "git", "-C", SandboxConventions.WorkDir, "fetch", "origin", preemptCheckpoint); - if (await IsStaleAgentTurnCheckpointAsync( - repoId, baseBranch, preemptCheckpoint, preRefreshBaseTipSha, ct).ConfigureAwait(false)) + // Two independent signals detect a base move under the + // checkpoint: the pickup pre/post-refresh tip difference (the + // base ref moved across this pickup's upstream refresh) and + // the source-lineage probe (the current base tip is no longer + // contained in the checkpoint source's history, so the tree + // was staged on an abandoned base after a recompose/rebase). + // Either firing discards for a fresh turn. A same-base branch + // advance fires neither and still resumes below against the + // fork-point baseline. The fork-point ancestry check alone + // stays vacuous for typed refs (the pre-turn commit is the + // fork point of the tip and the source, always an ancestor + // of the source itself), so it cannot be the signal. + var checkpointSourceSha = CheckpointSourceSha(preemptCheckpoint); + var baseMovedPastCheckpoint = checkpointSourceSha is not null + && await IsCheckpointBaseMovedAsync(repoId, baseBranch, checkpointSourceSha, ct).ConfigureAwait(false); + var refreshMovedBase = await IsStaleAgentTurnCheckpointAsync( + repoId, baseBranch, preemptCheckpoint, preRefreshBaseTipSha, ct).ConfigureAwait(false); + if (refreshMovedBase || baseMovedPastCheckpoint) { // The base moved under the checkpoint after it was // written (a pickup-time rebase onto a fresh base or a @@ -341,7 +357,9 @@ await ConvertRetainedSandboxToCheckpointAsync( // never silently dropped. item = await DiscardStaleAgentTurnCheckpointAsync( item, - "durable agent-turn checkpoint base moved across the pickup upstream refresh", + refreshMovedBase + ? "durable agent-turn checkpoint base moved across the pickup upstream refresh" + : "durable agent-turn checkpoint source predates the current base tip after the work branch was recomposed", runner.Kind, agentPhase, ct).ConfigureAwait(false); diff --git a/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs b/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs index 69e19bd16..a035fc261 100644 --- a/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs +++ b/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs @@ -67,9 +67,10 @@ public async Task StaleCheckpoint_AfterBranchRebase_DiscardedAndFreshTurnRuns() agent.WorkResults.Enqueue(new AgentResult(true, "fresh ok", "fresh ok", null)); var logger = new CapturingLogger(); + var seedRepoPath = await TestSupport.CreateSeedRepoAsync(_workspace); using var setup = TestSupport.BuildPipeline( _workspace, - await TestSupport.CreateSeedRepoAsync(_workspace), + seedRepoPath, agentOverride: agent, logger: logger); @@ -81,7 +82,7 @@ await TestSupport.CreateSeedRepoAsync(_workspace), // restart-recovery recomposition: the base advances and the work // branch is recomposed at the fresh tip. The checkpoint still // descends from the old base, not from this tip. - var freshBaseTip = await RecomposeBranchOnFreshBaseAsync(setup, item); + var freshBaseTip = await RecomposeBranchOnFreshBaseAsync(setup, item, seedRepoPath); var barePath = BarePathFor(setup, item); var (staleCode, _, _) = await TestSupport.RunGitNoThrow( barePath, "merge-base", "--is-ancestor", await TipOfAsync(barePath, item.WorkBranch!), await CheckpointCommitOfAsync(setup, item)); @@ -163,9 +164,10 @@ public async Task StaleCheckpoint_WithAdvancedTipAfterRecompose_DiscardedAndFres agent.WorkResults.Enqueue(new AgentResult(true, "fresh ok", "fresh ok", null)); var logger = new CapturingLogger(); + var seedRepoPath = await TestSupport.CreateSeedRepoAsync(_workspace); using var setup = TestSupport.BuildPipeline( _workspace, - await TestSupport.CreateSeedRepoAsync(_workspace), + seedRepoPath, agentOverride: agent, logger: logger); @@ -173,7 +175,7 @@ await TestSupport.CreateSeedRepoAsync(_workspace), await setup.Store.CreateAsync(item); await PreemptFirstTurnAsync(setup, item, ready); - var freshBaseTip = await RecomposeBranchOnFreshBaseAsync(setup, item); + var freshBaseTip = await RecomposeBranchOnFreshBaseAsync(setup, item, seedRepoPath); var rivalTip = await AdvanceBranchAsync(setup, item, "rival.txt", "rival change\n", "rival advance"); var barePath = BarePathFor(setup, item); var (staleCode, _, _) = await TestSupport.RunGitNoThrow( @@ -256,9 +258,10 @@ public async Task StaleBranchMove_WithRetainedSandboxLease_RefusedWithExplicitMe var sandboxHook = new LeaseInjectingSandboxProvider( new ProcessSandboxProvider(Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance)); var logger = new CapturingLogger(); + var seedRepoPath = await TestSupport.CreateSeedRepoAsync(_workspace); using var setup = TestSupport.BuildPipeline( _workspace, - await TestSupport.CreateSeedRepoAsync(_workspace), + seedRepoPath, agentOverride: agent, logger: logger, sandboxProvider: sandboxHook); @@ -275,7 +278,7 @@ await TestSupport.CreateSeedRepoAsync(_workspace), // discard must refuse explicitly rather than silently drop // provider-owned state. var barePath = BarePathFor(setup, item); - await RecomposeBranchOnFreshBaseAsync(setup, item); + await RecomposeBranchOnFreshBaseAsync(setup, item, seedRepoPath); var staleCheckpointCommit = await CheckpointCommitOfAsync(setup, item); var (staleCode, _, _) = await TestSupport.RunGitNoThrow( barePath, "merge-base", "--is-ancestor", await TipOfAsync(barePath, item.WorkBranch!), staleCheckpointCommit); @@ -356,23 +359,25 @@ private static async Task PreemptFirstTurnAsync( /// /// Advances the base branch with a fresh commit and recomposes the work - /// branch at the fresh tip, returning the fresh base tip SHA. + /// branch at the fresh tip, returning the fresh base tip SHA. The base + /// advance lands in the upstream seed repo — never straight in the host + /// bare repo, whose main is force-refreshed from the seed on every + /// pickup (canonical base refresh) and would clobber a direct push. The + /// work branch is repointed at the fresh tip in the host bare repo, the + /// same end state a pickup-time rebase or restart-recovery recomposition + /// leaves behind. /// - private async Task RecomposeBranchOnFreshBaseAsync(TestPipeline setup, WorkItem item) + private async Task RecomposeBranchOnFreshBaseAsync(TestPipeline setup, WorkItem item, string seedRepoPath) { + await File.WriteAllTextAsync(Path.Combine(seedRepoPath, "base2.txt"), "fresh base\n"); + await TestSupport.RunGit(seedRepoPath, "add", "base2.txt"); + await TestSupport.RunGit(seedRepoPath, "commit", "-q", "-m", "fresh base commit"); + var (_, freshBaseTip, _) = await TestSupport.RunGit(seedRepoPath, "rev-parse", "HEAD"); var barePath = BarePathFor(setup, item); - var clone = Path.Combine(_workspace, "rebase-" + Guid.NewGuid().ToString("N")[..8]); - await TestSupport.RunGit(_workspace, "clone", "-q", barePath, clone); - await TestSupport.RunGit(clone, "config", "user.email", "rebase@example.invalid"); - await TestSupport.RunGit(clone, "config", "user.name", "Rebase Writer"); - await TestSupport.RunGit(clone, "checkout", "-q", "main"); - await File.WriteAllTextAsync(Path.Combine(clone, "base2.txt"), "fresh base\n"); - await TestSupport.RunGit(clone, "add", "base2.txt"); - await TestSupport.RunGit(clone, "commit", "-q", "-m", "fresh base commit"); - await TestSupport.RunGit(clone, "push", "-q", "origin", "HEAD:main"); - var (_, freshBaseTip, _) = await TestSupport.RunGit(clone, "rev-parse", "HEAD"); - await TestSupport.RunGit(clone, "checkout", "-q", "-B", item.WorkBranch!, "main"); - await TestSupport.RunGit(clone, "push", "-q", "origin", $"HEAD:{item.WorkBranch}"); + // Fetch the fresh base into the host bare repo and recompose the work + // branch at it (forced: recomposition rewrites the branch). The next + // pickup's canonical base refresh fetches the same tip for main. + await TestSupport.RunGit(barePath, "fetch", "-q", seedRepoPath, $"+main:refs/heads/{item.WorkBranch}"); return freshBaseTip.Trim(); }