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.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.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/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/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/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(); } 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'))); + } +}