From f0ebc8b66282163d42e58891a1bcb23f60f611f3 Mon Sep 17 00:00:00 2001 From: Adam Frisby Date: Mon, 12 Oct 2026 05:19:25 +0000 Subject: [PATCH 1/2] Restore stale agent-turn checkpoint discard across the pickup refresh The fork-point resume baseline (1b45c641) is an ancestor of the checkpoint by construction, so the stale check built on it could never fire: stale checkpoints resumed instead of being discarded, and a concurrent retained-sandbox lease surfaced as a generic checkpoint-changed failure instead of the explicit lease refusal. The branch tip alone cannot be the signal either, because a same-base branch advance must keep resuming while a base move must discard, and the two are indistinguishable after the pickup's upstream refresh rewrites the base ref. Snapshot the base-branch tip before the pickup refresh and decide staleness as a pre/post-refresh difference: any base move means the checkpoint's base was superseded. Same-base advances leave the base untouched and resume via the fork-point baseline as before; a missing snapshot falls back to base ancestry. Adds a combination test (stale checkpoint plus an advanced tip) proving the discard still stages the fresh turn at the current tip. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox --- docs/operating/agent-turn-checkpoints.md | 13 +- .../PipelineRunner.AgentTurnResume.cs | 82 ++++++------ .../PipelineRunner.Rebase.cs | 11 +- .../PipelineRunner.WorkPhase.cs | 26 ++-- src/CodeyBox.Orchestrator/PipelineRunner.cs | 18 ++- .../StaleCheckpointFreshTurnTests.cs | 120 ++++++++++++++++++ 6 files changed, 214 insertions(+), 56 deletions(-) diff --git a/docs/operating/agent-turn-checkpoints.md b/docs/operating/agent-turn-checkpoints.md index c6f27040..e675c725 100644 --- a/docs/operating/agent-turn-checkpoints.md +++ b/docs/operating/agent-turn-checkpoints.md @@ -99,14 +99,17 @@ accepts it only when the checkpoint already contains meaningful source changes relative to the pre-turn work-branch tip. An allow-empty checkpoint followed by a no-op resume uses the normal initial-work/rework no-diff failure path. -When the work branch legitimately moves after the checkpoint was written — a +When the base moves under the checkpoint after it was written — a pickup-time rebase onto a fresh base or a restart-recovery recomposition — the -checkpoint commit no longer descends from the current pre-turn tip and is -unusable, but that says nothing about the work. The turn preparation discards -the stale checkpoint (logged as `Discarding durable agent-turn resume +base ref changes across the pickup's upstream refresh and the checkpoint tree +no longer applies, but that says nothing about the work. The turn preparation +discards the stale checkpoint (logged as `Discarding durable agent-turn resume checkpoint`, the same record as a phase-boundary discard) and runs a normal fresh turn from the current branch tip with the branch preserved. This is not a -failure and does not count toward terminal failures. A checkpoint backed by a +failure and does not count toward terminal failures. A same-base advance of +the work branch (an outside writer landing commits without moving the base) +is not stale: the base ref is unchanged across the refresh, so the checkpoint +still applies and resumes against the fork-point baseline. A checkpoint backed by a retained-sandbox recovery lease is never discarded this way: provider-owned state stays authoritative and the resume is refused explicitly. diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs b/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs index e68f6482..3e42a974 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.AgentTurnResume.cs @@ -1209,24 +1209,34 @@ private static async Task IsPreTurnCommitAncestorOfCheckpointAsync( } /// - /// True when the published checkpoint commit no longer descends from the - /// pre-turn work-branch tip, e.g. the branch was rebased onto a fresh base - /// or recomposed by restart recovery after the checkpoint was written. The - /// checkpoint tree is then unusable, but that says nothing about the work. + /// True when the published checkpoint is stale: the base-branch tip moved + /// between the pickup's upstream refresh and turn preparation, so the + /// base the checkpoint was captured against was superseded (a pickup-time + /// rebase onto a fresh base or a restart-recovery recomposition). The + /// resume path checks the checkpoint tree out as-is and reconciles onto + /// the branch tip, which cannot span a base move — so the checkpoint tree + /// is unusable, but that says nothing about the work and the caller + /// discards the bookkeeping for a fresh turn. The move is detected as a + /// pre/post-refresh tip difference ( + /// was snapshotted before the pickup refresh ran): a same-base branch + /// advance leaves the base untouched and is NOT stale — it keeps resuming + /// against the fork-point baseline downstream. Neither the branch tip nor + /// the fork point can prove staleness (a fork point is an ancestor of the + /// checkpoint by construction, and the branch tip diverges on every + /// ordinary advance). Both resolutions below are host-side against the + /// authoritative bare repo, never the possibly trailing sandbox clone. /// The checkpoint commit resolves from its content binding for typed refs, - /// or from the published ref for legacy preempt checkpoints. Both shapes - /// are canonical hex after this resolution, so the merge-base argv below - /// carries no untrusted flag characters. + /// or from the published ref for legacy preempt checkpoints; both shapes + /// are validated canonical hex before the ancestry probe. /// - private static async Task IsStaleAgentTurnCheckpointAsync( - ISandbox sandbox, + private async Task IsStaleAgentTurnCheckpointAsync( + string repoId, + string baseBranch, string preemptCheckpointRef, - string checkpointBranch, - string? preTurnCommitSha, + string? baseTipBeforeRefresh, CancellationToken ct) { - if (string.IsNullOrWhiteSpace(preTurnCommitSha)) - throw new InvalidOperationException("Durable agent-turn resume has no pre-turn source commit."); + Validation.ValidateBranchName(baseBranch, nameof(baseBranch)); string checkpointCommitSha; if (AgentTurnCheckpointRef.TryParse(preemptCheckpointRef, out var typedRef) && typedRef is not null) @@ -1235,31 +1245,29 @@ private static async Task IsStaleAgentTurnCheckpointAsync( } else { - var resolved = await sandbox.ExecAsync(new SandboxExec - { - Argv = - [ - "git", "-C", SandboxConventions.WorkDir, - "rev-parse", "--verify", $"origin/{checkpointBranch}", - ], - MaxStdoutBytes = 4096, - MaxStderrBytes = 4096, - KillOnOutputLimit = true, - }, ct); - PipelineAgentExecutor.ThrowIfExecutionUnavailable(resolved); - if (!resolved.Success) - { - throw new InvalidOperationException( - "Failed to resolve the durable agent-turn checkpoint ref."); - } - checkpointCommitSha = resolved.Stdout.Trim(); + checkpointCommitSha = await _gitHost.ResolveCommitAsync( + repoId, preemptCheckpointRef, ct).ConfigureAwait(false); } - - return !await IsPreTurnCommitAncestorOfCheckpointAsync( - sandbox, - preTurnCommitSha, - checkpointCommitSha, - ct).ConfigureAwait(false); + Validation.ValidateCommitSha(checkpointCommitSha, nameof(checkpointCommitSha)); + var baseTip = await _gitHost.ResolveCommitAsync( + repoId, $"refs/heads/{baseBranch}", ct).ConfigureAwait(false); + Validation.ValidateCommitSha(baseTip, nameof(baseTip)); + if (string.IsNullOrWhiteSpace(baseTipBeforeRefresh)) + { + // No pre-refresh snapshot (the repo or base was absent at pickup): + // fall back to base ancestry — a checkpoint that does not descend + // from the current base cannot resume. + return !await _gitHost.IsAncestorAsync( + repoId, + baseTip.ToLowerInvariant(), + checkpointCommitSha.ToLowerInvariant(), + ct).ConfigureAwait(false); + } + Validation.ValidateCommitSha(baseTipBeforeRefresh, nameof(baseTipBeforeRefresh)); + return !string.Equals( + baseTipBeforeRefresh.ToLowerInvariant(), + baseTip.ToLowerInvariant(), + StringComparison.Ordinal); } private const string StaleCheckpointLeaseRefusalMessage = diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.Rebase.cs b/src/CodeyBox.Orchestrator/PipelineRunner.Rebase.cs index f6137699..8f712d06 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.Rebase.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.Rebase.cs @@ -1250,7 +1250,7 @@ internal sealed record WorkBranchStagingProof(string HostTip, string SandboxHead /// never throws for a missing branch. /// private async Task TryResolveHostBranchTipAsync( - string repoId, string branch, CancellationToken ct) + string repoId, string branch, CancellationToken ct, string scope = "work branch") { try { @@ -1261,8 +1261,8 @@ internal sealed record WorkBranchStagingProof(string HostTip, string SandboxHead catch (Exception ex) when (ex is not OperationCanceledException) { _log.LogDebug(ex, - "Could not resolve host tip of work branch '{Branch}' for staging log; continuing without it", - branch); + "Could not resolve host tip of {Scope} '{Branch}' for staging log; continuing without it", + scope, branch); return null; } } @@ -1278,6 +1278,11 @@ internal sealed record WorkBranchStagingProof(string HostTip, string SandboxHead /// ancestor, which keeps the validation in the safe direction. Falls back /// to the raw tip when the checkpoint source is unknown (legacy refs) or /// the fork point cannot be computed. + /// This baseline is only for the resumed no-op/meaningful-changes + /// validation. Whether the checkpoint is usable at all (stale discard) is + /// decided separately against the current base-branch tip — a fork point + /// is an ancestor of the checkpoint by construction and can never detect + /// a base move. /// private async Task ResolveAgentTurnResumeBaseAsync( string repoId, diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs b/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs index d2b3c899..a0660784 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.WorkPhase.cs @@ -52,7 +52,12 @@ public sealed partial class PipelineRunner // timings, streams, supervision, and prompt preprocessing instead of // "rework". Null keeps the legacy isInitial-derived labels. string? phaseLabelOverride = null, - AgentPromptPhase? promptPhaseOverride = null) + AgentPromptPhase? promptPhaseOverride = null, + // Base-branch tip snapshotted before the pickup's upstream refresh. + // Compared against the base tip at turn-preparation time: any move + // means the checkpoint's base was superseded and the checkpoint is + // stale. Null skips the comparison (fresh pickup without a resume). + string? preRefreshBaseTipSha = null) { item = await RefreshAgentTurnResumeCheckpointAsync(item, isInitial, iteration, ct); var resumingGitCheckpoint = !string.IsNullOrWhiteSpace(item.PreemptCheckpoint); @@ -320,20 +325,23 @@ await ConvertRetainedSandboxToCheckpointAsync( checkpointBranch = ValidatePreemptCheckpoint(item, preemptCheckpoint); await PipelineAgentExecutor.Run(sandbox, "git", "-C", SandboxConventions.WorkDir, "fetch", "origin", preemptCheckpoint); if (await IsStaleAgentTurnCheckpointAsync( - sandbox, preemptCheckpoint, checkpointBranch, resumePreTurnCommitSha, ct).ConfigureAwait(false)) + repoId, baseBranch, preemptCheckpoint, preRefreshBaseTipSha, ct).ConfigureAwait(false)) { - // The work branch legitimately moved after the checkpoint - // was written (a pickup-time rebase onto a fresh base or a - // restart-recovery recomposition). The checkpoint tree is + // The base moved under the checkpoint after it was + // written (a pickup-time rebase onto a fresh base or a + // restart-recovery recomposition moved the base ref across + // this pickup's upstream refresh). The checkpoint tree is // unusable, but that says nothing about the work: discard // the bookkeeping and run a normal fresh turn from the // current branch tip, preserving the branch — never a - // terminal failure. A retained-sandbox lease still refuses - // inside the discard: provider-owned state is never - // silently dropped. + // terminal failure. A same-base branch advance leaves the + // base untouched, so it still resumes below against the + // fork-point baseline. A retained-sandbox lease still + // refuses inside the discard: provider-owned state is + // never silently dropped. item = await DiscardStaleAgentTurnCheckpointAsync( item, - "durable agent-turn checkpoint no longer descends from its pre-turn work-branch commit", + "durable agent-turn checkpoint base moved across the pickup upstream refresh", runner.Kind, agentPhase, ct).ConfigureAwait(false); diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.cs b/src/CodeyBox.Orchestrator/PipelineRunner.cs index bddab671..9ab95130 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.cs @@ -316,6 +316,18 @@ await TransitionFailed(item, { await using var sandboxContext = new WorkSandboxContext(_sandboxes, _pipelineTuning, _log); var configuredBaseBranch = item.BaseBranch ?? project.DefaultBaseBranch; + // Snapshot the base-branch tip BEFORE the ensure below can + // refresh it from upstream. A durable agent-turn checkpoint is + // captured against the base it staged on: when the refresh moves + // the base, the checkpoint tree no longer applies and the work + // phase must discard it for a fresh turn. The branch tip alone + // cannot prove this — a same-base branch advance must keep + // resuming — so the pre/post-refresh comparison is the stale + // signal. Best-effort: a missing repo/branch yields null and the + // work phase falls back to base ancestry. + var baseTipBeforeRefresh = resumingPreempt && !string.IsNullOrWhiteSpace(configuredBaseBranch) + ? await TryResolveHostBranchTipAsync(item.Id.ToString(), configuredBaseBranch, ct, scope: "base branch") + : null; var (repoId, baseBranch) = await EnsurePipelineRepositoryAsync(item, project, configuredBaseBranch, ct); var hadRecordedWorkBranchAtEntry = !string.IsNullOrWhiteSpace(item.WorkBranch); var workBranch = item.WorkBranch ?? DefaultWorkBranchFor(item.Id); @@ -590,7 +602,8 @@ await RunWithStuckProbeAsync(trialItem, project, runner.Kind, "work", workPhase, hostShutdownToken, buildFailurePolicy: RequiredBuildPolicy.Terminal, auditorsForPreemptiveSelfReview: auditors, - resumePreTurnCommitSha: resumePreTurnCommitSha), + resumePreTurnCommitSha: resumePreTurnCommitSha, + preRefreshBaseTipSha: baseTipBeforeRefresh), workToken: attemptCt), ct, phaseCancellation: workPhase, @@ -658,7 +671,8 @@ trialItem.PreemptCheckpoint is { } checkpointRef // build gate and folded into the iteration's findings. buildFailurePolicy: RequiredBuildPolicy.DeferToAuditLoop, iteration: resumeIteration, - resumePreTurnCommitSha: resumePreTurnCommitSha), + resumePreTurnCommitSha: resumePreTurnCommitSha, + preRefreshBaseTipSha: baseTipBeforeRefresh), workToken: attemptCt), ct, phaseCancellation: reworkPhase, diff --git a/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs b/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs index 5d88956e..69e19bd1 100644 --- a/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs +++ b/tests/CodeyBox.Tests/StaleCheckpointFreshTurnTests.cs @@ -123,6 +123,104 @@ await TestSupport.CreateSeedRepoAsync(_workspace), Assert.DoesNotContain("output.txt", tree); } + [Fact] + public async Task StaleCheckpoint_WithAdvancedTipAfterRecompose_DiscardedAndFreshTurnStagesAtCurrentTip() + { + // Combination of a stale checkout tip (an outside writer lands a + // commit on the recomposed branch, as in the same-branch-advance + // recovery) plus a stale checkpoint (the base moved under it): the + // tip advance must not resurrect the stale resume. The checkpoint is + // still discarded and the fresh turn stages at the CURRENT tip, + // preserving the recomposed base, the landed commit, and the fresh + // turn's output — never the discarded checkpoint's tree. + var ready = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var calls = 0; + var agent = new ScriptedAgent([MergeStrategy.RealMerge]) { Kind = AgentKind.Claude }; + agent.BeforeWorkAsync = async (sandbox, workingDirectory, ct) => + { + calls++; + if (calls == 1) + { + var write = await sandbox.ExecAsync(new SandboxExec + { + Argv = ["sh", "-c", "echo 'real change' > \"$0\"", $"{workingDirectory}/output.txt"], + }, ct); + if (!write.Success) + throw new InvalidOperationException($"failed to write output.txt: {write.Stderr}"); + ready.TrySetResult(); + await Task.Delay(Timeout.Infinite, ct); + } + else + { + var write = await sandbox.ExecAsync(new SandboxExec + { + Argv = ["sh", "-c", "echo 'fresh change' > \"$0\"", $"{workingDirectory}/fresh.txt"], + }, ct); + if (!write.Success) + throw new InvalidOperationException($"failed to write fresh.txt: {write.Stderr}"); + } + }; + agent.WorkResults.Enqueue(new AgentResult(true, "fresh ok", "fresh ok", null)); + + var logger = new CapturingLogger(); + using var setup = TestSupport.BuildPipeline( + _workspace, + await TestSupport.CreateSeedRepoAsync(_workspace), + agentOverride: agent, + logger: logger); + + var item = NewItem(); + await setup.Store.CreateAsync(item); + await PreemptFirstTurnAsync(setup, item, ready); + + var freshBaseTip = await RecomposeBranchOnFreshBaseAsync(setup, item); + var rivalTip = await AdvanceBranchAsync(setup, item, "rival.txt", "rival change\n", "rival advance"); + var barePath = BarePathFor(setup, item); + var (staleCode, _, _) = await TestSupport.RunGitNoThrow( + barePath, "merge-base", "--is-ancestor", await TipOfAsync(barePath, item.WorkBranch!), await CheckpointCommitOfAsync(setup, item)); + Assert.Equal(1, staleCode); + + var resumeEntry = await setup.Store.GetAsync(item.Id) + ?? throw new InvalidOperationException("work item disappeared after preempt"); + await setup.Pipeline.RunAsync(resumeEntry, CancellationToken.None); + + var final = await setup.Store.GetAsync(item.Id); + Assert.NotNull(final); + Assert.Equal(WorkItemState.Done, final!.State); + Assert.Null(final.FailureKind); + Assert.Equal(0, final.TerminalFailureCount); + + Assert.Null(final.PreemptCheckpoint); + Assert.Null(final.AgentTurnResumeCheckpoint); + Assert.Null(final.AgentTurnRecoveryLease); + Assert.Equal(2, agent.WorkPrompts.Count); + Assert.DoesNotContain("codeybox/preempt", agent.WorkPrompts[1], StringComparison.OrdinalIgnoreCase); + Assert.Contains( + logger.Entries, + e => e.Message.Contains( + "Discarding durable agent-turn resume checkpoint", + StringComparison.OrdinalIgnoreCase)); + + // The fresh turn staged at the moved tip (not at the recomposed base + // alone): the tip advance survived the discard alongside the base. + Assert.Contains( + logger.Entries, + e => e.Message.Contains( + "at current bare-repo tip", + StringComparison.OrdinalIgnoreCase)); + var (baseAncestorCode, _, _) = await TestSupport.RunGitNoThrow( + barePath, "merge-base", "--is-ancestor", freshBaseTip, item.WorkBranch!); + Assert.Equal(0, baseAncestorCode); + var (rivalAncestorCode, _, _) = await TestSupport.RunGitNoThrow( + barePath, "merge-base", "--is-ancestor", rivalTip, item.WorkBranch!); + Assert.Equal(0, rivalAncestorCode); + var (_, tree, _) = await TestSupport.RunGit(barePath, "ls-tree", "-r", item.WorkBranch!, "--name-only"); + Assert.Contains("fresh.txt", tree); + Assert.Contains("base2.txt", tree); + Assert.Contains("rival.txt", tree); + Assert.DoesNotContain("output.txt", tree); + } + [Fact] public async Task StaleBranchMove_WithRetainedSandboxLease_RefusedWithExplicitMessage() { @@ -278,6 +376,28 @@ private async Task RecomposeBranchOnFreshBaseAsync(TestPipeline setup, W return freshBaseTip.Trim(); } + /// + /// Lands one outside-writer commit on top of the existing work branch, + /// returning the new tip SHA. + /// + private async Task AdvanceBranchAsync( + TestPipeline setup, WorkItem item, string file, string content, string message) + { + var barePath = BarePathFor(setup, item); + var clone = Path.Combine(_workspace, "advance-" + Guid.NewGuid().ToString("N")[..8]); + await TestSupport.RunGit(_workspace, "clone", "-q", barePath, clone); + await TestSupport.RunGit(clone, "config", "user.email", "advance@example.invalid"); + await TestSupport.RunGit(clone, "config", "user.name", "Advance Writer"); + await TestSupport.RunGit(clone, "fetch", "-q", "origin"); + await TestSupport.RunGit(clone, "checkout", "-q", "-B", item.WorkBranch!, $"origin/{item.WorkBranch}"); + await File.WriteAllTextAsync(Path.Combine(clone, file), content); + await TestSupport.RunGit(clone, "add", file); + await TestSupport.RunGit(clone, "commit", "-q", "-m", message); + await TestSupport.RunGit(clone, "push", "-q", "origin", $"HEAD:{item.WorkBranch}"); + var (_, tip, _) = await TestSupport.RunGit(clone, "rev-parse", "HEAD"); + return tip.Trim(); + } + private static async Task TipOfAsync(string barePath, string branch) { var (_, sha, _) = await TestSupport.RunGit(barePath, "rev-parse", branch); From 24c68c83cac958bbfa2e5189a9c722113bc4bb43 Mon Sep 17 00:00:00 2001 From: Adam Frisby Date: Mon, 12 Oct 2026 05:19:40 +0000 Subject: [PATCH 2/2] codeybox: stamp prompt-revision trailer CodeyBox-WorkItem: 138337e94f8c4e7e82078b352ca483e3 CodeyBox-Agent: copilot/muse-spark-1.3-contributor CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox