From ea3b2dcd010a7026d53a0a5cfef065f90c67c3e7 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Sat, 26 Sep 2026 00:22:45 +0800 Subject: [PATCH] Fence stale supervisor writes and close stopped work on Continue A Continue can land while a stop is still in flight: the stopped walk, on another host, may still be deciding, claiming or staging its supervisor's next turn, and the stop's teardown runs only after the stop commits. The generation fence covered the engine's own parks, but the supervisor writes its decisions, waves and questions in its own DI scope, so a turn the Continue overtook still wrote under the revived run: a decision the revived walk replayed as its own, a wave it re-parked on, a question card a person could answer. The engine now carries the generation it claimed to every node it runs, and every supervisor write takes the park's share lock on the run at that generation inside its own transaction: the decision's claim, begin and terminal record, the spawn wave with its budget admission, so an overtaken turn reserves nothing, and an ask_human question, whose card, wait and decision record now commit as one, so a question anyone can see always has its token on the tape the ask API answers from. The terminal record takes the lock before it reads the decision's status: a walk whose decision the revived walk finished first stands down instead of throwing the illegal transition into the engine as the step failing. An overtaken turn stands down with RunSupersededException, which the engine does not record as a failure. A decision it claimed before the revive stays in flight, and the revived walk finishes it the way it finishes a crashed walk's. The revive also ends what the stopped attempt left, in its own transaction after the generation bump: it discards every pending wait and cancels the queued and running agents and the staged child runs. The running agents' kills and the revived run's dispatch run once it commits, on no request token, so a client that goes away after the commit cannot cut them short. Before, the revived supervisor re-parked on the stopped wave, reclaimed its queued agent or folded a still-running agent onto its tape as "Running" for good, and a map branch adopted its old wait. The fold now also leaves a wave alone while any of its agents is live. A replayed spawn decision whose wave the stop closed restages only the closed slots: a finished agent keeps its answered wait, and each slot keeps the budget reservation it was first admitted under, which a recomputed deadline no longer turns into a refusal. The stop's teardown now skips waits another transaction holds, passes over them again while that transaction may still let them go, and runs each step on its own, so a Continue holding those waits can no longer deadlock it out of its kill-wave, nor leave one open by rolling back. --- .../Services/Agents/AgentRunService.cs | 100 ++- .../Agents/IRunningAgentCancellation.cs | 25 + .../RealSupervisorActionExecutor.Spawn.cs | 181 ++-- .../Supervisor/ISupervisorDecisionLog.cs | 112 ++- .../SupervisorTurnService.Rehydrate.cs | 13 +- .../Supervisor/SupervisorTurnService.cs | 26 +- .../Workflows/Engine/RunGenerationFence.cs | 89 ++ .../Engine/RunSupersededException.cs | 14 + .../Workflows/Engine/WorkflowEngine.cs | 23 +- .../Services/Workflows/IWorkflowService.cs | 8 +- .../Services/Workflows/WorkflowService.cs | 311 +++++-- .../Workflows/WorkflowServiceDependencies.cs | 3 +- .../Infrastructure/PostgresFixture.cs | 5 + .../Workflows/ContinueParkedRunFlowTests.cs | 203 ++++- .../Infrastructure/GatedAgentParkNode.cs | 17 +- .../ScriptedSupervisorDecider.cs | 53 +- .../Infrastructure/StopContinueTestKit.cs | 242 +++++ .../Infrastructure/WorkflowsTestSeed.cs | 7 +- .../OperatorCancelInProgressWalkFlowTests.cs | 80 +- ...SupervisorAskHumanStopContinueFlowTests.cs | 397 +++++++++ .../SupervisorStopContinueFlowTests.cs | 843 ++++++++++++++++++ .../Agents/SupervisorAgentResultsFoldTests.cs | 46 +- .../Architecture/FailureTaxonomyTests.cs | 8 +- .../ScopedTransactionInventoryTests.cs | 1 + .../Workflows/RunGenerationFenceTests.cs | 89 ++ 25 files changed, 2677 insertions(+), 219 deletions(-) create mode 100644 backend/src/CodeSpace.Core/Services/Agents/IRunningAgentCancellation.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/Engine/RunGenerationFence.cs create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/Engine/RunSupersededException.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/StopContinueTestKit.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorAskHumanStopContinueFlowTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorStopContinueFlowTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/RunGenerationFenceTests.cs diff --git a/backend/src/CodeSpace.Core/Services/Agents/AgentRunService.cs b/backend/src/CodeSpace.Core/Services/Agents/AgentRunService.cs index 12d66c928..de3371253 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/AgentRunService.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/AgentRunService.cs @@ -187,7 +187,7 @@ public interface IAgentRunService Task> GetEventsAsync(Guid runId, Guid teamId, long afterSequence, CancellationToken cancellationToken); } -public sealed partial class AgentRunService : IAgentRunService, IScopedDependency +public sealed partial class AgentRunService : IAgentRunService, IRunningAgentCancellation, IScopedDependency { public const string EventDataHolderKind = "agent_run_event"; @@ -908,24 +908,56 @@ public async Task CancelRunningAsync(Guid runId, string reason, AgentRunAb // would be held while the decisions are awaited. if (_db.Database.CurrentTransaction is not null) throw new InvalidOperationException("A stop lands its terminal and closes the run's decisions in statements of their own and cannot join an ambient transaction."); - // Read the run's epoch + handle FRESH + untracked, then flip via a status-guarded, epoch-fenced CAS pinned - // to Running (mirrors the reconciler's AbandonAsync, but → Cancelled, a deliberate cancel, not Failed). - // Fencing on the epoch we just read means a worker whose run was reclaimed (the reclaim bumped the epoch) - // and then revived can't be killed by a cancel that observed the old epoch — and, crucially, a run that - // legitimately completed in the same instant loses the CAS, so we never kill a finished run. 0 rows = no - // longer Running at this epoch → leave it alone. + if (await CancelRunningRowCoreAsync(runId, reason, cancellationToken).ConfigureAwait(false) is not { } cancelled) return false; + + await FinishCancelAsync(runId, cancelled, cause, cancellationToken).ConfigureAwait(false); + + _logger.LogInformation("Agent run cancelled while running. RunId={RunId} Reason={Reason}", runId, reason); + return true; + } + + public async Task CancelRunningRowAsync(Guid runId, string reason, CancellationToken cancellationToken) => + await CancelRunningRowCoreAsync(runId, reason, cancellationToken).ConfigureAwait(false) is not null; + + public async Task FinishRunningCancelAsync(Guid runId, AgentRunAbandonCause cause, CancellationToken cancellationToken) + { + // The decisions close in statements of their own, after the row's flip committed — as in CancelRunningAsync. + if (_db.Database.CurrentTransaction is not null) throw new InvalidOperationException("A cancel's side effects close the run's decisions in statements of their own and cannot join an ambient transaction."); + + var cancelled = await _db.AgentRun.AsNoTracking() + .Where(r => r.Id == runId && r.Status == AgentRunStatus.Cancelled) + .Select(r => new CancelledRun(r.TeamId, r.FenceEpoch, r.RunnerHandleJson, r.ResultJson)) + .SingleOrDefaultAsync(cancellationToken).ConfigureAwait(false); + + if (cancelled is null) return; + + await FinishCancelAsync(runId, cancelled, cause, cancellationToken).ConfigureAwait(false); + + _logger.LogInformation("Agent run cancel finished after its row was flipped. RunId={RunId} Cause={Cause}", runId, cause); + } + + /// + /// The row half of : read the run's epoch + handle FRESH + untracked, then flip via a + /// status-guarded, epoch-fenced CAS pinned to Running (mirrors the reconciler's AbandonAsync, but → Cancelled, a + /// deliberate cancel, not Failed). Fencing on the epoch just read means a worker whose run was reclaimed (the reclaim + /// bumped the epoch) and then revived can't be killed by a cancel that observed the old epoch — and, crucially, a run + /// that legitimately completed in the same instant loses the CAS, so a finished run is never killed. Null = no longer + /// Running at this epoch → leave it alone. The epoch bump is itself the owning worker's cue to stop: it loses its fence. + /// + /// 3c: a deliberate cancel is a CLEAN landing, so it releases the mid-run session checkpoint exactly as + /// completion does. Nobody owes this run a continuation, and a kept reference would pin the artifact Referenced + /// (terminal in the retention ledger) for good — and make a later retry of the same subtask read the cancel as a host + /// loss. Only an abandon-class ending keeps these columns: the reconciler's abandon, or its spool recovery. + /// + private async Task CancelRunningRowCoreAsync(Guid runId, string reason, CancellationToken cancellationToken) + { var snapshot = await _db.AgentRun.AsNoTracking() .Where(r => r.Id == runId) .Select(r => new { r.TeamId, r.Status, r.FenceEpoch, r.RunnerHandleJson, r.ResultJson }) .SingleOrDefaultAsync(cancellationToken).ConfigureAwait(false); - if (snapshot is null || snapshot.Status != AgentRunStatus.Running) return false; + if (snapshot is null || snapshot.Status != AgentRunStatus.Running) return null; - // 3c: a deliberate cancel is a CLEAN landing, so it releases the mid-run session checkpoint exactly as - // completion does. Nobody owes this run a continuation, and a kept reference would pin the artifact - // Referenced (terminal in the retention ledger) for good — and make a later retry of the same subtask read - // the cancel as a host loss. Only an abandon-class ending keeps these columns: the reconciler's abandon, or - // its spool recovery. var cancelled = await _db.AgentRun .Where(r => r.Id == runId && r.Status == AgentRunStatus.Running && r.FenceEpoch == snapshot.FenceEpoch) .ExecuteUpdateAsync(s => s @@ -934,16 +966,22 @@ public async Task CancelRunningAsync(Guid runId, string reason, AgentRunAb .SetProperty(r => r.Error, reason) .SetProperty(r => r.CompletedAt, (DateTimeOffset?)DateTimeOffset.UtcNow) .SetProperty(r => r.SessionTranscriptCheckpointArtifactId, (Guid?)null) - .SetProperty(r => r.SessionTranscriptCheckpointAt, (DateTimeOffset?)null), cancellationToken) - .ConfigureAwait(false); + .SetProperty(r => r.SessionTranscriptCheckpointAt, (DateTimeOffset?)null), cancellationToken).ConfigureAwait(false); - if (cancelled == 0) return false; + return cancelled == 0 ? null : new CancelledRun(snapshot.TeamId, snapshot.FenceEpoch + 1, snapshot.RunnerHandleJson, snapshot.ResultJson); + } + /// + /// The side-effect half of a won running cancel, every step best-effort — the run already reached Cancelled, and + /// none of this may change that. carries the run's FRESH fence (the CAS bumped it). + /// + private async Task FinishCancelAsync(Guid runId, CancelledRun cancelled, AgentRunAbandonCause cause, CancellationToken cancellationToken) + { // The run's unanswered decisions close with it, so its question leaves the queue and the Room the moment it stops. await StoppedRunDecisions.ExpireQuietlyAsync(_db, runId, _logger, cancellationToken).ConfigureAwait(false); // AFTER the CAS, never before: a cancel that lost the race leaves the claim to whoever lands the run. - await SettleSpendClaimsQuietlyAsync(runId, snapshot.TeamId, snapshot.ResultJson, cancellationToken).ConfigureAwait(false); + await SettleSpendClaimsQuietlyAsync(runId, cancelled.TeamId, cancelled.ResultJson, cancellationToken).ConfigureAwait(false); // FIRST side effect of a won cancel: withdraw the run's brokered model credential. Before the kill, not // after — a kill is a signal that races the agent's next model call, and losing that race used to mean the @@ -953,24 +991,24 @@ public async Task CancelRunningAsync(Guid runId, string reason, AgentRunAb // stops renewing on this very epoch bump and the lease lapses within its TTL. await RevokeBrokeredCredentialQuietlyAsync(runId, "run-cancelled").ConfigureAwait(false); - // The CAS above just bumped fence_epoch by exactly one, so this is the run's fresh fence — the closer's own - // fencing is what makes a call here safe even if that read were ever stale. - await TerminalizeCancelledHarnessExecutionQuietlyAsync(snapshot.TeamId, runId, snapshot.FenceEpoch + 1, cause, cancellationToken).ConfigureAwait(false); - - // Won the CAS → kill the sandbox process tree so the orphaned agent stops holding its workspace + burning - // the injected model credential. Best-effort (mirrors AbandonAsync's TerminateQuietlyAsync): the cancel - // stands even if the kill can't be issued. Only a durable runner with a parseable handle can be killed; a - // non-durable / handle-less run is already Cancelled and has no detached process to reap. Nor can a handle - // another HOST minted be reaped from here (the runner withholds the signal rather than kill whatever local - // process wears that pid) — that agent stops at its own wall-clock deadline instead. - var durable = ResolveDurableRunner(snapshot.RunnerHandleJson, out var handle); + // The CAS bumped fence_epoch by exactly one, so this is the run's fresh fence — the closer's own fencing is what + // makes a call here safe even if that read were ever stale. + await TerminalizeCancelledHarnessExecutionQuietlyAsync(cancelled.TeamId, runId, cancelled.FenceEpoch, cause, cancellationToken).ConfigureAwait(false); + + // Kill the sandbox process tree so the orphaned agent stops holding its workspace + burning the injected model + // credential. Best-effort (mirrors AbandonAsync's TerminateQuietlyAsync): the cancel stands even if the kill can't + // be issued. Only a durable runner with a parseable handle can be killed; a non-durable / handle-less run is + // already Cancelled and has no detached process to reap. Nor can a handle another HOST minted be reaped from here + // (the runner withholds the signal rather than kill whatever local process wears that pid) — that agent stops at + // its own wall-clock deadline instead. + var durable = ResolveDurableRunner(cancelled.RunnerHandleJson, out var handle); if (durable is not null && handle is not null) await TerminateQuietlyAsync(durable, handle, runId, cancellationToken).ConfigureAwait(false); - - _logger.LogInformation("Agent run cancelled while running. RunId={RunId} Reason={Reason}", runId, reason); - return true; } + /// What a won running cancel's side effects need: the run's team, its fresh fence (bumped by the cancel), its durable handle and its recorded result. + private sealed record CancelledRun(Guid TeamId, long FenceEpoch, string? RunnerHandleJson, string? ResultJson); + /// /// Close the cancelled run's own live native-record execution + any attempt still Running inside it, stamped /// with — the same closer the reconciler's own abandon paths use (#1864), reached here diff --git a/backend/src/CodeSpace.Core/Services/Agents/IRunningAgentCancellation.cs b/backend/src/CodeSpace.Core/Services/Agents/IRunningAgentCancellation.cs new file mode 100644 index 000000000..7af7f58a6 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/IRunningAgentCancellation.cs @@ -0,0 +1,25 @@ +using CodeSpace.Messages.Agents; + +namespace CodeSpace.Core.Services.Agents; + +/// +/// in its two halves, for a caller that must make the cancel visible +/// inside its own transaction but cannot run the cancel's side effects there. is the +/// epoch-fenced Running → Cancelled CAS alone: it joins the caller's transaction and touches nothing but the run's row, so +/// whatever reads the run after that commit already sees the agent Cancelled. is +/// the rest — expire its decisions, settle its spend claims, revoke its brokered credential, close its harness execution, +/// kill its process — which reach other scopes and connections that would wait on the caller's uncommitted row lock, and +/// closes the run's decisions, which the run's own end locks before its row: taken while the caller still held the row, +/// that close would invert the order. So the caller runs it after commit, outside any transaction, as +/// runs both halves. Continue's revive is the caller: the stopped +/// attempt's running agents are cancelled with the generation bump, and the stop's own teardown, landing later, simply +/// loses the CAS. +/// +public interface IRunningAgentCancellation +{ + /// The row half: Running → Cancelled at the epoch just read (bumping it), stamping . False when the run is no longer Running at that epoch — already terminal, or never launched — and it is left alone. + Task CancelRunningRowAsync(Guid runId, string reason, CancellationToken cancellationToken); + + /// The side-effect half, for a run whose row flipped and whose flip has committed. Best-effort end to end, and a no-op for a run that is not Cancelled. + Task FinishRunningCancelAsync(Guid runId, AgentRunAbandonCause cause, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/Executors/RealSupervisorActionExecutor.Spawn.cs b/backend/src/CodeSpace.Core/Services/Supervisor/Executors/RealSupervisorActionExecutor.Spawn.cs index 2b87c8f7e..9a9cf6f62 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/Executors/RealSupervisorActionExecutor.Spawn.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/Executors/RealSupervisorActionExecutor.Spawn.cs @@ -652,28 +652,35 @@ internal static DateTimeOffset AttemptReservationDeadline(AgentTask task) /// turn already exist; we REUSE them verbatim and re-park without staging anything. /// crash BEFORE the waits committed → no waits, but orphan Queued agents linger; we RECLAIM /// them for the leading slots and create agents only for the remainder. + /// the waits committed but the run's end CLOSED some of them (a stop's teardown, a failure's cleanup, a + /// Continue's revive — each Discards them and ends their agents) → nothing will answer those slots, so they + /// are staged afresh on the same keys; a slot whose agent had already finished keeps its agent and its + /// answered wait, and runs no second time. /// /// Safe because the node only reaches a spawn turn with ZERO pending agent waits (its re-entry guard re-parks /// otherwise), so neither an existing turn-wait nor a Queued agent here can be a healthy other-turn /// in-flight item — both are necessarily THIS decision's crash residue. + /// + /// FENCED on the walk's claimed run generation (): a turn a + /// Continue overtook stands down before it re-parks, and the wave's transaction takes the fence before it reserves + /// budget or writes a row, so such a turn reserves and stages nothing. A stop or a Continue that lands after the fence + /// waits for the whole wave to commit: the stop's teardown then ends all of it, and a revive closes it like any other + /// of the ended attempt's work. /// private async Task StageAgentsAndParkAsync(IReadOnlyList<(AgentTask Task, SupervisorAgentDispatch? Spec)> tasks, SupervisorTurnContext context, CancellationToken cancellationToken, SupervisorRetryEscalationOutcome? escalation = null, SupervisorStakeSet? stakes = null) { if (tasks.Count == 0) return SupervisorExecution.Synchronous(JsonSerializer.Serialize(new { agentRunIds = Array.Empty(), agentCount = 0, note = "no subtasks to spawn" }, AgentJson.Options)); - var existingWaitAgentIds = await ExistingTurnWaitAgentIdsAsync(context, cancellationToken).ConfigureAwait(false); + // A turn a Continue overtook stands down before it re-parks on a wave the revived walk now owns; every write + // after this is fenced where it happens. + await Workflows.Engine.RunGenerationFence.ThrowIfSupersededAsync(_db, context.SupervisorRunId, cancellationToken).ConfigureAwait(false); + + var wave = await TurnWaveAsync(context, cancellationToken).ConfigureAwait(false); - if (existingWaitAgentIds.Count > 0) - return ReparkOnExistingWaits(context, existingWaitAgentIds); + if (wave.IsOpen) + return ReparkOnExistingWaits(context, wave.AgentRunIds); - // W-hard 2a: ATOMIC wave admission — every attempt of this wave reserves its budget slice - // (cap ÷ total-spawn cap, the config-derived natural estimate) BEFORE anything stages, all-or-nothing: - // one rejection releases the wave's fresh reservations and returns a budget-blocked outcome the decider - // can read (mirroring the dependency-block precedent — positional integrity is never truncated mid-wave). - // Scope keys are the per-spawn iteration keys, so a crash-replayed staging lands on its own reservations - // (admitted as already-reserved). An uncapped run (no MaxCostUsd) reserves nothing — same authority as - // the realized-spend bound, which stays the user-facing stop. // D1 fail-CLOSED, BEFORE any reservation: a wave that would run a model nobody can price cannot be admitted // under a cost cap — its spend folds back as $0, so the cap it is admitted against would never trip. Blocking // is the same shape as the ledger's own refusal below (a synchronous budget-blocked outcome the decider reads, @@ -699,36 +706,6 @@ private async Task StageAgentsAndParkAsync(IReadOnlyList<(A }, AgentJson.Options)); } - if (context.MaxCostUsd is { } capUsd && context.SupervisorRunId != Guid.Empty && context.TeamId != Guid.Empty) - { - var estimate = capUsd / Math.Max(context.MaxTotalSpawns ?? SupervisorLane.DefaultMaxTotalSpawns, 1); - var reservedKeys = new List(); - - for (var k = 0; k < tasks.Count; k++) - { - var scopeKey = $"{(string.IsNullOrEmpty(context.NodeId) ? "sup" : context.NodeId)}#turn{context.TurnNumber}#{k}"; - var admission = await _budget.ReserveAsync(context.SupervisorRunId, context.TeamId, Workflows.Budget.BudgetKinds.AgentAttempt, scopeKey, estimate, capUsd, priceVersion: "realized-v1", parentReservationId: null, AttemptReservationDeadline(tasks[k].Task), cancellationToken).ConfigureAwait(false); - - if (!admission.Admitted) - { - foreach (var key in reservedKeys) - await _budget.ReleaseAsync(context.SupervisorRunId, context.TeamId, Workflows.Budget.BudgetKinds.AgentAttempt, key, cancellationToken).ConfigureAwait(false); - - _logger.LogWarning("Budget admission blocked a {Count}-agent wave on run {RunId}: {Reason}", tasks.Count, context.SupervisorRunId, admission.Reason); - - return SupervisorExecution.Synchronous(JsonSerializer.Serialize(new - { - budgetBlocked = tasks.Select(t => t.Task.SubtaskId).ToArray(), - reason = admission.Reason, - committedUsd = admission.CommittedUsd, - capUsd = admission.CapUsd, - }, AgentJson.Options)); - } - - reservedKeys.Add(scopeKey); - } - } - var orphans = await ReclaimableOrphanAgentIdsAsync(context, cancellationToken).ConfigureAwait(false); // P1a identity: every staged attempt carries the ATOMIC WorkUnitRef of the plan that dispatched it, read @@ -762,6 +739,14 @@ private async Task StageAgentsAndParkAsync(IReadOnlyList<(A // the wave back to zero visible residue; replay never observes a prefix and mistakes it for a complete wave. await using var stagingTransaction = await _db.Database.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); + // The fence comes first, so a turn a Continue overtook reserves and stages nothing (see RunGenerationFence), and a + // stop or a Continue that lands after it waits for the whole wave, reservations included, to commit. + await Workflows.Engine.RunGenerationFence.EnterAsync(_db, context.SupervisorRunId, cancellationToken).ConfigureAwait(false); + + if (await AdmitWaveAsync(tasks, context, wave, cancellationToken).ConfigureAwait(false) is { } budgetBlocked) return budgetBlocked; + + await DropClosedWaveAsync(context, cancellationToken).ConfigureAwait(false); + // P2a-2 (R): the staged units' acceptance obligations become durable requirement rows AT AUTHORIZATION — // the composer reads these, never re-derives them from the tape. Upsert-idempotent: a crash-replayed // staging lands on the same (run, kind, ref) rows. Model-authored oracles carry ModelProposal authority @@ -803,9 +788,18 @@ private async Task StageAgentsAndParkAsync(IReadOnlyList<(A var agentRunIds = new List(tasks.Count); var reclaimedAny = false; + var orphanCursor = 0; for (var k = 0; k < tasks.Count; k++) { + // A slot of a closed wave whose agent had already finished keeps that agent and its answered wait: only the + // slots the stop or failure closed run again. + if (wave.KeptAgentRunId(k) is { } kept) + { + agentRunIds.Add(kept); + continue; + } + // Reuse a reclaimed orphan for the leading slots (crash recovery — these were created by a prior // crashed pass of THIS decision, whose persisted TaskJson was ALREADY persona-resolved, so re-running // the resolver here would be redundant); else resolve the persona into the task (mirroring @@ -813,14 +807,14 @@ private async Task StageAgentsAndParkAsync(IReadOnlyList<(A // gate — team inherited from the supervisor run, never model-supplied. Linked to the supervisor run // + node so the completion notifier resumes the right run, and the reconciler's parent-terminal // guard governs it. - var reclaimed = k < orphans.Count; + var reclaimed = orphanCursor < orphans.Count; reclaimedAny |= reclaimed; // 3c: the ONE staging seam every spawn wave, retry and resolve passes through, so the checkpoint opt-in is // decided once here rather than at each verb's own task build — see CheckpointsSessionTranscript for who // is excluded and why. var agentRunId = reclaimed - ? orphans[k] + ? orphans[orphanCursor++] : await CreateResolvedAgentRunAsync(tasks[k].Task with { CheckpointSessionTranscript = CheckpointsSessionTranscript(tasks[k].Task, context, tasks.Count) }, tasks[k].Spec, context, cancellationToken).ConfigureAwait(false); StageAgentWait(context, k, agentRunId); @@ -846,9 +840,53 @@ private async Task StageAgentsAndParkAsync(IReadOnlyList<(A _logger.LogInformation("Supervisor staged {Count} agent run(s) at turn {Turn} on node {NodeId} (reused {Reused} crash orphan(s)); units: {Units}", agentRunIds.Count, context.TurnNumber, context.NodeId, Math.Min(orphans.Count, tasks.Count), DescribeStagedUnits(tasks)); + if (wave.KeptSlotCount > 0) + _logger.LogInformation("Supervisor re-staged a closed wave at turn {Turn} on node {NodeId}: kept {Kept} finished slot(s), staged {Restaged} afresh", context.TurnNumber, context.NodeId, wave.KeptSlotCount, tasks.Count - wave.KeptSlotCount); + return SupervisorExecution.ParkedOnAgents(outcome, agentRunIds.Count); } + /// + /// W-hard 2a: ATOMIC wave admission — every slot this wave stages for the first time reserves its budget slice + /// (cap ÷ total-spawn cap, the config-derived natural estimate), all-or-nothing, inside the wave's own fenced + /// transaction: the reservations commit with the agents and waits or not at all, so a turn a Continue overtook — or + /// a crash before the commit — leaves none behind to hold the cap or to refuse the replay as a different intent. A + /// refusal returns the budget-blocked outcome the decider reads (mirroring the dependency-block precedent — + /// positional integrity is never truncated mid-wave) and the caller's rollback takes back what this wave reserved. + /// A slot that already holds a row — a closed wave's slot staged afresh, or a finished slot kept — keeps the + /// reservation it was first admitted under: settlement settles {node}#turn{N}#{k} with whichever attempt the + /// decision records for slot k. An uncapped run (no MaxCostUsd) reserves nothing — same authority as the + /// realized-spend bound, which stays the user-facing stop. + /// + private async Task AdmitWaveAsync(IReadOnlyList<(AgentTask Task, SupervisorAgentDispatch? Spec)> tasks, SupervisorTurnContext context, TurnWave wave, CancellationToken cancellationToken) + { + if (context.MaxCostUsd is not { } capUsd || context.SupervisorRunId == Guid.Empty || context.TeamId == Guid.Empty) return null; + + var estimate = capUsd / Math.Max(context.MaxTotalSpawns ?? SupervisorLane.DefaultMaxTotalSpawns, 1); + + for (var k = 0; k < tasks.Count; k++) + { + if (wave.HasSlot(k)) continue; + + var scopeKey = $"{(string.IsNullOrEmpty(context.NodeId) ? "sup" : context.NodeId)}#turn{context.TurnNumber}#{k}"; + var admission = await _budget.ReserveAsync(context.SupervisorRunId, context.TeamId, Workflows.Budget.BudgetKinds.AgentAttempt, scopeKey, estimate, capUsd, priceVersion: "realized-v1", parentReservationId: null, AttemptReservationDeadline(tasks[k].Task), cancellationToken).ConfigureAwait(false); + + if (admission.Admitted) continue; + + _logger.LogWarning("Budget admission blocked a {Count}-agent wave on run {RunId}: {Reason}", tasks.Count, context.SupervisorRunId, admission.Reason); + + return SupervisorExecution.Synchronous(JsonSerializer.Serialize(new + { + budgetBlocked = tasks.Select(t => t.Task.SubtaskId).ToArray(), + reason = admission.Reason, + committedUsd = admission.CommittedUsd, + capUsd = admission.CapUsd, + }, AgentJson.Options)); + } + + return null; + } + /// The plan-local unit ids this staging dispatches, comma-joined ("s1,s2"); a task with no subtask key (a free-form spawn under no plan) reads "(unkeyed)". Pure + pinned — the other half of the plan log's edges↔units join. internal static string DescribeStagedUnits(IReadOnlyList<(AgentTask Task, SupervisorAgentDispatch? Spec)> tasks) => string.Join(",", tasks.Select(t => string.IsNullOrEmpty(t.Task.SubtaskId) ? "(unkeyed)" : t.Task.SubtaskId)); @@ -865,29 +903,68 @@ private async Task ReconcileEscalationWithDisp return NullIfBlank(actualModel) is { } model ? escalation with { To = model } : escalation; } - /// This turn's already-staged AgentRun wait tokens (the agent-run ids) in spawn-index order, or empty when none — the recovery anchor for a crash AFTER the waits committed but before the terminal was recorded. - private async Task> ExistingTurnWaitAgentIdsAsync(SupervisorTurnContext context, CancellationToken cancellationToken) + /// + /// This turn's wave as staged so far, one slot per spawn index: the agent-run id and its wait's status — the recovery + /// anchor for a crash AFTER the waits committed but before the terminal was recorded. Empty when the turn staged + /// nothing yet. + /// + private async Task TurnWaveAsync(SupervisorTurnContext context, CancellationToken cancellationToken) { - var keyPrefix = $"{context.NodeId}#turn{context.TurnNumber}#"; + var keyPrefix = TurnWaveKeyPrefix(context); var waits = await _db.WorkflowRunWait.AsNoTracking() .Where(w => w.RunId == context.SupervisorRunId && w.NodeId == context.NodeId && w.WaitKind == WorkflowWaitKinds.AgentRun && w.IterationKey.StartsWith(keyPrefix)) - .Select(w => new { w.IterationKey, w.Token }) + .Select(w => new { w.IterationKey, w.Token, w.Status }) .ToListAsync(cancellationToken).ConfigureAwait(false); // Order by the PARSED NUMERIC spawn index, NOT the lexicographic IterationKey: the key's trailing #{k} is raw // (non-zero-padded), so a text sort yields #0,#1,#10,…,#2 for K≥11 — scrambling agentRunIds out of the authored // subtaskIds[i] order the fan-out + the per-unit acceptance join rely on. SQL can't parse the index, so order // in memory (K ≤ 20). - return waits - .OrderBy(w => SupervisorOutcome.SpawnIndexOf(w.IterationKey)) - .Select(w => Guid.TryParse(w.Token, out var id) ? id : (Guid?)null) - .Where(id => id.HasValue) - .Select(id => id!.Value) - .ToList(); + return new TurnWave(waits + .Select(w => (Index: SupervisorOutcome.SpawnIndexOf(w.IterationKey), AgentRunId: Guid.TryParse(w.Token, out var id) ? id : (Guid?)null, w.Status)) + .Where(w => w.AgentRunId.HasValue) + .OrderBy(w => w.Index) + .Select(w => new WaveSlot(w.Index, w.AgentRunId!.Value, w.Status)) + .ToList()); + } + + /// + /// A turn's staged wave. OPEN while every slot's wait still stands (Pending, or Resolved by its agent's end): a replay + /// re-parks on it. CLOSED once the run's end Discarded any of its waits (a stop, or a failure's cleanup): those + /// agents were ended and nothing answers those waits any more, so the replay stages those slots afresh and keeps the + /// rest — a slot whose agent already finished is not run twice. + /// + private sealed record TurnWave(IReadOnlyList Slots) + { + public bool IsOpen => Slots.Count > 0 && Slots.All(s => s.Status != WorkflowWaitStatuses.Discarded); + + public IReadOnlyList AgentRunIds => Slots.Select(s => s.AgentRunId).ToList(); + + public int KeptSlotCount => Slots.Count(s => s.Status != WorkflowWaitStatuses.Discarded); + + public bool HasSlot(int index) => Slots.Any(s => s.Index == index); + + public Guid? KeptAgentRunId(int index) => Slots.FirstOrDefault(s => s.Index == index && s.Status != WorkflowWaitStatuses.Discarded)?.AgentRunId; } + /// One slot of a : its spawn index, its agent run and its wait's status. + private sealed record WaveSlot(int Index, Guid AgentRunId, string Status); + + /// Delete the wait rows of this turn's slots that the run's end closed, so their fresh waits can take the same per-turn-per-spawn keys under the unique (run, node, iteration) index — the supervisor's twin of the engine replacing a cell's earlier wait when its step parks again. A finished slot's answered wait stays. A no-op for a first staging. + private async Task DropClosedWaveAsync(SupervisorTurnContext context, CancellationToken cancellationToken) + { + var keyPrefix = TurnWaveKeyPrefix(context); + + await _db.WorkflowRunWait + .Where(w => w.RunId == context.SupervisorRunId && w.NodeId == context.NodeId && w.WaitKind == WorkflowWaitKinds.AgentRun && w.IterationKey.StartsWith(keyPrefix) && w.Status == WorkflowWaitStatuses.Discarded) + .ExecuteDeleteAsync(cancellationToken).ConfigureAwait(false); + } + + /// The IterationKey prefix every AgentRun wait of this turn's wave carries (<nodeId>#turn{N}#, then the spawn index). + private static string TurnWaveKeyPrefix(SupervisorTurnContext context) => $"{context.NodeId}#turn{context.TurnNumber}#"; + /// Re-park on the K waits a prior crashed pass already staged this turn — re-derive the outcome from their tokens WITHOUT staging or creating anything (no double-spawn). The node re-suspends on the existing waits. private SupervisorExecution ReparkOnExistingWaits(SupervisorTurnContext context, IReadOnlyList agentRunIds) { diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/ISupervisorDecisionLog.cs b/backend/src/CodeSpace.Core/Services/Supervisor/ISupervisorDecisionLog.cs index b1980d8ae..2722c1629 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/ISupervisorDecisionLog.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/ISupervisorDecisionLog.cs @@ -4,6 +4,7 @@ using CodeSpace.Core.Persistence; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Workflows.Engine; using CodeSpace.Messages.Agents; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; @@ -30,7 +31,9 @@ public interface ISupervisorDecisionLog /// Claim the right to execute this decision. INSERTs a Pending row; on the unique-index collision (a concurrent or /// prior decision for the same key) re-reads the existing row and returns /// (with the prior terminal outcome) when terminal, else . Exactly - /// one caller for a given key ever gets . + /// one caller for a given key ever gets . Inside a walk the INSERT + /// is fenced on the walk's claimed run generation (): a turn a Continue overtook + /// inserts nothing and gets . /// Task TryClaimAsync(SupervisorDecisionClaimRequest request, CancellationToken cancellationToken); @@ -39,10 +42,11 @@ public interface ISupervisorDecisionLog /// — it flips the row out of the claimable state BEFORE the side effect runs, so the synchronous path does NOT rely /// on the INSERT alone. Of N concurrent executors of the same claimed (run, key) exactly one update affects 1 row /// (true → run the side effect once); every loser affects 0 (false → re-read + replay). Returns whether THIS caller won. + /// Fenced like : an overtaken turn begins nothing and gets . /// Task TryBeginExecutionAsync(Guid decisionId, Guid teamId, CancellationToken cancellationToken); - /// Status-guarded CAS Running → terminal (Succeeded/Failed/Expired), team-scoped (defense-in-depth). Stores the execution outcome/error. Throws when the transition is illegal or lost the CAS. Gated by . + /// Status-guarded CAS Running → terminal (Succeeded/Failed/Expired), team-scoped (defense-in-depth). Stores the execution outcome/error. Throws when the transition is illegal or lost the CAS. Gated by . Fenced like : an overtaken turn records nothing and gets . Task RecordTerminalAsync(Guid decisionId, Guid teamId, SupervisorDecisionStatus status, string? outcomeJson, string? error, CancellationToken cancellationToken); /// Team-scoped audit/replay read of a run's decision rows, ordered by Sequence (the replay tape). A foreign run id returns empty. @@ -137,15 +141,12 @@ public async Task TryClaimAsync(SupervisorDecisionClaim QualityDecisionsJson = PersistedText.SanitizeJson(request.QualityDecisionsJson), }; - _db.SupervisorDecisionRecord.Add(row); - try { // INSERT-first against the unique (supervisor_run_id, idempotency_key) index — the serialization point. Two - // identical concurrent decisions both reach here; the DB lets exactly one INSERT win. - await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); - - return SupervisorDecisionClaim.Proceed(row.Id); + // identical concurrent decisions both reach here; the DB lets exactly one INSERT win. Fenced on the walk's + // claim: a turn a Continue overtook inserts nothing, so the revived run never finds a decision it did not make. + return await RunGenerationFence.CommitUnderClaimAsync(_db, request.SupervisorRunId, () => InsertClaimAsync(row, cancellationToken), cancellationToken).ConfigureAwait(false); } catch (DbUpdateException ex) when (IsUniqueViolation(ex)) { @@ -158,66 +159,89 @@ public async Task TryClaimAsync(SupervisorDecisionClaim } public async Task TryBeginExecutionAsync(Guid decisionId, Guid teamId, CancellationToken cancellationToken) + { + // Fenced like the claim: a turn a Continue overtook between its claim and here begins nothing, and leaves the + // decision Pending for the revived walk, which finds it in flight and finishes it. + if (await DecisionRunIdAsync(decisionId, teamId, cancellationToken).ConfigureAwait(false) is not { } runId) return false; + + var claimed = await RunGenerationFence.CommitUnderClaimAsync(_db, runId, () => BeginExecutionAsync(decisionId, teamId, cancellationToken), cancellationToken).ConfigureAwait(false); + + if (claimed) _logger.LogInformation("Supervisor decision claimed for execution. DecisionId={DecisionId}", decisionId); + + return claimed; + } + + /// + /// Single-winner CAS Pending → Running (mirrors RecordTerminalAsync's ExecuteUpdate discipline). The Status == Pending + /// guard is the must-fix-#2 gate: it flips the row out of the claimable state BEFORE the side effect runs, so of N + /// executors racing the same claimed (run, key) exactly one update affects 1 row (true → run the side effect once), + /// every loser affects 0 (false → re-read + replay). This is the single-winner guarantee the INSERT alone cannot + /// provide for the synchronous execution path. + /// + private async Task BeginExecutionAsync(Guid decisionId, Guid teamId, CancellationToken cancellationToken) { var now = DateTimeOffset.UtcNow; - // Single-winner CAS Pending → Running (mirrors RecordTerminalAsync's ExecuteUpdate discipline). The Status == - // Pending guard is the must-fix-#2 gate: it flips the row out of the claimable state BEFORE the side effect runs, - // so of N executors racing the same claimed (run, key) exactly one update affects 1 row (true → run the side - // effect once), every loser affects 0 (false → re-read + replay). This is the single-winner guarantee the INSERT - // alone cannot provide for the synchronous execution path. var claimed = await _db.SupervisorDecisionRecord .Where(d => d.Id == decisionId && d.TeamId == teamId && d.Status == SupervisorDecisionStatus.Pending) - .ExecuteUpdateAsync(s => s - .SetProperty(d => d.Status, SupervisorDecisionStatus.Running) - .SetProperty(d => d.LastModifiedDate, now), cancellationToken) - .ConfigureAwait(false); - - if (claimed > 0) _logger.LogInformation("Supervisor decision claimed for execution. DecisionId={DecisionId}", decisionId); + .ExecuteUpdateAsync(s => s.SetProperty(d => d.Status, SupervisorDecisionStatus.Running).SetProperty(d => d.LastModifiedDate, now), cancellationToken).ConfigureAwait(false); return claimed > 0; } + /// INSERT the Pending claim row; a unique violation escapes to , which re-reads the winner. + private async Task InsertClaimAsync(SupervisorDecisionRecord row, CancellationToken cancellationToken) + { + _db.SupervisorDecisionRecord.Add(row); + await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + + return SupervisorDecisionClaim.Proceed(row.Id); + } + + /// The supervisor run a decision belongs to — the run whose generation fences its writes — or null for an unknown or foreign decision. Team-scoped. + private async Task DecisionRunIdAsync(Guid decisionId, Guid teamId, CancellationToken cancellationToken) => + await _db.SupervisorDecisionRecord.AsNoTracking().Where(d => d.Id == decisionId && d.TeamId == teamId).Select(d => (Guid?)d.SupervisorRunId).SingleOrDefaultAsync(cancellationToken).ConfigureAwait(false); + public async Task RecordTerminalAsync(Guid decisionId, Guid teamId, SupervisorDecisionStatus status, string? outcomeJson, string? error, CancellationToken cancellationToken) { if (!SupervisorDecisionStateMachine.IsTerminal(status)) throw new SupervisorDecisionTransitionException($"SupervisorDecision terminal status must be terminal — got {status}."); + var runId = await DecisionRunIdAsync(decisionId, teamId, cancellationToken).ConfigureAwait(false) + ?? throw new SupervisorDecisionTransitionException($"SupervisorDecision {decisionId} not found."); + + // Both carry model/harness words; hoisted out of the expression tree so they are plain parameters. + var outcome = PersistedText.SanitizeJson(outcomeJson); + var storableError = PersistedText.Sanitize(error); + // Read the current status FRESH + untracked (team-scoped — defense-in-depth), then flip via a status-guarded CAS // (NOT a tracked save on the xmin token — same rationale as ToolCallLedgerService.RecordTerminalAsync). - var row = await _db.SupervisorDecisionRecord.AsNoTracking() - .Where(d => d.Id == decisionId && d.TeamId == teamId) - .Select(d => new { d.Status, d.SupervisorRunId }) - .SingleOrDefaultAsync(cancellationToken).ConfigureAwait(false) - ?? throw new SupervisorDecisionTransitionException($"SupervisorDecision {decisionId} not found."); + async Task FlipToTerminalAsync() + { + var current = await _db.SupervisorDecisionRecord.AsNoTracking().Where(d => d.Id == decisionId && d.TeamId == teamId).Select(d => d.Status).SingleAsync(cancellationToken).ConfigureAwait(false); - var current = row.Status; + if (!SupervisorDecisionStateMachine.IsLegalTransition(current, status)) + throw new SupervisorDecisionTransitionException($"Illegal SupervisorDecision transition {current} → {status} (decision {decisionId})."); - if (!SupervisorDecisionStateMachine.IsLegalTransition(current, status)) - throw new SupervisorDecisionTransitionException($"Illegal SupervisorDecision transition {current} → {status} (decision {decisionId})."); + var flipped = await _db.SupervisorDecisionRecord + .Where(d => d.Id == decisionId && d.TeamId == teamId && d.Status == current) + .ExecuteUpdateAsync(s => s.SetProperty(d => d.Status, status).SetProperty(d => d.OutcomeJson, outcome).SetProperty(d => d.Error, storableError).SetProperty(d => d.LastModifiedDate, DateTimeOffset.UtcNow), cancellationToken).ConfigureAwait(false); - var now = DateTimeOffset.UtcNow; + if (flipped == 0) + throw new SupervisorDecisionTransitionException($"SupervisorDecision {decisionId} was no longer {current} at terminal record — a concurrent transition won the race."); - // Both carry model/harness words; hoisted out of the expression tree so they are plain parameters. - var outcome = PersistedText.SanitizeJson(outcomeJson); - var storableError = PersistedText.Sanitize(error); + // P2 (ledger-version full coverage): a decision turning terminal enters the completion composer's read set. + await Services.Completion.CompletionLedgerVersionBump.BumpAsync(_db, runId, cancellationToken).ConfigureAwait(false); - var flipped = await _db.SupervisorDecisionRecord - .Where(d => d.Id == decisionId && d.TeamId == teamId && d.Status == current) - .ExecuteUpdateAsync(s => s - .SetProperty(d => d.Status, status) - .SetProperty(d => d.OutcomeJson, outcome) - .SetProperty(d => d.Error, storableError) - .SetProperty(d => d.LastModifiedDate, now), cancellationToken) - .ConfigureAwait(false); + return true; + } - if (flipped == 0) - throw new SupervisorDecisionTransitionException($"SupervisorDecision {decisionId} was no longer {current} at terminal record — a concurrent transition won the race."); + // Fenced like the claim, and the fence comes BEFORE the status is read: a turn a Continue overtook records nothing + // and stands down — never reading the revived walk's terminal as an illegal transition, which it would otherwise + // throw into the engine as the supervisor step failing, in the revived run's journal. + await RunGenerationFence.CommitUnderClaimAsync(_db, runId, FlipToTerminalAsync, cancellationToken).ConfigureAwait(false); _logger.LogInformation("Supervisor decision recorded terminal. DecisionId={DecisionId} Status={Status}", decisionId, status); - - // P2 (ledger-version full coverage): a decision turning terminal enters the completion composer's read set. - await Services.Completion.CompletionLedgerVersionBump.BumpAsync(_db, row.SupervisorRunId, cancellationToken).ConfigureAwait(false); } public async Task> GetForRunAsync(Guid supervisorRunId, Guid teamId, CancellationToken cancellationToken) => diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs index f6776d2b9..0953178f8 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.Rehydrate.cs @@ -519,8 +519,14 @@ private sealed record HumanAnswer(string Comment, string? Decision); /// parent-terminal-guarded) → an explicit Unknown placeholder, so the folded set is always N-for-N and the /// decider never sees a silent hole shorter than agentCount. Ids are iterated in RECORDED spawn order /// (replay-deterministic), never DB-row order. + /// + /// Nothing is folded while any staged agent is still Queued or Running: the fold is written once and never + /// revisited, so a status read before the agent ended would stand on the tape as its result for good. The barrier + /// normally rules that out; a turn re-entered without it — a Continue, whose revive ends the stopped attempt's agents + /// in the same commit, or any re-walk of a parked supervisor — is what this guards. Unfolded, the decision is read + /// again on the next rehydrate. /// - private static SupervisorPriorDecision FoldAgentResults(SupervisorPriorDecision decision, IReadOnlyDictionary resultsById) + internal static SupervisorPriorDecision FoldAgentResults(SupervisorPriorDecision decision, IReadOnlyDictionary resultsById) { if (!SupervisorDecisionKinds.StagesAgents(decision.DecisionKind)) return decision; @@ -532,9 +538,14 @@ private static SupervisorPriorDecision FoldAgentResults(SupervisorPriorDecision var folded = ids.Select(id => resultsById.TryGetValue(id, out var r) ? r : UnknownAgentResult(id)).ToList(); + if (folded.Any(IsStillLive)) return decision; + return decision with { OutcomeJson = SupervisorOutcome.FoldAgentResults(decision.OutcomeJson, folded) }; } + /// A folded result whose agent has not ended: Queued or Running. The placeholder for an agent that no longer resolves is final, not live. + private static bool IsStillLive(SupervisorAgentResult result) => Enum.TryParse(result.Status, out var status) && !AgentRunStateMachine.IsTerminal(status); + /// /// Validate one active-plan resolve's fixed K=1 carrier against its tenant-scoped durable AgentRun. A failure adds /// one fixed-size typed fact and never copies row/result/compact bytes. Existing facts are monotonic; healthy and diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.cs b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.cs index 95c8dd925..261e885cf 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorTurnService.cs @@ -706,9 +706,10 @@ private async Task ClaimAndExecuteAsync(Guid supervisorRunI /// CRASH-RECOVERY path, NOT a concurrent racer: the engine's run-level Enqueued → Running single-writer claim /// means no second walk executes this run concurrently, so a row already past Pending here was flipped Running /// by a PRIOR walk that crashed before recording terminal (e.g. mid spawn fan-out — orphan agents staged, no - /// waits, decision stuck Running). RE-EXECUTE under the existing Running claim so the turn doesn't self-advance - /// past an unfinished decision; the executor's spawn staging is idempotent (it reclaims this turn's orphan - /// agents), so the recovery produces exactly K agents + K waits with no double-spawn. + /// waits, decision stuck Running) — or, since a Continue fences the walk it overtook, by that walk. RE-EXECUTE + /// under the existing Running claim so the turn doesn't self-advance past an unfinished decision; the executor's + /// spawn staging is idempotent (it reclaims this turn's orphan agents), so the recovery produces exactly K agents + + /// K waits with no double-spawn. /// private async Task ExecuteUnderClaimAsync(Guid decisionId, Guid teamId, SupervisorTurnContext context, SupervisorDecision decision, CancellationToken cancellationToken) { @@ -717,6 +718,25 @@ private async Task ExecuteUnderClaimAsync(Guid decisionId, if (!won) _logger.LogWarning("Supervisor decision {DecisionId} was already Running (a prior walk crashed before recording terminal) — re-executing to recover, not self-advancing", decisionId); + return decision.Kind == SupervisorDecisionKinds.AskHuman + ? await ExecuteAndRecordTogetherAsync(decisionId, teamId, context, decision, cancellationToken).ConfigureAwait(false) + : await ExecuteAndRecordAsync(decisionId, teamId, context, decision, cancellationToken).ConfigureAwait(false); + } + + /// + /// ask_human's card, its wait and its terminal record commit as one transaction, fenced on the walk's claimed run + /// generation. A turn a Continue overtook writes none of the three, and one that got in first lands all three: a + /// question anyone can see — on its card, or through the run's ask API when there is no conversation — always has its + /// token on the tape the answer paths read (, the plan confirmation, the + /// human-touch reader). Recorded apart, a refused or crashed terminal left the decision in flight behind a posted + /// question, which the node's human re-entry guard then re-parked on and nothing could answer. + /// + private async Task ExecuteAndRecordTogetherAsync(Guid decisionId, Guid teamId, SupervisorTurnContext context, SupervisorDecision decision, CancellationToken cancellationToken) => + await Workflows.Engine.RunGenerationFence.CommitUnderClaimAsync(_db, context.SupervisorRunId, () => ExecuteAndRecordAsync(decisionId, teamId, context, decision, cancellationToken), cancellationToken).ConfigureAwait(false); + + /// Run the side effect ONCE, then record its terminal with the outcome enriched (see ). + private async Task ExecuteAndRecordAsync(Guid decisionId, Guid teamId, SupervisorTurnContext context, SupervisorDecision decision, CancellationToken cancellationToken) + { var execution = await ExecuteOrTerminalizeFailureAsync(decisionId, teamId, context, decision, cancellationToken).ConfigureAwait(false); // L4 P1: a terminal stop carrying a MODEL-authored acceptance check is graded HERE — inline on the decided-stop diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Engine/RunGenerationFence.cs b/backend/src/CodeSpace.Core/Services/Workflows/Engine/RunGenerationFence.cs new file mode 100644 index 000000000..d8d9a8ae6 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Engine/RunGenerationFence.cs @@ -0,0 +1,89 @@ +using CodeSpace.Core.Persistence.Db; +using Microsoft.EntityFrameworkCore; + +namespace CodeSpace.Core.Services.Workflows.Engine; + +/// +/// The run-generation fence () for what a walk commits in a +/// transaction of its own. is the share lock a park takes on the run row AT the walk's claimed +/// generation: a Continue's bump either waits for that commit or has already landed, and then nothing is written. +/// +/// The engine parks with the generation it holds. The supervisor records its decisions and stages its waves in its +/// own DI scope, where that generation is out of reach, so the engine carries its claim to every node it runs +/// () and those writes read it back () to take the same lock +/// (, ). It flows with the async call, so two walks of one +/// run on one host — an overtaken one and the revived one — each see their own. A turn driven outside a walk carries no +/// claim, and there is nothing to fence. +/// +public static class RunGenerationFence +{ + private static readonly AsyncLocal Current = new(); + + /// Carry as the claim of the walk running until the returned handle is disposed, which restores the claim it replaced. + public static IDisposable Claim(Guid runId, int generation) + { + var replaced = Current.Value; + Current.Value = new WalkClaim(runId, generation); + return new Restore(replaced); + } + + /// The generation the walk executing this code claimed for , or null outside a walk of that run. + public static int? ClaimedFor(Guid runId) => Current.Value is { } claim && claim.RunId == runId ? claim.Generation : null; + + /// + /// Share-lock the run row while it still stands at ; false once a Continue has moved it. + /// Held to the end of the caller's transaction, so a Continue's bump waits for it — which is why it refuses to run + /// outside one: there the lock would end with its own statement, and the fence would be only a check. + /// + public static async Task TryLockAsync(CodeSpaceDbContext db, Guid runId, int generation, CancellationToken cancellationToken) + { + if (db.Database.CurrentTransaction is null) + throw new InvalidOperationException($"The generation fence on run {runId} needs the caller's transaction: outside one its share lock ends with its own statement."); + + return (await db.Database.SqlQuery($"SELECT 1 AS \"Value\" FROM workflow_run WHERE id = {runId} AND generation = {generation} FOR SHARE").ToListAsync(cancellationToken).ConfigureAwait(false)).Count > 0; + } + + /// The fence for a transaction the caller already has open: share-lock the run at the current walk's claim, or throw once a Continue has moved it. Outside a walk it does nothing. + public static async Task EnterAsync(CodeSpaceDbContext db, Guid runId, CancellationToken cancellationToken) + { + if (ClaimedFor(runId) is not { } generation) return; + + if (!await TryLockAsync(db, runId, generation, cancellationToken).ConfigureAwait(false)) + throw new RunSupersededException(generation); + } + + /// Commit under the current walk's claim on : in one transaction (the caller's, or its own), fenced first, so a walk a Continue overtook writes nothing and stands down with . Outside a walk the write commits as it always did. + public static async Task CommitUnderClaimAsync(CodeSpaceDbContext db, Guid runId, Func> write, CancellationToken cancellationToken) + { + if (ClaimedFor(runId) is null) return await write().ConfigureAwait(false); + + await using var transaction = await ScopedTransaction.OwnOrJoinAsync(db.Database, cancellationToken).ConfigureAwait(false); + + await EnterAsync(db, runId, cancellationToken).ConfigureAwait(false); + + var result = await write().ConfigureAwait(false); + await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + + return result; + } + + /// Stand down before a step that writes nothing itself — a re-park on a wave or question already staged: throw once a Continue has moved the run past the current walk's claim. A read, not a lock: what follows is fenced where it writes. + public static async Task ThrowIfSupersededAsync(CodeSpaceDbContext db, Guid runId, CancellationToken cancellationToken) + { + if (ClaimedFor(runId) is not { } generation) return; + + if (!await db.WorkflowRun.AsNoTracking().AnyAsync(r => r.Id == runId && r.Generation == generation, cancellationToken).ConfigureAwait(false)) + throw new RunSupersededException(generation); + } + + private sealed record WalkClaim(Guid RunId, int Generation); + + private sealed class Restore : IDisposable + { + private readonly WalkClaim? _replaced; + + public Restore(WalkClaim? replaced) { _replaced = replaced; } + + public void Dispose() => Current.Value = _replaced; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Engine/RunSupersededException.cs b/backend/src/CodeSpace.Core/Services/Workflows/Engine/RunSupersededException.cs new file mode 100644 index 000000000..303fe77e0 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Engine/RunSupersededException.cs @@ -0,0 +1,14 @@ +namespace CodeSpace.Core.Services.Workflows.Engine; + +/// +/// Thrown once a Continue has revived the run past the generation the walk doing the work claimed — at a wave check, at +/// a step's park, or at the staging a node commits itself under (the supervisor's spawn +/// wave). It unwinds the overtaken walk to WorkflowEngine.RunAfterClaimAsync, which stands the walk down writing +/// nothing: the run belongs to the revived walk. +/// +public sealed class RunSupersededException : Exception +{ + public RunSupersededException(int claimed) : base($"Run was continued past this walk (claimed generation {claimed}).") { Claimed = claimed; } + + public int Claimed { get; } +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs b/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs index 4d9d23981..24419ac01 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Engine/WorkflowEngine.cs @@ -3097,7 +3097,7 @@ private async Task TryParkAsync(WorkflowRunWait wait, CancellationToken ca // caller's whole unit of work. The engine walks on its own scope, so nothing is ambient here. await using var transaction = await _db.Database.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); - if (!await LockRunAtGenerationAsync(wait.RunId, cancellationToken).ConfigureAwait(false)) return false; + if (!await RunGenerationFence.TryLockAsync(_db, wait.RunId, _generation, cancellationToken).ConfigureAwait(false)) return false; await _recordLogger.NodeSuspendedAsync(wait.RunId, wait.NodeId, wait.IterationKey, wait.WaitKind, wait.WakeAt, cancellationToken).ConfigureAwait(false); await ReplaceCellWaitAsync(wait, cancellationToken).ConfigureAwait(false); @@ -3106,10 +3106,6 @@ private async Task TryParkAsync(WorkflowRunWait wait, CancellationToken ca return true; } - /// Share-lock the run row while it still stands at this walk's generation; false once a Continue has moved it. Held to the end of the caller's transaction, so a Continue's bump waits for it. - private async Task LockRunAtGenerationAsync(Guid runId, CancellationToken cancellationToken) => - (await _db.Database.SqlQuery($"SELECT 1 AS \"Value\" FROM workflow_run WHERE id = {runId} AND generation = {_generation} FOR SHARE").ToListAsync(cancellationToken).ConfigureAwait(false)).Count > 0; - /// One outstanding wait per (run, node, iteration): drop any prior (resolved) wait for the cell so a re-suspend can't trip the unique index, then add this one. private async Task ReplaceCellWaitAsync(WorkflowRunWait wait, CancellationToken cancellationToken) { @@ -3428,7 +3424,9 @@ private async Task ExecuteNodeAsync(WorkflowRun run, NodeDefinit /// return; on a thrown exception returns (null, message) so the retry loop can treat it /// as a failure. Cancellation and the secret-leak guard are re-thrown — they're not retryable: /// a cancel stops the run, and a leak is a contract violation that must surface its detailed - /// message (the loop would otherwise mask it as a generic node failure). + /// message (the loop would otherwise mask it as a generic node failure). So is a + /// from staging the node fenced on this walk's claim: the walk + /// stands down, and recording it as the node's failure would write that failure into the revived run. /// private async Task<(NodeResult? Result, Exception? Thrown)> RunNodeOnceAsync(NodeExecution exec, CancellationToken cancellationToken) { @@ -3436,6 +3434,10 @@ private async Task ExecuteNodeAsync(WorkflowRun run, NodeDefinit { var context = BuildNodeRunContext(exec); + // The generation this walk claimed rides along with the node, for staging it commits in a transaction of its + // own (the supervisor's spawn wave) to fence on, as this engine fences its own park. + using var claim = RunGenerationFence.Claim(exec.Run.Id, _generation); + // Recording (make "record EVERY in-process model call" literally true, not just supervisor.decision): push the // run/node correlation + this engine's SCOPED ledger writer + offloader for the duration of the node, so the // singleton RecordingLLMClientDecorator captures the interaction.* triple of ANY model call the node makes @@ -3451,6 +3453,7 @@ private async Task ExecuteNodeAsync(WorkflowRun run, NodeDefinit return (result, null); } catch (OperationCanceledException) { throw; } + catch (RunSupersededException) { throw; } catch (WorkflowSecretLeakException) { throw; } catch (WorkflowRedactedOutputsUnrecoverableException) { throw; } catch (Exception ex) @@ -3888,14 +3891,6 @@ private sealed class RunSuspendedException : Exception public RunSuspendedException(string nodeId) : base($"Run suspended on node '{nodeId}'.") { } } - /// Thrown at a wave check, or at a step's park, once a Continue has revived the run past the generation this walk claimed. Caught in RunAfterClaimAsync, which stands the walk down without writing — the run belongs to the revived walk. - private sealed class RunSupersededException : Exception - { - public RunSupersededException(int claimed) : base($"Run was continued past this walk (claimed generation {claimed}).") { Claimed = claimed; } - - public int Claimed { get; } - } - /// /// Per-run mutable bookkeeping for the frontier walker. Lives only for the duration of /// — gets garbage-collected when the run ends. Pre-computes diff --git a/backend/src/CodeSpace.Core/Services/Workflows/IWorkflowService.cs b/backend/src/CodeSpace.Core/Services/Workflows/IWorkflowService.cs index ee268954a..8909bb7c0 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/IWorkflowService.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/IWorkflowService.cs @@ -175,7 +175,13 @@ public interface IWorkflowService /// (the dispatcher's Pending → Enqueued CAS is the double-dispatch guard). /// /// Reviving a terminal run starts a new run generation: a walk still winding down from the stop or failure, - /// and a stop's teardown still in flight, carry the old one and touch nothing of the revived run. + /// and a stop's teardown still in flight, carry the old one and touch nothing of the revived run. The revive also + /// closes, in the same transaction, what the ended attempt left pending — its waits Discarded, its queued and + /// running agents cancelled, its staged child runs cancelled — so the revived run never parks on, reuses or folds + /// as its own work a late teardown then ends. A running agent gets only its row flipped there; its process kill, + /// credential revoke and spend settlement run once that commit is visible, and a stop's teardown landing later + /// loses every CAS on what the revive already ended. A Continue after a Failure, which has no teardown, gets its + /// kills from the revive alone. /// /// A step the continue re-runs starts over, because a wait the stop or failure closed carries no answer. So a /// flow.sleep parks a fresh timer for its FULL delay — the time it already slept is not credited — and a diff --git a/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs b/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs index af1bb76f6..b8b9d6659 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/WorkflowService.cs @@ -44,6 +44,7 @@ public sealed class WorkflowService : IWorkflowService, IScopedDependency private readonly Engine.IWorkflowResumeService _resumeService; private readonly IPostCommitActions _postCommit; private readonly IAgentRunService _agentRunService; + private readonly IRunningAgentCancellation _runningAgents; private readonly Rerun.IRerunCellSeeder _cellSeeder; private readonly Engine.IRunCancellationRegistry _cancellationRegistry; private readonly ILogger _logger; @@ -51,6 +52,12 @@ public sealed class WorkflowService : IWorkflowService, IScopedDependency /// Reason stamped on a branch agent run aborted by the kill-wave when an operator cancels its parent workflow run. private const string OperatorCancelledAgentReason = "Cancelled because its parent workflow run was cancelled by an operator."; + /// Reason stamped on an agent run that was still queued when Continue revived its parent workflow run: the attempt that staged it had ended, and the continued run stages its own. + private const string ContinuedRunAgentReason = "Cancelled before it started because its parent workflow run was continued; the continued run stages its own agents."; + + /// Reason stamped on an agent run that was still running when Continue revived its parent workflow run: the attempt it worked for had ended, and nothing of the continued run waits on it. + private const string ContinuedRunRunningAgentReason = "Cancelled because its parent workflow run was continued; the attempt it worked for had ended, and the continued run stages its own agents."; + /// What someone answering the question of a run that ended — its card or its Room decision — is told. The run's end closed the wait unanswered (Discarded): an operator's stop, or the engine's own cleanup when the run failed. Continue re-runs the step, which asks again: telling them the question was "already resolved" would say someone answered it. public const string EndedRunQuestionMessage = "This run ended (it was stopped or failed), so its question closed unanswered. It opens again when the run is continued."; @@ -67,6 +74,7 @@ public WorkflowService(CodeSpaceDbContext db, WorkflowDefinitionServices definit _resumeService = control.Resume; _postCommit = launch.PostCommit; _agentRunService = control.Agents; + _runningAgents = control.RunningAgents; _cellSeeder = control.Cells; _cancellationRegistry = control.Cancellation; _logger = logger; @@ -949,7 +957,7 @@ private async Task ContinueFailedRunAsync(Guid runId, Guid teamId, Cancell foreach (var nodeId in toReset) await _recordLogger.NodeStartedAsync(runId, nodeId, WorkflowIterationKeys.TopLevel, EmptyNodeBag, EmptyNodeBag, cancellationToken).ConfigureAwait(false); - await _postCommit.RunAfterCommitAsync(ct => _runDispatcher.DispatchAsync(runId, ct), cancellationToken).ConfigureAwait(false); + await DispatchRevivedRunAfterCommitAsync(runId, cancellationToken).ConfigureAwait(false); _logger.LogInformation("Failed workflow run continued in place by operator. RunId={RunId} TeamId={TeamId} ResetNodes={ResetNodes}", runId, teamId, string.Join(",", toReset)); @@ -995,7 +1003,7 @@ private async Task ContinueCancelledRunAsync(Guid runId, Guid teamId, Canc foreach (var nodeId in frontier) await _recordLogger.NodeStartedAsync(runId, nodeId, WorkflowIterationKeys.TopLevel, EmptyNodeBag, EmptyNodeBag, cancellationToken).ConfigureAwait(false); - await _postCommit.RunAfterCommitAsync(ct => _runDispatcher.DispatchAsync(runId, ct), cancellationToken).ConfigureAwait(false); + await DispatchRevivedRunAfterCommitAsync(runId, cancellationToken).ConfigureAwait(false); _logger.LogInformation("Cancelled workflow run continued in place by operator. RunId={RunId} TeamId={TeamId} ResumedNodes={ResumedNodes}", runId, teamId, string.Join(",", frontier)); @@ -1003,22 +1011,156 @@ private async Task ContinueCancelledRunAsync(Guid runId, Guid teamId, Canc } /// - /// The CAS both in-place revivals of a terminal run share: → Pending as a NEW generation, - /// atomically claiming the revive (false = a concurrent continue / replay already moved it). The bump is the fence: - /// the walk that ended the run — or one the reconciler gave up on that is still alive — and a stop's post-commit - /// teardown still in flight both carry the old generation, and lose to it from here on. The finish time and error - /// go in the same statement: the revived run is live again and they no longer describe it; its next terminal writes - /// its own. + /// The revive both in-place continues of a terminal run share, in one transaction (the command's, or its own). First + /// the CAS: → Pending as a NEW generation, atomically claiming the revive (false = a + /// concurrent continue / replay already moved it). The bump is the fence: the walk that ended the run — or one the + /// reconciler gave up on that is still alive — and a stop's post-commit teardown still in flight both carry the old + /// generation, and lose to it from here on. The finish time and error go in the same statement: the revived run is + /// live again and they no longer describe it; its next terminal writes its own. + /// + /// Then what the ended attempt left pending is closed (). The bump comes + /// first because it takes the run row's lock: a park, a supervisor decision or spawn wave of the old walk either + /// committed before it, and the close sees what it staged, or is refused at its own fence on the old generation. + /// The cancelled agents' side effects — process kills, credential revokes, spend settlement — run once that commit + /// is visible (), ahead of the revived walk's dispatch. /// - private async Task ReviveTerminalRunAsync(Guid runId, Guid teamId, WorkflowRunStatus terminal, CancellationToken cancellationToken) => + private async Task ReviveTerminalRunAsync(Guid runId, Guid teamId, WorkflowRunStatus terminal, CancellationToken cancellationToken) + { + IReadOnlyList runningCancelled; + + await using (var transaction = await ScopedTransaction.OwnOrJoinAsync(_db.Database, cancellationToken).ConfigureAwait(false)) + { + if (!await BumpToNewGenerationAsync(runId, teamId, terminal, cancellationToken).ConfigureAwait(false)) return false; + + runningCancelled = await CloseEndedAttemptAsync(runId, cancellationToken).ConfigureAwait(false); + await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + } + + await FinishRunningCancelsAsync(runningCancelled, cancellationToken).ConfigureAwait(false); + + return true; + } + + /// The revive's CAS: → Pending one generation on, the finish time and error cleared (see ). + private async Task BumpToNewGenerationAsync(Guid runId, Guid teamId, WorkflowRunStatus terminal, CancellationToken cancellationToken) => await _db.WorkflowRun .Where(r => r.Id == runId && r.TeamId == teamId && r.Status == terminal) .ExecuteUpdateAsync(s => s .SetProperty(r => r.Status, WorkflowRunStatus.Pending) .SetProperty(r => r.Generation, r => r.Generation + 1) .SetProperty(r => r.CompletedAt, (DateTimeOffset?)null) - .SetProperty(r => r.Error, (string?)null), cancellationToken) - .ConfigureAwait(false) == 1; + .SetProperty(r => r.Error, (string?)null), cancellationToken).ConfigureAwait(false) == 1; + + /// + /// Close what the ended attempt left, so the revived run starts from the state the stop's teardown — or the engine's + /// failure cleanup — leaves when it runs first, whether it has run yet or not, and never from less. Every still-Pending + /// wait goes Discarded: closed unanswered, so its step parks a fresh one instead of the revived walk adopting it (a + /// re-entered map branch) or re-parking on it (a supervisor's agent barrier). Every agent still Queued or Running is + /// cancelled, and every staged child run still Pending or Enqueued: nothing the ended attempt started can be picked + /// up by a worker, reclaimed by the revived supervisor, or folded onto its tape as still running. A Running agent + /// gets only its row flipped here (); returned, so its side effects run after + /// commit. A stop's teardown landing afterwards finds nothing of the revived run's: its close is keyed by the stopped + /// attempt's own agents and children, its run-wide close is fenced on the stopped generation, and every CAS it takes + /// on what was already cancelled here simply loses — which also makes this the only kill a Continue after a Failure + /// gets, since that path has no teardown. + /// + private async Task> CloseEndedAttemptAsync(Guid runId, CancellationToken cancellationToken) + { + var stagedChildren = await StagedChildRunIdsAsync(runId, cancellationToken).ConfigureAwait(false); + + var waitsDiscarded = await DiscardPendingWaitsAsync(runId, cancellationToken).ConfigureAwait(false); + var queuedCancelled = await CancelQueuedAgentsAsync(runId, cancellationToken).ConfigureAwait(false); + var runningCancelled = await CancelRunningAgentRowsAsync(runId, cancellationToken).ConfigureAwait(false); + + await CancelStagedChildRunsAsync(stagedChildren, cancellationToken).ConfigureAwait(false); + + if (waitsDiscarded + queuedCancelled + runningCancelled.Count + stagedChildren.Count > 0) + _logger.LogInformation("Continue closed what the ended attempt left. RunId={RunId} WaitsDiscarded={WaitsDiscarded} QueuedAgentsCancelled={QueuedAgentsCancelled} RunningAgentsCancelled={RunningAgentsCancelled} StagedChildren={StagedChildren}", runId, waitsDiscarded, queuedCancelled, runningCancelled.Count, stagedChildren.Count); + + return runningCancelled; + } + + /// + /// Cancel the stopped attempt's staged child runs still Pending or Enqueued, in the revive's transaction, so no worker + /// claims one between the Continue and the stop's teardown — the same status-guarded CAS the engine's own terminal + /// cleanup takes for a staged child, which has started nothing of its own yet. One already walking is left to that + /// teardown, which cancels every child it captured through the full cancel. + /// + private async Task CancelStagedChildRunsAsync(IReadOnlyList ids, CancellationToken cancellationToken) + { + if (ids.Count == 0) return; + + await _db.WorkflowRun + .Where(r => ids.Contains(r.Id) && (r.Status == WorkflowRunStatus.Pending || r.Status == WorkflowRunStatus.Enqueued)) + .ExecuteUpdateAsync(s => s.SetProperty(r => r.Status, WorkflowRunStatus.Cancelled).SetProperty(r => r.CompletedAt, (DateTimeOffset?)DateTimeOffset.UtcNow), cancellationToken).ConfigureAwait(false); + } + + /// Close the run's still-Pending waits as Discarded, which no replay reader treats as an answer; returns how many. + private async Task DiscardPendingWaitsAsync(Guid runId, CancellationToken cancellationToken) => + await _db.WorkflowRunWait + .Where(w => w.RunId == runId && w.Status == WorkflowWaitStatuses.Pending) + .ExecuteUpdateAsync(s => s.SetProperty(w => w.Status, WorkflowWaitStatuses.Discarded).SetProperty(w => w.ResolvedAt, (DateTimeOffset?)DateTimeOffset.UtcNow), cancellationToken).ConfigureAwait(false); + + /// Cancel the run's still-Queued agent runs through the agent service's Queued-guarded CAS — one a worker claimed a moment ago is Running by then, and takes it; returns how many. + private async Task CancelQueuedAgentsAsync(Guid runId, CancellationToken cancellationToken) + { + var queued = await _db.AgentRun.AsNoTracking().Where(r => r.WorkflowRunId == runId && r.Status == AgentRunStatus.Queued).Select(r => r.Id).ToListAsync(cancellationToken).ConfigureAwait(false); + + var cancelled = 0; + + foreach (var agentId in queued) + { + if (await _agentRunService.CancelQueuedAsync(agentId, ContinuedRunAgentReason, cancellationToken).ConfigureAwait(false)) cancelled++; + } + + return cancelled; + } + + /// Flip the run's still-Running agent runs to Cancelled with the epoch-fenced CAS alone (the owning worker loses its fence and stops them), in this transaction; returns the ones flipped, whose side effects run after commit. + private async Task> CancelRunningAgentRowsAsync(Guid runId, CancellationToken cancellationToken) + { + var running = await _db.AgentRun.AsNoTracking().Where(r => r.WorkflowRunId == runId && r.Status == AgentRunStatus.Running).OrderBy(r => r.Id).Select(r => r.Id).ToListAsync(cancellationToken).ConfigureAwait(false); + + var cancelled = new List(running.Count); + + foreach (var agentId in running) + { + if (await _runningAgents.CancelRunningRowAsync(agentId, ContinuedRunRunningAgentReason, cancellationToken).ConfigureAwait(false)) cancelled.Add(agentId); + } + + return cancelled; + } + + /// + /// The rest of each Running cancel the revive flipped — expire its decisions, settle its spend claims, revoke its + /// credential, close its harness execution, kill its process — once the revive's commit is visible: joined to a + /// command, after the command commits and ahead of the revived walk's dispatch. Each is its own best-effort action, + /// so one agent's failed kill neither stops the others' nor fails the Continue that already committed. Each runs on + /// , as the stop's teardown does: it is owed to a revive that has committed, and + /// a request that went away after that commit must not leave a flipped agent with its decisions, spend claims and + /// harness execution still open. + /// + private async Task FinishRunningCancelsAsync(IReadOnlyList runningCancelled, CancellationToken cancellationToken) + { + foreach (var agentId in runningCancelled) + await _postCommit.RunAfterCommitAsync(_ => FinishRunningCancelQuietlyAsync(agentId), cancellationToken).ConfigureAwait(false); + } + + private async Task FinishRunningCancelQuietlyAsync(Guid agentId) + { + try + { + await _runningAgents.FinishRunningCancelAsync(agentId, AgentRunAbandonCause.OperatorCancelled, CancellationToken.None).ConfigureAwait(false); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "A continued run's cancelled agent {AgentRunId} could not finish its cancel; its owning worker stops it on its lost fence, and the reconciler's cancelled-agent sweep backs that up", agentId); + } + } + + /// Dispatch the revived run once the Continue's commit is visible, on like the running agents' kills registered ahead of it: a request that went away after the commit must not leave the continued run Pending with nothing to walk it until the reconciler's sweep. + private async Task DispatchRevivedRunAfterCommitAsync(Guid runId, CancellationToken cancellationToken) => + await _postCommit.RunAfterCommitAsync(_ => _runDispatcher.DispatchAsync(runId, CancellationToken.None), cancellationToken).ConfigureAwait(false); private static readonly IReadOnlyDictionary EmptyNodeBag = new Dictionary(); @@ -1155,9 +1297,17 @@ private async Task> StagedChildRunIdsAsync(Guid runId, int g .Select(w => w.Token) .ToListAsync(cancellationToken).ConfigureAwait(false); - return tokens.Select(t => Guid.TryParse(t, out var id) ? id : (Guid?)null).Where(id => id.HasValue).Select(id => id!.Value).ToList(); + return ChildRunIds(tokens); } + /// The child runs the run's still-Pending Subworkflow waits staged — read by the revive, inside its own transaction, before it discards those waits. + private async Task> StagedChildRunIdsAsync(Guid runId, CancellationToken cancellationToken) => + ChildRunIds(await _db.WorkflowRunWait.AsNoTracking().Where(w => w.RunId == runId && w.WaitKind == WorkflowWaitKinds.Subworkflow && w.Status == WorkflowWaitStatuses.Pending).Select(w => w.Token).ToListAsync(cancellationToken).ConfigureAwait(false)); + + /// A Subworkflow wait's token is its child run id; one that does not parse names no child. + private static IReadOnlyList ChildRunIds(IEnumerable tokens) => + tokens.Select(t => Guid.TryParse(t, out var id) ? id : (Guid?)null).Where(id => id.HasValue).Select(id => id!.Value).ToList(); + /// A branch agent run the stop ends, with the status it was read in (the kill-wave's first CAS is chosen by it). private sealed record BranchAgent(Guid Id, AgentRunStatus Status); @@ -1180,59 +1330,111 @@ private async Task ReReadTerminalOutcomeAsync(Guid runId, Guid } /// - /// Tear down a just-cancelled run, best-effort: close the waits the stopped attempt's agents and children answer, - /// KILL-WAVE those branch agent runs (Queued + Running), cancel those sub-workflow children through this same cancel, - /// and close the run's other still-pending waits so none dangle. Best-effort end to end — one agent's failed kill or - /// one child's failed cancel is logged and the rest of the teardown still runs (the run is already Cancelled; the - /// reconciler's parent-run-terminal guard re-cleans an agent run it missed, and the stuck-run reconciler never - /// dispatches a child still staged under a finished parent, but nothing stops a child that had already started). - /// Returns how many branch agent runs the kill-wave flipped. The work is the attempt's plus - /// whatever the old walk staged at the stopped since; the run-wide close is fenced on - /// that generation (see ). + /// Tear down a just-cancelled run: close the waits the stopped attempt's agents and children answer, KILL-WAVE those + /// branch agent runs (Queued + Running), cancel those sub-workflow children through this same cancel, and close the + /// run's other still-pending waits so none dangle. The work is the attempt's plus whatever + /// the old walk staged at the stopped since; the run-wide close is fenced on that + /// generation (see ). Returns how many branch agent runs the kill-wave flipped. + /// + /// Each step is best-effort on its own (), as is each agent's kill and each + /// child's cancel inside its step: the run is already Cancelled, so one failure never aborts the cancel, and never + /// costs what comes after it — a failed read or wait close still leaves the kill-wave to run. The reconciler's + /// parent-run-terminal guard re-cleans an agent run a failure missed, and the stuck-run reconciler never dispatches a + /// child still staged under a finished parent, but nothing stops a child that had already started. + /// + /// Neither wait close waits on a row lock: both skip a wait another transaction holds. That is how a Continue + /// landing mid-teardown is met — its revive discards every Pending wait of the run in its own transaction and holds + /// them to its commit, so the teardown never queues behind it, and the two can no longer deadlock over waits each + /// locks in its own order. A skipped wait is the revive's to close when it commits; one it releases by rolling back + /// is closed by a later pass (). /// private async Task TearDownCancelledRunAsync(Guid runId, Guid teamId, int generation, StoppedAttempt stopped, CancellationToken cancellationToken) { - try - { - var attempt = stopped.Union(await ReadStoppedAttemptAsync(runId, generation, cancellationToken).ConfigureAwait(false)); + var attempt = await TearDownStepAsync(async () => stopped.Union(await ReadStoppedAttemptAsync(runId, generation, cancellationToken).ConfigureAwait(false)), stopped, "read what the stopped walk staged since", runId).ConfigureAwait(false); + + await TearDownStepAsync(() => CloseStoppedAttemptWaitsAsync(runId, attempt, cancellationToken), "close the stopped attempt's waits", runId).ConfigureAwait(false); - await CloseStoppedAttemptWaitsAsync(runId, attempt, cancellationToken).ConfigureAwait(false); + var agentRunsCancelled = await TearDownStepAsync(() => KillWaveBranchAgentsAsync(attempt.Agents, cancellationToken), 0, "kill the stopped attempt's agents", runId).ConfigureAwait(false); - var agentRunsCancelled = await KillWaveBranchAgentsAsync(attempt.Agents, cancellationToken).ConfigureAwait(false); + await TearDownStepAsync(() => CancelChildRunsAsync(attempt.Children, teamId, cancellationToken), "cancel the stopped attempt's children", runId).ConfigureAwait(false); - await CancelChildRunsAsync(attempt.Children, teamId, cancellationToken).ConfigureAwait(false); + await TearDownStepAsync(() => CancelPendingWaitsAsync(runId, generation, cancellationToken), "close the run's remaining waits", runId).ConfigureAwait(false); - await CancelPendingWaitsAsync(runId, generation, cancellationToken).ConfigureAwait(false); + return agentRunsCancelled; + } - return agentRunsCancelled; + /// One best-effort teardown step: a failure is logged and the teardown goes on with . The reconciler's parent-run-terminal guard re-cleans an agent run a failed step missed while the run stays stopped. + private async Task TearDownStepAsync(Func> step, T fallback, string stepName, Guid runId) + { + try + { + return await step().ConfigureAwait(false); } catch (Exception ex) { - _logger.LogWarning(ex, "Run {RunId} was cancelled but tearing down its agents/children failed; the reconciler catches an orphaned agent run, not an unfinished sub-workflow", runId); - return 0; + _logger.LogWarning(ex, "Run {RunId} was cancelled but the teardown step to {Step} failed; the rest of the teardown still runs — the reconciler catches an orphaned agent run, not an unfinished sub-workflow", runId, stepName); + return fallback; } } + private async Task TearDownStepAsync(Func step, string stepName, Guid runId) => + await TearDownStepAsync(async () => { await step().ConfigureAwait(false); return true; }, true, stepName, runId).ConfigureAwait(false); + /// - /// Close the pending waits the stopped attempt's agents and children would answer, BEFORE they are killed. A Continue - /// that beat the teardown keeps the run's waits open (the fenced close in skips - /// them), and the kill lands each agent Cancelled — a result its completion hands back through the step's wait. Left - /// open, that wait became the answer the continued walk replays, and the continued step failed on the stop's own - /// kill. Closed, the step finds no answer and parks a fresh one, exactly as when the teardown ran first. Keyed by the - /// stopped attempt's own agent and child ids, so no wait the revived walk parked is among them. + /// Close the pending waits the stopped attempt's agents and children would answer, BEFORE they are killed. The kill + /// lands each agent Cancelled — a result its completion hands back through the step's wait — and, left open, that + /// wait became the answer a Continue replays, so the continued step failed on the stop's own kill. Closed, the step + /// finds no answer and parks a fresh one. A Continue that beat the teardown has already closed them in its revive (and + /// the fenced close in skips its run), so this then finds nothing left. Keyed by + /// the stopped attempt's own agent and child ids, so no wait the revived walk parked is among them. A wait another + /// transaction holds is skipped, not waited for, and passed over again once it may have cleared + /// (). /// private async Task CloseStoppedAttemptWaitsAsync(Guid runId, StoppedAttempt attempt, CancellationToken cancellationToken) { - var tokens = attempt.Agents.Select(a => a.Id).Concat(attempt.Children).Select(id => id.ToString()).ToList(); + var tokens = attempt.Agents.Select(a => a.Id).Concat(attempt.Children).Select(id => id.ToString()).ToArray(); - if (tokens.Count == 0) return; + if (tokens.Length == 0) return; - await _db.WorkflowRunWait - .Where(w => w.RunId == runId && w.Status == WorkflowWaitStatuses.Pending && tokens.Contains(w.Token)) - .ExecuteUpdateAsync(s => s - .SetProperty(w => w.Status, WorkflowWaitStatuses.Discarded) - .SetProperty(w => w.ResolvedAt, (DateTimeOffset?)DateTimeOffset.UtcNow), cancellationToken) - .ConfigureAwait(false); + Task Close() => _db.Database.ExecuteSqlInterpolatedAsync($""" + UPDATE workflow_run_wait SET status = {WorkflowWaitStatuses.Discarded}, resolved_at = {DateTimeOffset.UtcNow} + WHERE id IN (SELECT id FROM workflow_run_wait WHERE run_id = {runId} AND status = {WorkflowWaitStatuses.Pending} AND token = ANY({tokens}) FOR UPDATE SKIP LOCKED) + """, cancellationToken); + + Task StillPending() => _db.WorkflowRunWait.AsNoTracking().CountAsync(w => w.RunId == runId && w.Status == WorkflowWaitStatuses.Pending && tokens.Contains(w.Token), cancellationToken); + + await CloseWaitsSkippingHeldOnesAsync(runId, "stopped-attempt waits", Close, StillPending).ConfigureAwait(false); + } + + /// How many more passes a teardown wait close makes over waits it had to skip, and the pause before each: long enough for the transaction holding them — typically a Continue's revive — to end. + private const int SkippedWaitRetries = 3; + + private static readonly TimeSpan SkippedWaitRetryDelay = TimeSpan.FromMilliseconds(200); + + /// + /// A teardown wait close (, which skips a wait another transaction holds), passed again while + /// any of its waits is still Pending, up to more times. The holder is usually a + /// Continue's revive, which closes those waits itself when it commits. One that rolls back instead releases them + /// still Pending under the Cancelled run, where the stopped attempt's kill would answer them and a later Continue + /// replay that as the step's result — so a later pass closes them once the lock clears. What a holder keeps past the + /// last pass is logged. + /// + private async Task CloseWaitsSkippingHeldOnesAsync(Guid runId, string waits, Func close, Func> stillPending) + { + var left = 0; + + for (var pass = 0; pass <= SkippedWaitRetries; pass++) + { + if (pass > 0) await Task.Delay(SkippedWaitRetryDelay).ConfigureAwait(false); + + await close().ConfigureAwait(false); + + left = await stillPending().ConfigureAwait(false); + + if (left == 0) return; + } + + _logger.LogWarning("Run {RunId} teardown could not close {Count} {Waits}: another transaction still held them after {Passes} passes", runId, left, waits, SkippedWaitRetries + 1); } /// @@ -1329,16 +1531,21 @@ private async Task CancelChildRunQuietlyAsync(Guid childRunId, Guid teamId, Canc /// resume affordance, and — unlike Resolved, which a real answer writes — is never replayed as an answer, so /// a later Continue re-parks the step instead of feeding it the request its payload still holds. The UPDATE itself /// checks the run is still at the stopped : a wait the revived walk parked is visible to - /// it only once the Continue's bump is too, so the revived run's fresh waits are never closed. + /// it only once the Continue's bump is too, so the revived run's fresh waits are never closed. A wait another + /// transaction holds is skipped, not waited for, and passed over again once it may have cleared + /// (). /// private async Task CancelPendingWaitsAsync(Guid runId, int generation, CancellationToken cancellationToken) { - await _db.WorkflowRunWait - .Where(w => w.RunId == runId && w.Status == WorkflowWaitStatuses.Pending && _db.WorkflowRun.Any(r => r.Id == runId && r.Generation == generation)) - .ExecuteUpdateAsync(s => s - .SetProperty(w => w.Status, WorkflowWaitStatuses.Discarded) - .SetProperty(w => w.ResolvedAt, (DateTimeOffset?)DateTimeOffset.UtcNow), cancellationToken) - .ConfigureAwait(false); + Task Close() => _db.Database.ExecuteSqlInterpolatedAsync($""" + UPDATE workflow_run_wait SET status = {WorkflowWaitStatuses.Discarded}, resolved_at = {DateTimeOffset.UtcNow} + WHERE id IN (SELECT w.id FROM workflow_run_wait w WHERE w.run_id = {runId} AND w.status = {WorkflowWaitStatuses.Pending} + AND EXISTS (SELECT 1 FROM workflow_run r WHERE r.id = {runId} AND r.generation = {generation}) FOR UPDATE OF w SKIP LOCKED) + """, cancellationToken); + + Task StillPending() => _db.WorkflowRunWait.AsNoTracking().CountAsync(w => w.RunId == runId && w.Status == WorkflowWaitStatuses.Pending && _db.WorkflowRun.Any(r => r.Id == runId && r.Generation == generation), cancellationToken); + + await CloseWaitsSkippingHeldOnesAsync(runId, "remaining waits", Close, StillPending).ConfigureAwait(false); } /// Cap on a single page — clamps a caller-supplied limit so an unbounded page can't be requested. diff --git a/backend/src/CodeSpace.Core/Services/Workflows/WorkflowServiceDependencies.cs b/backend/src/CodeSpace.Core/Services/Workflows/WorkflowServiceDependencies.cs index 9f1fc711b..3317f8aec 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/WorkflowServiceDependencies.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/WorkflowServiceDependencies.cs @@ -38,9 +38,10 @@ public WorkflowLaunchServices(IRunStarter starter, IRunFromSnapshotStarter snaps public sealed class WorkflowControlServices : IScopedDependency { - public WorkflowControlServices(IWorkflowResumeService resume, IAgentRunService agents, IRerunCellSeeder cells, IRunCancellationRegistry cancellation) { Resume = resume; Agents = agents; Cells = cells; Cancellation = cancellation; } + public WorkflowControlServices(IWorkflowResumeService resume, IAgentRunService agents, IRunningAgentCancellation runningAgents, IRerunCellSeeder cells, IRunCancellationRegistry cancellation) { Resume = resume; Agents = agents; RunningAgents = runningAgents; Cells = cells; Cancellation = cancellation; } public IWorkflowResumeService Resume { get; } public IAgentRunService Agents { get; } + public IRunningAgentCancellation RunningAgents { get; } public IRerunCellSeeder Cells { get; } public IRunCancellationRegistry Cancellation { get; } } diff --git a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs index 3c1164df0..0cc358224 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Infrastructure/PostgresFixture.cs @@ -433,6 +433,11 @@ private static void RegisterTestAssemblyTypes(ContainerBuilder builder) // (resolved AsSelf by CodeSpaceModule), so the live brain drives the real engine. Same InstancePerLifetimeScope // lifetime + last-wins position, so the supervisor node's own DI scope resolves whichever the flag selects. builder.RegisterType().AsSelf().SingleInstance(); + + // A decision-log hold a test arms per run (SupervisorDecisionScript.HoldDecisionLog), reachable inside the + // supervisor node's own scope, which the engine resolves from this root where a test's child-scope decorator + // cannot reach. Pass-through for every run nothing is armed for. + builder.RegisterDecorator((c, _, inner) => new Workflows.Infrastructure.ScriptedDecisionLog(inner, c.Resolve(), c.Resolve())); builder.RegisterType().AsSelf().SingleInstance(); builder.RegisterType().AsSelf().InstancePerLifetimeScope(); builder.Register(c => diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/ContinueParkedRunFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/ContinueParkedRunFlowTests.cs index 8534961ca..24f82a300 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/ContinueParkedRunFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/ContinueParkedRunFlowTests.cs @@ -1,3 +1,4 @@ +using System.Data.Common; using System.Text.Json; using Autofac; using CodeSpace.Core.Middlewares.Transactional; @@ -14,6 +15,8 @@ using CodeSpace.Messages.Enums; using MediatR; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Npgsql; using Shouldly; namespace CodeSpace.IntegrationTests.Workflows; @@ -28,7 +31,8 @@ namespace CodeSpace.IntegrationTests.Workflows; /// a map branch, a loop pass) through the operator stop's teardown, and one through the engine's own teardown when a /// run lands Failure beside a parked step — plus one where the stop's teardown lands only after the Continue has /// re-parked every step, and must leave the revived run's fresh waits, agent and child alone, and one where it lands -/// before the continued walk starts, and the agent it kills must not answer the continued step. +/// before the continued walk starts, and the agent it kills must not answer the continued step. Two more pin the teardown +/// itself: it never waits on a Continue holding the stopped attempt's waits, and no failed step costs it its kill-wave. /// /// Fidelity (Rule 12) 🟢 high for everything under test: the REAL WorkflowService cancel (terminal flip plus /// its post-commit teardown) and the REAL engine terminal cleanup, which are what close the waits; the REAL Continue for @@ -162,6 +166,38 @@ public async Task Continuing_a_map_stopped_while_its_branch_agent_was_parked_re_ branchWait.Token.ShouldNotBe(firstBranchWait.Token, "the branch staged a NEW agent run on a fresh wait"); } + [Fact] + public async Task A_stop_teardown_landing_after_a_continue_never_leaves_a_map_branch_parked_on_the_wait_it_closes() + { + // The Continue lands before the stop's teardown closed the branch's wait. The revived walk re-entered the branch, + // found that wait still open, and parked the branch on it as its own; the teardown then closed it, and the branch + // sat parked on a closed wait until the reconciler re-dispatched the run. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var workflowId = await CreateWorkflowAsync(teamId, userId, MapOverCappedAgentDefinition()); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId, payloadJson: """{ "things": ["a"] }"""); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); + var stoppedWait = (await WaitsAsync(runId)).Single(w => w.IterationKey == "map#0" && w.WaitKind == WorkflowWaitKinds.AgentRun); + stoppedWait.Status.ShouldBe(WorkflowWaitStatuses.Pending, "precondition: the branch agent parked"); + + using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); + await ContinueAsync(runId, teamId); + await RunEngineAsync(runId); + await stop.Resolve().RunAllAsync(CancellationToken.None); // the stop's teardown lands only now + + using var verify = _fixture.BeginScope(); + var db = verify.Resolve(); + + var branchWait = await db.WorkflowRunWait.AsNoTracking().SingleAsync(w => w.RunId == runId && w.IterationKey == "map#0"); + branchWait.Status.ShouldBe(WorkflowWaitStatuses.Pending, customMessage: $"the branch is parked on an open wait after the late teardown — Discarded means it adopted the stopped attempt's wait. Records: {await StepVerdictsAsync(db, runId)}"); + branchWait.Token.ShouldNotBe(stoppedWait.Token, "the branch staged a fresh agent instead of adopting the stopped attempt's"); + (await db.AgentRun.AsNoTracking().SingleAsync(r => r.Id == Guid.Parse(branchWait.Token))).Status.ShouldBe(AgentRunStatus.Queued, "the late teardown left the fresh branch agent alone"); + (await db.AgentRun.AsNoTracking().SingleAsync(r => r.Id == Guid.Parse(stoppedWait.Token))).Status.ShouldBe(AgentRunStatus.Cancelled, "the stopped attempt's branch agent is ended"); + (await db.WorkflowRun.AsNoTracking().SingleAsync(r => r.Id == runId)).Status.ShouldBe(WorkflowRunStatus.Suspended, "the revived run is parked on the branch's fresh wait"); + } + [Fact] public async Task Continuing_a_loop_stopped_while_its_body_approval_was_parked_re_parks_that_pass() { @@ -263,6 +299,11 @@ public async Task A_stop_teardown_landing_after_a_continue_ends_the_stopped_atte using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); await ContinueAsync(runId, teamId); + + using (var afterRevive = _fixture.BeginScope()) + (await afterRevive.Resolve().WorkflowRun.AsNoTracking().SingleAsync(r => r.Id == stoppedChildId)).Status + .ShouldBe(WorkflowRunStatus.Cancelled, "the revive cancelled the stopped attempt's staged child with the waits it closed — no worker can claim it while the teardown is still to come"); + await RunEngineAsync(runId); var fresh = (await WaitsAsync(runId)).Where(w => w.Status == WorkflowWaitStatuses.Pending).ToList(); @@ -289,9 +330,9 @@ public async Task A_stop_teardown_landing_after_a_continue_ends_the_stopped_atte .ShouldBe(WorkflowRunStatus.Suspended, "the revived run is still parked on its fresh waits"); (await db.AgentRun.AsNoTracking().SingleAsync(r => r.Id == stoppedAgentId)).Status - .ShouldBe(AgentRunStatus.Cancelled, "the stopped attempt's agent was live when the stop committed — the late teardown still ends it"); + .ShouldBe(AgentRunStatus.Cancelled, "the stopped attempt's agent was live when the stop committed — it stays ended"); (await db.WorkflowRun.AsNoTracking().SingleAsync(r => r.Id == stoppedChildId)).Status - .ShouldBe(WorkflowRunStatus.Cancelled, "and cancels the stopped attempt's staged child run"); + .ShouldBe(WorkflowRunStatus.Cancelled, "and so does the stopped attempt's staged child run"); } [Fact] @@ -333,8 +374,148 @@ public async Task A_stopped_attempts_agent_killed_after_a_continue_never_answers .ShouldNotBe(stoppedAgentWait.Token, "the continued step staged a fresh agent"); } + [Fact] + public async Task A_continue_holding_the_stopped_attempts_waits_never_stalls_the_stop_teardown_short_of_its_kill_wave() + { + // The revive discards every pending wait of the run in its own transaction, holding those rows to its commit, while + // the stop's teardown closes the waits of the same stopped attempt. Each statement locked the rows in its own order, + // so the two could deadlock — and a teardown picked as the victim gave up before its kill-wave. The teardown's wait + // closes now skip a wait another transaction holds: they never wait on the revive, so the kill-wave always runs. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var workflowId = await CreateWorkflowAsync(teamId, userId, CappedAgentBesideApprovalDefinition()); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); + var stoppedWaits = (await WaitsAsync(runId)).Where(w => w.Status == WorkflowWaitStatuses.Pending).ToList(); + stoppedWaits.Count.ShouldBe(2, "precondition: the agent and the approval parked"); + var stoppedAgentId = Guid.Parse(stoppedWaits.Single(w => w.WaitKind == WorkflowWaitKinds.AgentRun).Token); + await MarkRunningAsync(stoppedAgentId); // a worker claimed it + + using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); + + var discarded = new HeldCommand(text => text.StartsWith("UPDATE", StringComparison.Ordinal) && text.Contains("workflow_run_wait"), afterItRuns: true); + using var continueScope = StopContinueSignals.InterceptedScope(_fixture, discarded); + var revive = continueScope.Resolve().ContinueRunAsync(runId, teamId, CancellationToken.None); + + try + { + await StopContinueSignals.AwaitAsync(discarded.Reached.Task, "the Continue's revive holding every pending wait of the run, its commit still to come"); + await StopContinueSignals.AwaitAsync(stop.Resolve().RunAllAsync(CancellationToken.None), "the stop's teardown finishing while the revive holds the waits"); + + (await AgentStatusAsync(stoppedAgentId)).ShouldBe(AgentRunStatus.Cancelled, "its kill-wave ran: the stopped attempt's running agent is ended"); + } + finally + { + discarded.Release.TrySetResult(); + } + + await StopContinueSignals.AwaitAsync(revive, "the Continue returning"); + (await revive).ShouldBeTrue("the run continued in place"); + + var closed = await WaitStatusesAsync(stoppedWaits); + closed.Count.ShouldBe(2); + closed.ShouldAllBe(s => s == WorkflowWaitStatuses.Discarded, "the waits the teardown skipped, the revive closed"); + } + + [Fact] + public async Task A_teardown_step_that_fails_never_costs_the_kill_wave() + { + // The teardown ran as one best-effort block: a failure closing the stopped attempt's waits — a deadlock victim, a + // dropped connection — abandoned it before its kill-wave, and the stopped attempt's running agent ran on. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var workflowId = await CreateWorkflowAsync(teamId, userId, CappedAgentBesideApprovalDefinition()); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); + var stoppedWaits = (await WaitsAsync(runId)).Where(w => w.Status == WorkflowWaitStatuses.Pending).ToList(); + stoppedWaits.Count.ShouldBe(2, "precondition: the agent and the approval parked"); + var stoppedAgentId = Guid.Parse(stoppedWaits.Single(w => w.WaitKind == WorkflowWaitKinds.AgentRun).Token); + await MarkRunningAsync(stoppedAgentId); + + var victim = new DeadlockVictim(text => text.Contains("SKIP LOCKED") && text.Contains("token = ANY")); // the close of the stopped attempt's own waits + + using (var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId, victim)) + await stop.Resolve().RunAllAsync(CancellationToken.None); + + victim.Fired.ShouldBeTrue("precondition: the teardown's close of the stopped attempt's waits failed"); + (await AgentStatusAsync(stoppedAgentId)).ShouldBe(AgentRunStatus.Cancelled, "the kill-wave after it still ran"); + + var closed = await WaitStatusesAsync(stoppedWaits); + closed.Count.ShouldBe(2); + closed.ShouldAllBe(s => s == WorkflowWaitStatuses.Discarded, "and the run-wide close after that still closed the waits the failed step missed"); + } + + [Fact] + public async Task A_wait_the_teardown_had_to_skip_is_closed_once_the_transaction_holding_it_lets_go() + { + // The teardown skips a wait another transaction holds — a Continue's revive, which closes it itself. When that + // transaction rolled back instead of committing, the skipped wait stayed Pending under the Cancelled run, for the + // stopped attempt's kill to answer and a later Continue to replay as the step's result. The teardown's closes now + // pass over the waits they skipped again, and one released in time is closed. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var workflowId = await CreateWorkflowAsync(teamId, userId, CappedAgentBesideApprovalDefinition()); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); + var stoppedWaits = (await WaitsAsync(runId)).Where(w => w.Status == WorkflowWaitStatuses.Pending).ToList(); + stoppedWaits.Count.ShouldBe(2, "precondition: the agent and the approval parked"); + var agentWait = stoppedWaits.Single(w => w.WaitKind == WorkflowWaitKinds.AgentRun); + + using var holderScope = _fixture.BeginScope(); // a second connection holds the agent's wait, as a revive about to roll back does + var holderDb = holderScope.Resolve(); + await using var holder = await holderDb.Database.BeginTransactionAsync(); + await holderDb.Database.ExecuteSqlInterpolatedAsync($"SELECT 1 FROM workflow_run_wait WHERE id = {agentWait.Id} FOR UPDATE"); + + var lastClose = new HeldCommand(text => text.Contains("SKIP LOCKED") && text.Contains("EXISTS"), afterItRuns: true); // the teardown's run-wide close, its first pass done + using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId, lastClose); + var teardown = stop.Resolve().RunAllAsync(CancellationToken.None); + + try + { + await StopContinueSignals.AwaitAsync(lastClose.Reached.Task, "the teardown's last wait close, having skipped the held wait"); + + await holder.RollbackAsync(); // the holder lets go without closing it + } + finally + { + lastClose.Release.TrySetResult(); + } + + await StopContinueSignals.AwaitAsync(teardown, "the teardown finishing"); + + var closed = await WaitStatusesAsync(stoppedWaits); + closed.Count.ShouldBe(2); + closed.ShouldAllBe(s => s == WorkflowWaitStatuses.Discarded, "a later pass closed the wait once its lock cleared, so no kill can answer it"); + } + // ─── Helpers ──────────────────────────────────────────────────────────────────── + private async Task MarkRunningAsync(Guid agentRunId) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().MarkRunningAsync(agentRunId, CancellationToken.None); + } + + private async Task AgentStatusAsync(Guid agentRunId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().AgentRun.AsNoTracking().Where(r => r.Id == agentRunId).Select(r => r.Status).SingleAsync(); + } + + private async Task> WaitStatusesAsync(IEnumerable waits) + { + var ids = waits.Select(w => w.Id).ToList(); + + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunWait.AsNoTracking().Where(w => ids.Contains(w.Id)).Select(w => w.Status).ToListAsync(); + } + private async Task NotifyAgentEndedAsync(Guid agentRunId) { using var scope = _fixture.BeginScope(); @@ -535,4 +716,20 @@ private InMemoryBackgroundJobClient ResolveJobClient() new() { From = "ls", To = "gate" }, }, }; + + /// Fails the first command accepts the way Postgres fails a deadlock victim (40P01), once. + private sealed class DeadlockVictim(Func matches) : DbCommandInterceptor + { + private int _fired; + + public bool Fired => Volatile.Read(ref _fired) == 1; + + public override ValueTask> NonQueryExecutingAsync(DbCommand command, CommandEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + if (matches(command.CommandText) && Interlocked.Exchange(ref _fired, 1) == 0) + throw new PostgresException("deadlock detected", "ERROR", "ERROR", PostgresErrorCodes.DeadlockDetected); + + return ValueTask.FromResult(result); + } + } } diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/GatedAgentParkNode.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/GatedAgentParkNode.cs index 32725de1d..edd760495 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/GatedAgentParkNode.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/GatedAgentParkNode.cs @@ -9,11 +9,13 @@ namespace CodeSpace.IntegrationTests.Workflows.Infrastructure; /// -/// Test-only node whose FIRST pass holds on a gate (the handshake) and then parks on an -/// AgentRun wait — the suspend an agent.run step produces, from which the engine stages a real agent run. It -/// opens the window of a step still in flight when its run is stopped and continued: the step parks only when the test -/// lets it, after the revived walk has parked the same cell. A resumed pass completes. Registered through -/// PostgresFixture.RegisterTestAssemblyTypes; NOT in any IPluginModule, so it never reaches the editor palette. +/// Test-only node whose FIRST pass holds on a gate (the handshake) and then parks: on an +/// AgentRun wait by default — the suspend an agent.run step produces, from which the engine stages a real agent +/// run — or, with the "wait": "Action" input, on an Action wait that stages nothing, which a step can still park +/// after its run was stopped. It opens the window of a step still in flight when its run is stopped and continued: the +/// step parks only when the test lets it, after the revived walk has parked the same cell or while the Continue waits +/// on the park. A resumed pass completes. Registered through PostgresFixture.RegisterTestAssemblyTypes; NOT in +/// any IPluginModule, so it never reaches the editor palette. /// public sealed class GatedAgentParkNode : INodeRuntime { @@ -54,6 +56,11 @@ public async Task RunAsync(NodeRunContext context, CancellationToken await gate.Release.Task.WaitAsync(cancellationToken).ConfigureAwait(false); } + // "wait": "Action" parks a wait with no staged child instead — one a step can still park after its run was stopped, + // where agent admission refuses a new agent under the terminal run. + if (context.Inputs.TryGetValue("wait", out var wait) && wait.GetString() == WorkflowWaitKinds.Action) + return NodeResult.Suspend(new SuspensionToken { Kind = WorkflowWaitKinds.Action, Payload = JsonSerializer.SerializeToElement(new { }) }); + var task = new AgentTask { Goal = "Fix the failing billing tests", Harness = "codex-cli", Model = "gpt-5.3-codex", RunnerKind = "local" }; return NodeResult.Suspend(new SuspensionToken { Kind = WorkflowWaitKinds.AgentRun, Payload = JsonSerializer.SerializeToElement(task, AgentJson.Options) }); diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/ScriptedSupervisorDecider.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/ScriptedSupervisorDecider.cs index 4ae4078e0..b1e126694 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/ScriptedSupervisorDecider.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/ScriptedSupervisorDecider.cs @@ -28,8 +28,18 @@ public sealed class ScriptedSupervisorDecider : ISupervisorDecider /// The question the AskHumanStop arc asks at turn 0 — the integration test asserts the posted card body + the recorded outcome carry it. public const string AskQuestion = "which approach: rewrite or patch?"; - public Task DecideAsync(SupervisorTurnContext context, CancellationToken cancellationToken) + public async Task DecideAsync(SupervisorTurnContext context, CancellationToken cancellationToken) { + // A test holds a turn mid-decision — the slow model call is where a stop and a continue most likely land — and can + // have the held turn come back with a decision of its own, as a real model asked twice may. + if (_script.TakeDecisionHold(context.SupervisorRunId) is { } hold) + { + hold.Gate.Started.TrySetResult(); + await hold.Gate.Release.Task.WaitAsync(cancellationToken).ConfigureAwait(false); + + if (hold.Decides is { } decides) return decides; + } + // A test injects a TRANSIENT (retryable) infra fault on a specific turn — thrown BEFORE any decision is produced, // so the production RetryingSupervisorDeciderDecorator that wraps this scripted decider must retry + recover it. if (_script.TryConsumeTransientFault(context.TurnNumber)) @@ -57,7 +67,7 @@ public Task DecideAsync(SupervisorTurnContext context, Cance _ => PlanThenStop(context), }; - return Task.FromResult(decision); + return decision; } // E5 arc: turn 0 plan(2) → EVERY later turn spawn(both). The decider NEVER stops on its own — it's the @@ -310,6 +320,12 @@ private static SupervisorDecision PlanConfirmReactive(SupervisorTurnContext cont }, }); + /// A spawn of the given plan units, in the scripted decider's own canonical shape — for a held turn that decides differently from the script. + public static SupervisorDecision SpawnOf(params string[] subtaskIds) => Canonical(SupervisorDecisionKinds.Spawn, new SupervisorSpawnPayload { SubtaskIds = subtaskIds }); + + /// A server-authored gate question, as the publish or delivery gate substitutes one for a model's decision — the scripted decider stands in for the whole decider pipeline, so it may pose one. Unflagged, the ask clamp would strip the gate's reserved prefix as a model posing as a server card. + public static SupervisorDecision GateAskOf(string question) => Canonical(SupervisorDecisionKinds.AskHuman, new SupervisorAskHumanPayload { Question = question }) with { ServerAuthored = true }; + private static SupervisorDecision Canonical(string kind, TPayload payload) => new() { Kind = kind, @@ -327,6 +343,39 @@ public sealed class SupervisorDecisionScript private readonly Dictionary _transientFaults = new(); + private readonly System.Collections.Concurrent.ConcurrentDictionary _decisionHolds = new(); + + /// Hold the next decision 's supervisor makes until the test releases the returned gate: the decider signals , awaits , then decides — or as scripted, when none is given. One-shot, and keyed by run, so no sibling test's run is ever held. + public CancelGateNode.Gate HoldNextDecision(Guid runId, SupervisorDecision? decides = null) + { + var gate = new CancelGateNode.Gate(); + _decisionHolds[runId] = new DecisionHold(gate, decides); + return gate; + } + + /// Take the hold armed for , if any (called by the scripted decider on each decide). + public DecisionHold? TakeDecisionHold(Guid runId) => _decisionHolds.TryRemove(runId, out var hold) ? hold : null; + + /// A held decision: the gate the decider waits on, and the decision the held turn comes back with (null → as scripted). + public sealed record DecisionHold(CancelGateNode.Gate Gate, SupervisorDecision? Decides); + + /// The next decision 's supervisor makes is , whatever the script says: a hold released before it is reached. + public void DecideNext(Guid runId, SupervisorDecision decides) => HoldNextDecision(runId, decides).Release.TrySetResult(); + + private readonly System.Collections.Concurrent.ConcurrentDictionary _decisionLogHolds = new(); + + /// Hold 's supervisor once at in its decision log, in whichever walk reaches it first — the engine's own included, through the fixture-root . + public DecisionLogHold HoldDecisionLog(Guid runId, DecisionLogStep at) => _decisionLogHolds[runId] = new DecisionLogHold(at); + + /// Whether any run has a decision-log hold armed. + public bool AnyDecisionLogHold => !_decisionLogHolds.IsEmpty; + + /// The decision-log hold armed for , if any. + public DecisionLogHold? DecisionLogHoldFor(Guid runId) => _decisionLogHolds.TryGetValue(runId, out var hold) ? hold : null; + + /// Disarm 's decision-log hold (its hold fired). + public void DisarmDecisionLogHold(Guid runId) => _decisionLogHolds.TryRemove(runId, out _); + /// Inject a transient (retryable) brain-call fault on a specific TURN: the next decide invocations for that turn throw a Transient LlmApiException before any decision is produced, then it proceeds — driving the production retry decorator that wraps the scripted decider. UNDER the in-call retry budget → the decorator recovers in place; AT/OVER it → the exhausted fault escapes and the node's infra park (P1.1) engages. public void FailTransientlyOnTurn(int turn, int times) => _transientFaults[turn] = times; diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/StopContinueTestKit.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/StopContinueTestKit.cs new file mode 100644 index 000000000..f7a3eeb5f --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/StopContinueTestKit.cs @@ -0,0 +1,242 @@ +using System.Data.Common; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Supervisor; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Agents; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; + +namespace CodeSpace.IntegrationTests.Workflows.Infrastructure; + +/// +/// The hold points the stop / continue interleaving suites put a writer on: a named signal awaited with a bound, a +/// session seen blocked on a row lock, a database command held once before or after it runs, and a decision-log call +/// held once around its commit. Every hold is one-shot and fires only in the scope it is registered in. +/// +public static class StopContinueSignals +{ + public static readonly TimeSpan Timeout = TimeSpan.FromSeconds(30); + + /// Awaits for at most , failing with the signal's name. + public static async Task AwaitAsync(Task signal, string name) + { + try { await signal.WaitAsync(Timeout).ConfigureAwait(false); } + catch (TimeoutException) { throw new TimeoutException($"Timed out after {Timeout.TotalSeconds}s waiting for {name}."); } + } + + /// + /// Waits, bounded, until session is blocked on a lock — the signal a lock handoff proceeds on. + /// Fails at once when , the call running on that session, finishes first: it never queued + /// behind the lock it was meant to wait for, and there is nothing left to wait on. + /// + public static async Task WaitForLockWaitAsync(PostgresFixture fixture, int pid, string signal, Task writer) + { + using var scope = fixture.BeginScope(); + var db = scope.Resolve(); + var deadline = DateTime.UtcNow + Timeout; + + while (!await db.Database.SqlQuery($"SELECT EXISTS (SELECT 1 FROM pg_stat_activity WHERE pid = {pid} AND wait_event_type = 'Lock') AS \"Value\"").SingleAsync().ConfigureAwait(false)) + { + if (writer.IsCompleted) + throw new InvalidOperationException($"The writer on session {pid} finished without ever waiting for {signal}: nothing held the lock it should have queued behind."); + + if (DateTime.UtcNow > deadline) + throw new TimeoutException($"Timed out after {Timeout.TotalSeconds}s waiting for {signal}. Diagnose with: psql -c \"SELECT wait_event_type, wait_event, query FROM pg_stat_activity WHERE pid = {pid}\""); + + await Task.Delay(10).ConfigureAwait(false); + } + } + + /// A scope whose database commands pass through , over the fixture's own context options. + public static ILifetimeScope InterceptedScope(PostgresFixture fixture, params IInterceptor[] interceptors) => InterceptedScope(fixture, _ => { }, interceptors); + + /// An intercepted scope with further registrations of the caller's. + public static ILifetimeScope InterceptedScope(PostgresFixture fixture, Action configure, params IInterceptor[] interceptors) + { + DbContextOptions baseOptions; + using (var root = fixture.BeginScope()) + baseOptions = root.Resolve>(); + + var options = new DbContextOptionsBuilder(baseOptions).AddInterceptors(interceptors).Options; + + return fixture.BeginScope(builder => + { + builder.RegisterInstance(options).As>().SingleInstance(); + configure(builder); + }); + } +} + +/// Holds the first database command accepts, once: before it runs, or — with — after, with its locks taken and its transaction still open. Signals , then waits for . +public sealed class HeldCommand(Func matches, bool afterItRuns = false) : DbCommandInterceptor +{ + private int _fired; + + public TaskCompletionSource Reached { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public TaskCompletionSource Release { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public override async ValueTask> ReaderExecutingAsync(DbCommand command, CommandEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + if (!afterItRuns) await HoldIfMatchedAsync(command).ConfigureAwait(false); + return result; + } + + public override async ValueTask ReaderExecutedAsync(DbCommand command, CommandExecutedEventData eventData, DbDataReader result, CancellationToken cancellationToken = default) + { + if (afterItRuns) await HoldIfMatchedAsync(command).ConfigureAwait(false); + return result; + } + + public override async ValueTask> NonQueryExecutingAsync(DbCommand command, CommandEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + if (!afterItRuns) await HoldIfMatchedAsync(command).ConfigureAwait(false); + return result; + } + + public override async ValueTask NonQueryExecutedAsync(DbCommand command, CommandExecutedEventData eventData, int result, CancellationToken cancellationToken = default) + { + if (afterItRuns) await HoldIfMatchedAsync(command).ConfigureAwait(false); + return result; + } + + private async Task HoldIfMatchedAsync(DbCommand command) + { + if (!matches(command.CommandText) || Interlocked.Exchange(ref _fired, 1) == 1) return; + + Reached.TrySetResult(); + await Release.Task.ConfigureAwait(false); + } +} + +/// The decision-log step a holds a supervisor turn at. +public enum DecisionLogStep +{ + /// Once the decision's claim row has committed, still Pending. + AfterClaim, + + /// Once the decision has committed Running, before its side effect. + AfterBegin, + + /// Once the side effect has committed, before the decision's terminal record is written. + BeforeTerminal, +} + +/// +/// Holds a supervisor turn once at : signals , then waits for +/// . puts it on a scope's decision log; armed through +/// it holds the run's next walk instead, the engine's own included. +/// +public sealed class DecisionLogHold(DecisionLogStep at) +{ + private int _fired; + + public TaskCompletionSource Reached { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public TaskCompletionSource Release { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public void Decorate(ContainerBuilder builder) => builder.RegisterDecorator((_, _, inner) => new HeldDecisionLog(inner, this)); + + /// Whether the call at is the one to hold: the named step, reached for the first time. + internal bool TryFire(DecisionLogStep step) => step == at && Interlocked.Exchange(ref _fired, 1) == 0; + + internal async Task HoldAsync() + { + Reached.TrySetResult(); + await Release.Task.ConfigureAwait(false); + } + + internal async Task HoldAtAsync(DecisionLogStep step) + { + if (TryFire(step)) await HoldAsync().ConfigureAwait(false); + } +} + +/// The real decision log with the calls names held around their commit; everything reaches the real log unchanged. +public sealed class HeldDecisionLog(ISupervisorDecisionLog inner, DecisionLogHold hold) : ISupervisorDecisionLog +{ + public async Task TryClaimAsync(SupervisorDecisionClaimRequest request, CancellationToken cancellationToken) + { + var claim = await inner.TryClaimAsync(request, cancellationToken).ConfigureAwait(false); + await hold.HoldAtAsync(DecisionLogStep.AfterClaim).ConfigureAwait(false); + return claim; + } + + public async Task TryBeginExecutionAsync(Guid decisionId, Guid teamId, CancellationToken cancellationToken) + { + var began = await inner.TryBeginExecutionAsync(decisionId, teamId, cancellationToken).ConfigureAwait(false); + await hold.HoldAtAsync(DecisionLogStep.AfterBegin).ConfigureAwait(false); + return began; + } + + public async Task RecordTerminalAsync(Guid decisionId, Guid teamId, SupervisorDecisionStatus status, string? outcomeJson, string? error, CancellationToken cancellationToken) + { + await hold.HoldAtAsync(DecisionLogStep.BeforeTerminal).ConfigureAwait(false); + await inner.RecordTerminalAsync(decisionId, teamId, status, outcomeJson, error, cancellationToken).ConfigureAwait(false); + } + + public Task> GetForRunAsync(Guid supervisorRunId, Guid teamId, CancellationToken cancellationToken) => inner.GetForRunAsync(supervisorRunId, teamId, cancellationToken); + + public Task> GetTerminalDecisionsAsync(Guid supervisorRunId, Guid teamId, CancellationToken cancellationToken) => inner.GetTerminalDecisionsAsync(supervisorRunId, teamId, cancellationToken); + + public Task UpdateOutcomeAsync(Guid decisionId, Guid teamId, string foldedOutcomeJson, CancellationToken cancellationToken) => inner.UpdateOutcomeAsync(decisionId, teamId, foldedOutcomeJson, cancellationToken); + + public Task ExpireStalePendingAsync(DateTimeOffset olderThan, CancellationToken cancellationToken) => inner.ExpireStalePendingAsync(olderThan, cancellationToken); +} + +/// +/// The fixture-root decoration of the real decision log, so a test can hold a turn inside the supervisor node's own scope +/// — the engine's walk resolves it from the root container, where a test's child-scope decorator cannot reach. Pass-through +/// for every run nothing is armed for (); a hold, once it fires, +/// is disarmed. +/// +public sealed class ScriptedDecisionLog(ISupervisorDecisionLog inner, SupervisorDecisionScript script, CodeSpaceDbContext db) : ISupervisorDecisionLog +{ + public async Task TryClaimAsync(SupervisorDecisionClaimRequest request, CancellationToken cancellationToken) + { + var claim = await inner.TryClaimAsync(request, cancellationToken).ConfigureAwait(false); + await HoldAsync(request.SupervisorRunId, DecisionLogStep.AfterClaim).ConfigureAwait(false); + return claim; + } + + public async Task TryBeginExecutionAsync(Guid decisionId, Guid teamId, CancellationToken cancellationToken) + { + var began = await inner.TryBeginExecutionAsync(decisionId, teamId, cancellationToken).ConfigureAwait(false); + await HoldForDecisionAsync(decisionId, DecisionLogStep.AfterBegin, cancellationToken).ConfigureAwait(false); + return began; + } + + public async Task RecordTerminalAsync(Guid decisionId, Guid teamId, SupervisorDecisionStatus status, string? outcomeJson, string? error, CancellationToken cancellationToken) + { + await HoldForDecisionAsync(decisionId, DecisionLogStep.BeforeTerminal, cancellationToken).ConfigureAwait(false); + await inner.RecordTerminalAsync(decisionId, teamId, status, outcomeJson, error, cancellationToken).ConfigureAwait(false); + } + + public Task> GetForRunAsync(Guid supervisorRunId, Guid teamId, CancellationToken cancellationToken) => inner.GetForRunAsync(supervisorRunId, teamId, cancellationToken); + + public Task> GetTerminalDecisionsAsync(Guid supervisorRunId, Guid teamId, CancellationToken cancellationToken) => inner.GetTerminalDecisionsAsync(supervisorRunId, teamId, cancellationToken); + + public Task UpdateOutcomeAsync(Guid decisionId, Guid teamId, string foldedOutcomeJson, CancellationToken cancellationToken) => inner.UpdateOutcomeAsync(decisionId, teamId, foldedOutcomeJson, cancellationToken); + + public Task ExpireStalePendingAsync(DateTimeOffset olderThan, CancellationToken cancellationToken) => inner.ExpireStalePendingAsync(olderThan, cancellationToken); + + /// The run a decision belongs to is read only while some hold is armed, so an unarmed fixture pays nothing. + private async Task HoldForDecisionAsync(Guid decisionId, DecisionLogStep step, CancellationToken cancellationToken) + { + if (!script.AnyDecisionLogHold) return; + + var runId = await db.SupervisorDecisionRecord.AsNoTracking().Where(d => d.Id == decisionId).Select(d => d.SupervisorRunId).SingleOrDefaultAsync(cancellationToken).ConfigureAwait(false); + + await HoldAsync(runId, step).ConfigureAwait(false); + } + + private async Task HoldAsync(Guid runId, DecisionLogStep step) + { + if (script.DecisionLogHoldFor(runId) is not { } hold || !hold.TryFire(step)) return; + + script.DisarmDecisionLogHold(runId); + await hold.HoldAsync().ConfigureAwait(false); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/WorkflowsTestSeed.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/WorkflowsTestSeed.cs index 926e222ed..7db5c1869 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/WorkflowsTestSeed.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/WorkflowsTestSeed.cs @@ -296,11 +296,12 @@ public static async Task SeedManualRunAsync(PostgresFixture fixture, Guid /// /// Stop a run inside a command transaction and commit it WITHOUT draining: the stop's teardown stays queued on the /// returned scope's post-commit actions until the caller runs them (IPostCommitActions.RunAllAsync), standing - /// in for a teardown still in flight when the next operator action lands. The caller disposes the scope. + /// in for a teardown still in flight when the next operator action lands. The caller disposes the scope. The stop and + /// its teardown pass their commands through , when given. /// - public static async Task StopWithTeardownHeldAsync(PostgresFixture fixture, Guid runId, Guid teamId) + public static async Task StopWithTeardownHeldAsync(PostgresFixture fixture, Guid runId, Guid teamId, params Microsoft.EntityFrameworkCore.Diagnostics.IInterceptor[] interceptors) { - var scope = fixture.BeginScope(); + var scope = interceptors.Length == 0 ? fixture.BeginScope() : StopContinueSignals.InterceptedScope(fixture, interceptors); var db = scope.Resolve(); await using (var transaction = await db.Database.BeginTransactionAsync().ConfigureAwait(false)) diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/OperatorCancelInProgressWalkFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/OperatorCancelInProgressWalkFlowTests.cs index 3ca8a7b88..cea08e81d 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/OperatorCancelInProgressWalkFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/OperatorCancelInProgressWalkFlowTests.cs @@ -514,6 +514,58 @@ public async Task A_step_overtaken_mid_park_undoes_what_it_staged_and_leaves_the .ShouldBe(new[] { AgentRunStatus.Cancelled }, "the one agent the overtaken step staged was cancelled when its park was refused"); } + [Fact] + public async Task A_park_holding_the_run_lock_when_a_continue_lands_is_waited_for_then_closed_by_the_revive() + { + // The stopped walk's step parks after the stop, still at the stopped generation, and holds its share lock on the + // run row as the Continue lands. The revive waits for that park to commit and only then closes what the stopped + // attempt left pending — the park included. Closing before the bump would miss the park still in flight and leave + // its wait open under the revived run, for the revived walk to adopt (a map branch re-entered) or leave dangling. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var gateKey = Guid.NewGuid().ToString("N"); + var gate = GatedAgentParkNode.Arm(gateKey); + + var workflowId = await CreateWorkflowAsync(teamId, userId, GatedAgentParkDefinition(gateKey, WorkflowWaitKinds.Action)); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + + var heldPark = new HeldParkLockFault(); + var stoppedWalk = WalkWithFaultInBackground(runId, heldPark); + await StopContinueSignals.AwaitAsync(gate.Started.Task, "the walk's step reaching its gate"); + + await StopAsync(runId, teamId); // on this host; the walk runs on another, so only its park's fence can see it + + using var continueScope = _fixture.BeginScope(); + Task? revive = null; + + try + { + heldPark.Arm(); + gate.Release.TrySetResult(); + await StopContinueSignals.AwaitAsync(heldPark.Reached.Task, "the step's park taking its share lock on the run row"); // nothing of the park written yet + + var continueDb = continueScope.Resolve(); + await continueDb.Database.OpenConnectionAsync(); + var continuePid = await continueDb.Database.SqlQueryRaw("SELECT pg_backend_pid() AS \"Value\"").SingleAsync(); + revive = continueScope.Resolve().ContinueRunAsync(runId, teamId, CancellationToken.None); + + await StopContinueSignals.WaitForLockWaitAsync(_fixture, continuePid, "the Continue's revive to block on the parking step's lock on the run row", revive); + } + finally + { + heldPark.Release.TrySetResult(); + } + + await StopContinueSignals.AwaitAsync(revive, "the Continue returning once the park committed"); + (await revive).ShouldBeTrue("the run continued in place"); + await StopContinueSignals.AwaitAsync(stoppedWalk, "the stopped walk returning after its park"); + + using var verify = _fixture.BeginScope(); + var db = verify.Resolve(); + + (await db.WorkflowRunWait.AsNoTracking().SingleAsync(w => w.RunId == runId && w.NodeId == "park")).Status + .ShouldBe(WorkflowWaitStatuses.Discarded, "the revive closed the park it waited for — the stopped attempt's, committed just before the bump"); + } + [Fact] public async Task A_child_walk_a_continue_overtook_does_not_wake_the_parent_with_a_cancel_as_it_unwinds() { @@ -679,6 +731,28 @@ public override async ValueTask ReaderExecutedAsync(DbCommand comm private static bool IsGenerationCheck(DbCommand command) => command.CommandText.Contains("FROM workflow_run AS") && command.CommandText.Contains("generation <>"); } + /// Once armed, holds the scope's next park right after its share lock on the run row is granted — the park's transaction open and holding the lock, nothing of the park written yet. + private sealed class HeldParkLockFault : DbCommandInterceptor + { + private int _armed; + + public TaskCompletionSource Reached { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public TaskCompletionSource Release { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public void Arm() => Interlocked.Exchange(ref _armed, 1); + + public override async ValueTask ReaderExecutedAsync(DbCommand command, CommandExecutedEventData eventData, DbDataReader result, CancellationToken cancellationToken = default) + { + if (!command.CommandText.Contains("FOR SHARE") || Interlocked.CompareExchange(ref _armed, 0, 1) != 1) return result; + + Reached.TrySetResult(); + await Release.Task.ConfigureAwait(false); + + return result; + } + } + /// /// The body-level twin of the top-level overtaken-walk test: the old walk (on another host) holds mid-body in the gate; /// the run is stopped and continued; the revived walk re-runs the container and holds in the same gate; the old walk's @@ -794,14 +868,14 @@ private async Task RunDetailAsync(Guid runId, Guid teamId) }, }; - // start → park (holds on its gate, then parks on an AgentRun wait) → end. - private static WorkflowDefinition GatedAgentParkDefinition(string gateKey) => new() + // start → park (holds on its gate, then parks on an AgentRun wait — or on the wait kind named) → end. + private static WorkflowDefinition GatedAgentParkDefinition(string gateKey, string wait = WorkflowWaitKinds.AgentRun) => new() { SchemaVersion = 1, Nodes = new List { new() { Id = "start", TypeKey = "trigger.manual", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, - new() { Id = "park", TypeKey = GatedAgentParkNode.Key, Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.Json($$"""{ "gate": "{{gateKey}}" }""") }, + new() { Id = "park", TypeKey = GatedAgentParkNode.Key, Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.Json($$"""{ "gate": "{{gateKey}}", "wait": "{{wait}}" }""") }, new() { Id = "end", TypeKey = "builtin.terminal", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, }, Edges = new List diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorAskHumanStopContinueFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorAskHumanStopContinueFlowTests.cs new file mode 100644 index 000000000..58c13a050 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorAskHumanStopContinueFlowTests.cs @@ -0,0 +1,397 @@ +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Chat; +using CodeSpace.Core.Services.Supervisor; +using CodeSpace.Core.Services.Workflows; +using CodeSpace.Core.Services.Workflows.Engine; +using CodeSpace.Core.Services.Workflows.Lifecycle; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Infrastructure.Jobs; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Commands.Workflows; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Dtos.Chat; +using CodeSpace.Messages.Dtos.Agents; +using CodeSpace.Messages.Dtos.Chat.Interactions; +using CodeSpace.Messages.Dtos.Workflows; +using CodeSpace.Messages.Enums; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// Stop then Continue while a supervisor turn is asking a human. The question — its card, when the run has a +/// conversation — its wait and the ask's decision record commit as one transaction, behind the same share lock on the run +/// at the walk's claimed generation as the spawn wave. A turn the Continue overtook writes none of the three once the +/// revive has moved the run on; a Continue landing while a question is being asked waits for all three, then closes the +/// wait like any other of the stopped attempt's, to be re-opened, same question, by the revived run. Either way the +/// question anyone can see has its token on the decision tape, which is what the run's ask API answers from. +/// +/// Fidelity 🟢 high: the REAL turn service, executor, chat bot, message service and ask answer service, the REAL +/// cancel and Continue, the REAL engine for the revived walk, over real Postgres; the scripted decider stands in for the +/// model. The overtaken turn is driven through the turn service directly, under its walk's claim on the run's +/// generation, because the supervisor node resolves its own scope from the root container where a test's hold on the bot +/// or the decision log cannot reach; its node start is recorded the way that walk's engine records it. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class SupervisorAskHumanStopContinueFlowTests : IDisposable +{ + private const string Goal = "ship the feature"; + + private readonly PostgresFixture _fixture; + + public SupervisorAskHumanStopContinueFlowTests(PostgresFixture fixture) + { + _fixture = fixture; + + using var scope = _fixture.BeginScope(); + scope.Resolve().AskHumanStop(); // turn 0 asks, turn 1 stops echoing the answer + } + + public void Dispose() + { + using var scope = _fixture.BeginScope(); + scope.Resolve().PlanThenStop(); // restore the default for sibling tests + } + + [Fact] + public async Task An_ask_a_continue_overtook_before_its_question_posts_no_card_parks_no_wait_and_records_nothing() + { + // The overtaken turn had begun its ask before the stop, and reached its card only after the Continue. Card and wait + // were written unfenced: a person could answer a question the revived run never asked, and the revived supervisor + // re-parked on that wait as its own. + var (teamId, userId, conversationId) = await SeedTeamWithConversationAsync(); + var runId = await SeedAskRunAsync(teamId, userId, conversationId); + + using var manual = ResolveJobClient().ManualExecution(); + + await RecordSupervisorStartedAsync(runId); + + var begun = new DecisionLogHold(DecisionLogStep.AfterBegin); // the ask claimed and begun, its question not yet asked + var overtakenTurn = RunOvertakenTurnInBackground(runId, teamId, conversationId, begun.Decorate); + + try + { + await StopContinueSignals.AwaitAsync(begun.Reached.Task, "the overtaken turn beginning its ask"); + + await StopAsync(runId, teamId); + await ContinueAsync(runId, teamId); + } + finally + { + begun.Release.TrySetResult(); + } + + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenTurn, "the overtaken turn returning"))).ShouldBeOfType("the overtaken turn stood down at its question's fence"); + (await CardCountAsync(conversationId)).ShouldBe(0, "it posted no card"); + (await AskWaitsAsync(runId)).ShouldBeEmpty("parked no wait"); + (await AskDecisionAsync(runId, teamId)).Status.ShouldBe(SupervisorDecisionStatus.Running, "and recorded nothing: the ask it began before the stop is still in flight, for the revived walk to finish"); + + await RunEngineAsync(runId); // the revived walk finishes the ask: its own card, its own wait, its own record + + (await CardCountAsync(conversationId)).ShouldBe(1, "exactly one card — the revived run's"); + var wait = (await AskWaitsAsync(runId)).ShouldHaveSingleItem("exactly one wait"); + wait.Status.ShouldBe(WorkflowWaitStatuses.Pending, "and the revived run is parked on it"); + SupervisorOutcome.ReadHumanWaitToken((await AskDecisionAsync(runId, teamId)).OutcomeJson).ShouldBe(wait.Token, "the ask's record names that question"); + (await NodeFailuresAsync(runId)).ShouldBe(0, "standing down is not recorded as the supervisor step failing"); + + await AnswerThroughTheAskApiAsync(runId, teamId, userId, "patch it"); + } + + [Fact] + public async Task A_continue_landing_while_an_ask_posts_its_card_waits_for_it_and_the_revived_run_answers_from_that_card() + { + // The Continue lands after the overtaken ask took its fence, while its card is being posted. The ask's record used to + // be written after the card, on its own: the revive's bump, let through once the card committed, refused it, the + // decision stayed in flight with no question on it, and the revived run re-parked on the card's re-opened wait — + // where the run's ask API, reading the question off the tape, found nothing to answer. + var (teamId, userId, conversationId) = await SeedTeamWithConversationAsync(); + var runId = await SeedAskRunAsync(teamId, userId, conversationId); + + using var manual = ResolveJobClient().ManualExecution(); + + await RecordSupervisorStartedAsync(runId); + await StopAsync(runId, teamId); // the stop lands first; the overtaken turn, on another host, asks on + + var posting = new HeldBot(); // inside the question's fenced transaction + var overtakenTurn = RunOvertakenTurnInBackground(runId, teamId, conversationId, b => b.RegisterDecorator((_, _, inner) => posting.Wrap(inner))); + + await ContinueWhileTheQuestionIsAskedAsync(runId, teamId, posting.Reached.Task, posting.Release, "the overtaken turn posting its card under its fence"); + + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenTurn, "the overtaken turn returning"))).ShouldBeNull("its card, wait and record committed together before the bump — nothing of the turn was left to refuse"); + + var closed = (await AskWaitsAsync(runId)).ShouldHaveSingleItem("the card's wait committed with it"); + closed.Status.ShouldBe(WorkflowWaitStatuses.Discarded, "and the revive, let through after it, closed it"); + SupervisorOutcome.ReadHumanWaitToken((await AskDecisionAsync(runId, teamId)).OutcomeJson).ShouldBe(closed.Token, "the ask's record committed with its card, naming it"); + + await RunEngineAsync(runId); // the revived walk re-opens the posted question and parks on it + + var reopened = (await AskWaitsAsync(runId)).ShouldHaveSingleItem(); + reopened.Id.ShouldBe(closed.Id, "the same wait"); + reopened.Status.ShouldBe(WorkflowWaitStatuses.Pending, "re-opened, so the card already posted answers it"); + (await CardCountAsync(conversationId)).ShouldBe(1, "no second card was posted"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Suspended, "the revived run is parked on the question"); + (await NodeFailuresAsync(runId)).ShouldBe(0, "neither walk recorded the supervisor step failing"); + + await AnswerThroughTheAskApiAsync(runId, teamId, userId, "patch it"); + } + + [Fact] + public async Task A_gate_question_asked_with_no_conversation_as_a_continue_lands_stays_answerable_through_the_ask_api() + { + // A gate question in a run with no conversation posts no card: its wait is answered only through the run's ask API, + // which reads the question off the decision tape. The ask's record, written apart from its wait, was refused behind + // the revive's bump; the revived run re-parked on the re-opened wait with no question on the tape, and nothing could + // ever answer it — the run stuck, and a stop and Continue asked the same way again. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedAskRunAsync(teamId, userId, conversationId: null); + + using var manual = ResolveJobClient().ManualExecution(); + + await RecordSupervisorStartedAsync(runId); + await StopAsync(runId, teamId); // the stop lands first; the overtaken turn, on another host, asks on + ResolveScript().DecideNext(runId, ScriptedSupervisorDecider.GateAskOf(GateQuestion)); + + var recording = new DecisionLogHold(DecisionLogStep.BeforeTerminal); // its wait staged, its record about to be written + var overtakenTurn = RunOvertakenTurnInBackground(runId, teamId, conversationId: null, recording.Decorate); + + await ContinueWhileTheQuestionIsAskedAsync(runId, teamId, recording.Reached.Task, recording.Release, "the overtaken turn recording its gate question under its fence"); + + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenTurn, "the overtaken turn returning"))).ShouldBeNull("its wait and record committed together before the bump"); + + var closed = (await AskWaitsAsync(runId)).ShouldHaveSingleItem("the question's wait committed with its record"); + closed.Status.ShouldBe(WorkflowWaitStatuses.Discarded, "and the revive closed it"); + + await RunEngineAsync(runId); // the revived walk re-opens the question and parks on it + + (await AskWaitsAsync(runId)).ShouldHaveSingleItem().Status.ShouldBe(WorkflowWaitStatuses.Pending, "the same question, re-opened"); + + await AnswerThroughTheAskApiAsync(runId, teamId, userId, "publish it"); + } + + /// + /// Land a Continue while an overtaken turn is held inside its question's fenced transaction: it blocks on the share + /// lock that transaction holds on the run row, and returns once the question committed. + /// + private async Task ContinueWhileTheQuestionIsAskedAsync(Guid runId, Guid teamId, Task held, TaskCompletionSource release, string heldSignal) + { + using var continueScope = _fixture.BeginScope(); + Task? revive = null; + + try + { + await StopContinueSignals.AwaitAsync(held, heldSignal); + + var continueDb = continueScope.Resolve(); + await continueDb.Database.OpenConnectionAsync(); + var continuePid = await continueDb.Database.SqlQueryRaw("SELECT pg_backend_pid() AS \"Value\"").SingleAsync(); + + revive = continueScope.Resolve().ContinueRunAsync(runId, teamId, CancellationToken.None); + await StopContinueSignals.WaitForLockWaitAsync(_fixture, continuePid, "the Continue's revive to block on the question's share lock on the run row", revive); + } + finally + { + release.TrySetResult(); + } + + await StopContinueSignals.AwaitAsync(revive, "the Continue returning once the question committed"); + (await revive).ShouldBeTrue("the run continued in place"); + } + + /// + /// Answer the run's newest ask through the run's ask API — which reads the question off the decision tape, not the + /// wait — and walk the revived run on: the answer folds into the ask, and the next turn stops on it. + /// + private async Task AnswerThroughTheAskApiAsync(Guid runId, Guid teamId, Guid userId, string answer) + { + SupervisorAskAnswerOutcome? answered; + using (var scope = _fixture.BeginScope()) + answered = await scope.Resolve().AnswerAsync(runId, teamId, userId, answer, decision: null, CancellationToken.None); + + answered.ShouldNotBeNull("the run's ask API found the question on the decision tape").Resumed.ShouldBeTrue("and its answer resumed the run"); + + await RunEngineAsync(runId); // the answered ask folds, and turn 1 stops on it + + SupervisorOutcome.ReadAskHumanAnswer((await AskDecisionAsync(runId, teamId)).OutcomeJson).ShouldBe(answer, "the answer folded into the ask"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Success, "and the run completed on it"); + } + + /// The overtaken walk's turn, on another host: the REAL turn service under that walk's claim on the run's current generation, in a scope with registered. + private Task RunOvertakenTurnInBackground(Guid runId, Guid teamId, Guid? conversationId, Action hold) => Task.Run(async () => + { + var generation = await GenerationAsync(runId); + + using var scope = _fixture.BeginScope(hold); + using var claim = RunGenerationFence.Claim(runId, generation); + + await scope.Resolve().RunTurnAsync(runId, teamId, "sup", Goal, conversationId, GoalConfig(conversationId), CancellationToken.None); + }); + + /// The overtaken walk started the supervisor step before the stop — recorded the way its engine records a node start, so the Continue finds the step to resume. + private async Task RecordSupervisorStartedAsync(Guid runId) + { + var none = new Dictionary(); + + using var scope = _fixture.BeginScope(); + await scope.Resolve().NodeStartedAsync(runId, "sup", WorkflowIterationKeys.TopLevel, none, none, CancellationToken.None); + } + + private async Task<(Guid TeamId, Guid UserId, Guid ConversationId)> SeedTeamWithConversationAsync() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + + using var scope = _fixture.BeginScope(); + var slug = "sup-ask-stop-" + Guid.NewGuid().ToString("N")[..8]; + var conversationId = await scope.Resolve().CreateChannelAsync(teamId, slug, slug, isPrivate: false, userId, CancellationToken.None); + + return (teamId, userId, conversationId); + } + + private async Task SeedAskRunAsync(Guid teamId, Guid userId, Guid? conversationId) + { + Guid workflowId; + using (var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin)) + workflowId = await scope.Resolve().Send(new CreateWorkflowCommand + { + Name = "sup-ask-stop-" + Guid.NewGuid().ToString("N")[..6], + Description = null, + Definition = SupervisorDefinition(conversationId), + Activations = new List(), + Enabled = true, + }); + + return await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + } + + private async Task RunEngineAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + } + + private async Task StopAsync(Guid runId, Guid teamId) + { + using var scope = _fixture.BeginScope(); + (await scope.Resolve().CancelRunAsync(runId, teamId, CancellationToken.None))!.Cancelled.ShouldBeTrue("the run was stopped"); + } + + private async Task ContinueAsync(Guid runId, Guid teamId) + { + using var scope = _fixture.BeginScope(); + (await scope.Resolve().ContinueRunAsync(runId, teamId, CancellationToken.None)).ShouldBeTrue("the stopped run continues in place"); + } + + private async Task GenerationAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRun.AsNoTracking().Where(r => r.Id == runId).Select(r => r.Generation).SingleAsync(); + } + + private async Task CardCountAsync(Guid conversationId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().Message.AsNoTracking().IgnoreQueryFilters().CountAsync(m => m.ConversationId == conversationId && m.InteractionJson != null && m.DeletedDate == null); + } + + private async Task> AskWaitsAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunWait.AsNoTracking().Where(w => w.RunId == runId && w.WaitKind == WorkflowWaitKinds.Action).ToListAsync(); + } + + private async Task AskDecisionAsync(Guid runId, Guid teamId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().SupervisorDecisionRecord.AsNoTracking().SingleAsync(d => d.SupervisorRunId == runId && d.TeamId == teamId && d.DecisionKind == SupervisorDecisionKinds.AskHuman); + } + + private async Task RunStatusAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRun.AsNoTracking().Where(r => r.Id == runId).Select(r => r.Status).SingleAsync(); + } + + private async Task NodeFailuresAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunRecord.AsNoTracking().CountAsync(r => r.RunId == runId && r.NodeId == "sup" && (r.RecordType == WorkflowRunRecordTypes.NodeFailed || r.RecordType == WorkflowRunRecordTypes.AttemptFailed)); + } + + private SupervisorDecisionScript ResolveScript() + { + using var scope = _fixture.BeginScope(); + return scope.Resolve(); + } + + private InMemoryBackgroundJobClient ResolveJobClient() + { + using var scope = _fixture.BeginScope(); + return scope.Resolve(); + } + + /// A server-authored gate question's shape (the pinned I3 publish-gate prefix): with no conversation it parks on its wait instead of degrading. + private const string GateQuestion = "I3 publish gate: the run has accepted work that could not be published — a human must resolve this"; + + private static string SupervisorConfig(Guid? conversationId) => conversationId is { } id ? $$"""{"goal":"{{Goal}}","conversationId":"{{id}}"}""" : $$"""{"goal":"{{Goal}}"}"""; + + /// The goal config the supervisor node reads off its own config, for a turn driven through the turn service directly. + private static SupervisorGoalConfig? GoalConfig(Guid? conversationId) => Core.Services.Workflows.Nodes.Builtin.AgentSupervisorNode.ReadGoalConfig(JsonSerializer.Deserialize>(SupervisorConfig(conversationId))!); + + // manual → sup (agent.supervisor, asking into a team conversation) → terminal. Shadow completion, as the ask suites run + // it: the scripted stop stakes no contract, and completion arbitration is not this suite's subject. + private static WorkflowDefinition SupervisorDefinition(Guid? conversationId) => new() + { + SchemaVersion = 1, + CompletionMode = WorkflowDefinition.CompletionModeShadow, + Nodes = new List + { + new() { Id = "start", TypeKey = "trigger.manual", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "sup", TypeKey = "agent.supervisor", Config = WorkflowsTestSeed.Json(SupervisorConfig(conversationId)), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "end", TypeKey = "builtin.terminal", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + }, + Edges = new List + { + new() { From = "start", To = "sup" }, + new() { From = "sup", To = "end" }, + }, + }; + + /// The real chat bot with its card post held, once — the post runs inside the question's fenced transaction. + private sealed class HeldBot + { + private int _fired; + + public TaskCompletionSource Reached { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public TaskCompletionSource Release { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public IChatBotService Wrap(IChatBotService inner) => new Held(inner, this); + + private async Task HoldAsync() + { + if (Interlocked.Exchange(ref _fired, 1) == 1) return; + + Reached.TrySetResult(); + await Release.Task.ConfigureAwait(false); + } + + private sealed class Held(IChatBotService inner, HeldBot hold) : IChatBotService + { + public Task GetOrCreateTeamBotAsync(Guid teamId, CancellationToken cancellationToken) => inner.GetOrCreateTeamBotAsync(teamId, cancellationToken); + + public async Task PostAsBotAsync(Guid conversationId, string body, MessageInteraction? interaction, CancellationToken cancellationToken) + { + await hold.HoldAsync().ConfigureAwait(false); + return await inner.PostAsBotAsync(conversationId, body, interaction, cancellationToken).ConfigureAwait(false); + } + + public Task ConversationBelongsToTeamAsync(Guid conversationId, Guid teamId, CancellationToken cancellationToken) => inner.ConversationBelongsToTeamAsync(conversationId, teamId, cancellationToken); + } + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorStopContinueFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorStopContinueFlowTests.cs new file mode 100644 index 000000000..b341cead1 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/SupervisorStopContinueFlowTests.cs @@ -0,0 +1,843 @@ +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Middlewares.Transactional; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Agents; +using CodeSpace.Core.Services.Supervisor; +using CodeSpace.Core.Services.Workflows; +using CodeSpace.Core.Services.Workflows.Engine; +using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Infrastructure.Jobs; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Commands.Workflows; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Decisions; +using CodeSpace.Messages.Dtos.Agents; +using CodeSpace.Messages.Dtos.Workflows; +using CodeSpace.Messages.Enums; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// Stop then Continue while a supervisor wave is in flight. A stop reaches only a walk on the host that took it, and its +/// teardown runs after its commit, so a Continue can land while the old walk is still deciding, claiming, staging or +/// recording its next wave, and before the teardown has ended the stopped wave. The revived run must own none of it. The +/// old walk's every write — its decision's claim, begin and terminal record, and its wave — takes the engine park's share +/// lock on the run at the generation that walk claimed, so it stands down the moment the revive has moved the run on; and +/// the revive ends what the stopped attempt left — its waits, its queued and running agents — so the revived turn folds +/// only finished work, stages a wave of its own, and the late teardown finds nothing of it to end. +/// +/// Every case runs capped and uncapped: under a cost cap the wave's budget admission rides in the same fenced +/// transaction, so an overtaken turn reserves nothing, and a closed wave staged afresh keeps the reservations its slots +/// were first admitted under instead of being refused as a different intent. +/// +/// Fidelity 🟢 high: the REAL engine for every walk (the overtaken one on its own cancellation registry, as on +/// another host, so the stop never trips it), the REAL supervisor node, turn service, executor and budget ledger, the REAL +/// cancel (its flip, and its post-commit teardown held back the way a slow kill-wave holds it) and the REAL Continue, over +/// real Postgres. The scripted decider stands in for the model, and holds the overtaken turn mid-decision, the window a +/// stop and a continue most likely land in. The binary-less harness never runs (ManualExecution()). The cases that +/// hold an overtaken turn past its decision drive that turn through the turn service directly, under the walk's claim on +/// the run's generation, because the supervisor node resolves its own scope from the root container where a test's +/// decision-log hold or command interceptor cannot reach. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class SupervisorStopContinueFlowTests : IDisposable +{ + private const string Goal = "ship the feature"; + + private const decimal CapUsd = 10m; + + private readonly PostgresFixture _fixture; + + public SupervisorStopContinueFlowTests(PostgresFixture fixture) + { + _fixture = fixture; + + using var scope = _fixture.BeginScope(); + scope.Resolve().PlanThenSpawnForever(); // turn 0 plans, every later turn spawns both units + } + + public void Dispose() + { + using var scope = _fixture.BeginScope(); + scope.Resolve().PlanThenStop(); // restore the default for sibling tests + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_turn_a_continue_overtook_while_it_decided_stages_nothing_and_the_revived_turn_stages_the_wave(bool capped) + { + // The old walk's turn is mid-decision when the run is stopped and continued, and decides only after the revive. Its + // decision used to be claimed under the revived run, which then replayed it as its own — and, before that, its wave + // committed there too. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans and parks on its self-advance + await ResolveSelfAdvanceAsync(runId); + + var deciding = ResolveScript().HoldNextDecision(runId); + var overtakenWalk = WalkOnAnotherHostInBackground(runId); + await StopContinueSignals.AwaitAsync(deciding.Started.Task, "the old walk's turn 1 reaching its decision"); + + await StopAsync(runId, teamId); + await ContinueAsync(runId, teamId); + + deciding.Release.TrySetResult(); + var overtaken = await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenWalk, "the overtaken walk returning")); + + overtaken.ShouldBeNull("the overtaken walk stood down at its claim, writing nothing"); + (await DecisionKindsAsync(runId, teamId)).ShouldBe(new[] { SupervisorDecisionKinds.Plan }, "the overtaken turn claimed no decision under the revived run"); + (await AgentIdsAsync(runId)).ShouldBeEmpty("staged no agent"); + (await AgentWaitsAsync(runId)).ShouldBeEmpty("nor any agent wait"); + (await ReservationKeysAsync(runId)).ShouldBeEmpty("nor reserved any budget"); + (await NodeFailuresAsync(runId, "sup")).ShouldBe(0, "standing down is not recorded as the supervisor step failing"); + + await RunEngineAsync(runId); // the revived walk decides turn 1 itself + + var wave = await AgentWaitsAsync(runId); + wave.Select(w => w.IterationKey).ShouldBe(WaveKeys(1), ignoreOrder: true, "the revived turn staged the wave itself"); + wave.ShouldAllBe(w => w.Status == WorkflowWaitStatuses.Pending, "and parked on it"); + + var waveAgents = wave.Select(w => Guid.Parse(w.Token)).ToList(); + (await AgentIdsAsync(runId)).ShouldBe(waveAgents, ignoreOrder: true, "the only agents under the run are the revived wave's"); + (await AgentStatusesAsync(waveAgents)).ShouldBe(new[] { AgentRunStatus.Queued, AgentRunStatus.Queued }, "queued for their own execution"); + SupervisorOutcome.ReadStagedAgentRunIds((await SpawnDecisionAsync(runId, teamId)).OutcomeJson).ShouldBe(waveAgents, ignoreOrder: true, "the spawn decision records the wave the revived turn staged"); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1) : Array.Empty(), ignoreOrder: true, "a capped run holds one reservation per unit of the revived wave, and nothing more"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Suspended, "the revived run is parked on its wave"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_turn_a_continue_overtook_claims_nothing_after_the_revived_turn_decided_otherwise(bool capped) + { + // The revived walk decides turn 1 first, and only then does the overtaken turn come back from its decision — with a + // different wave, as a model asked twice may. Its decision carries a key of its own, so nothing but the fence stands + // in its way: it used to land as a second turn-1 decision, left in flight for the revived run to replay at its next + // turn as a duplicate spawn. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + + var deciding = ResolveScript().HoldNextDecision(runId, ScriptedSupervisorDecider.SpawnOf(ScriptedSupervisorDecider.SubtaskA)); + var overtakenWalk = WalkOnAnotherHostInBackground(runId); + await StopContinueSignals.AwaitAsync(deciding.Started.Task, "the old walk's turn 1 reaching its decision"); + + await StopAsync(runId, teamId); + await ContinueAsync(runId, teamId); + await RunEngineAsync(runId); // the revived walk decides turn 1 — both units — stages them and parks + + var revivedWave = await AgentWaitsAsync(runId); + revivedWave.Count.ShouldBe(2, "precondition: the revived turn staged its two-agent wave"); + var revivedSpawn = await SpawnDecisionAsync(runId, teamId); + + deciding.Release.TrySetResult(); // the overtaken turn now decides a one-unit wave and claims it + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenWalk, "the overtaken walk returning"))).ShouldBeNull("the overtaken walk stood down at its claim"); + + var decisions = await DecisionsAsync(runId, teamId); + decisions.Select(d => d.Id).ShouldBe(new[] { decisions[0].Id, revivedSpawn.Id }, "the overtaken decision was never inserted: the tape holds the plan and the revived turn's spawn, nothing else"); + decisions.ShouldAllBe(d => SupervisorDecisionStateMachine.IsTerminal(d.Status), "and no decision is left in flight for a later turn to replay"); + (await NodeFailuresAsync(runId, "sup")).ShouldBe(0, "standing down is not recorded as the supervisor step failing"); + + (await WaitStatusesAsync(revivedWave)).ShouldBe(new[] { WorkflowWaitStatuses.Pending, WorkflowWaitStatuses.Pending }, "the revived wave is untouched"); + (await AgentIdsAsync(runId)).Count.ShouldBe(2, "and no agent exists beyond it"); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1) : Array.Empty(), ignoreOrder: true, "a capped run holds the revived wave's reservations only"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Suspended, "the revived run is parked on its wave"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_stop_teardown_landing_after_a_continue_finds_nothing_of_the_revived_wave_to_end(bool capped) + { + // Stop during a wave (one agent claimed by a worker, one still queued), then a Continue before the stop's teardown + // ran. The revived supervisor re-parked on the stopped wave's open waits, or reclaimed its queued agent for its own + // next wave; the teardown then closed those waits and ended those agents, and the revived run sat parked on nothing + // until the reconciler re-dispatched it. And the running agent, still running when the revived turn folded its wave, + // stood on the tape as "Running" for good. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + await RunEngineAsync(runId); // turn 1 spawns both units and parks on them + + var stoppedWave = await AgentWaitsAsync(runId); + stoppedWave.Count.ShouldBe(2, "precondition: turn 1 parked on a two-agent wave"); + var claimedAgent = Guid.Parse(stoppedWave.Single(w => w.IterationKey == "sup#turn1#0").Token); + var queuedAgent = Guid.Parse(stoppedWave.Single(w => w.IterationKey == "sup#turn1#1").Token); + await MarkRunningAsync(claimedAgent); // a worker claimed one of them + var question = await SeedOpenAgentDecisionAsync(teamId, claimedAgent); // and it has a question out + + using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); + await ContinueAsync(runId, teamId); + + var closedWaits = await WaitStatusesAsync(stoppedWave); + closedWaits.Count.ShouldBe(2, "the stopped wave's waits are still there"); + closedWaits.ShouldAllBe(s => s == WorkflowWaitStatuses.Discarded, "the revive closed them, so nothing re-parks on them"); + (await AgentStatusesAsync(new[] { claimedAgent, queuedAgent })).ShouldBe(new[] { AgentRunStatus.Cancelled, AgentRunStatus.Cancelled }, "the revive ended both — the queued one, so no reclaim can take it, and the running one, so no revived turn folds it as still running"); + (await DecisionStatusAsync(question)).ShouldBe(ToolCallLedgerStatus.Expired, "the running agent's cancel finished once the revive committed: its open question left the queue with it"); + + await RunEngineAsync(runId); // the revived walk: turn 2 folds turn 1's wave and spawns its own + await stop.Resolve().RunAllAsync(CancellationToken.None); // the stop's teardown lands only now + + await AssertParkedOnARevivedWaveAsync(runId, "sup#turn2#", claimedAgent, queuedAgent); + + var folded = SupervisorOutcome.ReadAgentResults((await SpawnDecisionsAsync(runId, teamId))[0].OutcomeJson); + folded.Count.ShouldBe(2, "turn 1's spawn folded a result for each of its agents"); + folded.ShouldAllBe(r => r.Status == nameof(AgentRunStatus.Cancelled), "each as it ended — never as still running"); + (await AgentStatusesAsync(new[] { claimedAgent, queuedAgent })).ShouldBe(new[] { AgentRunStatus.Cancelled, AgentRunStatus.Cancelled }, "the late teardown's CAS on what the revive already ended simply lost"); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1).Concat(WaveKeys(2)).ToArray() : Array.Empty(), ignoreOrder: true, "a capped run admitted the revived wave on reservations of its own"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_stopped_wave_whose_decision_was_still_in_flight_is_staged_afresh_rather_than_re_parked_on(bool capped) + { + // The stop landed between the wave's commit and its spawn decision's terminal record, so the Continue replays that + // decision. The replay found the wave's rows — closed by the stop — and re-parked on them as a wave already + // staged, where nothing would ever answer. Under a cost cap the fresh staging was refused as over budget: its + // slots' reservations exist, and a re-reservation computes a new deadline, which the ledger reads as a new intent. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + await StageWaveBehindAnInterruptedDecisionAsync(runId, teamId, capped); + + var stoppedWave = await AgentWaitsAsync(runId); + stoppedWave.Count.ShouldBe(2, "precondition: turn 1 committed its two-agent wave"); + var stoppedAgents = stoppedWave.Select(w => Guid.Parse(w.Token)).ToArray(); + (await SpawnDecisionAsync(runId, teamId)).Status.ShouldBe(SupervisorDecisionStatus.Running, "precondition: the spawn decision was still in flight"); + + using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); + await ContinueAsync(runId, teamId); + + (await AgentStatusesAsync(stoppedAgents)).ShouldBe(new[] { AgentRunStatus.Cancelled, AgentRunStatus.Cancelled }, "the revive cancelled the stopped wave's agents — never dispatched, both still queued"); + + await RunEngineAsync(runId); // the revived walk replays the in-flight spawn + await stop.Resolve().RunAllAsync(CancellationToken.None); + + await AssertParkedOnARevivedWaveAsync(runId, "sup#turn1#", stoppedAgents); + (await SpawnDecisionAsync(runId, teamId)).Status.ShouldBe(SupervisorDecisionStatus.Succeeded, "the replay finished the spawn decision on its fresh wave"); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1) : Array.Empty(), ignoreOrder: true, "the closed slots kept the reservations they were first admitted under — the fresh wave is neither refused nor reserved twice"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_stopped_wave_staged_afresh_keeps_the_agent_that_had_already_finished(bool capped) + { + // One agent of the stopped wave had finished and answered its wait before the stop. The fresh staging of the closed + // wave dropped that answered wait with the closed one and ran the finished unit a second time. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + await StageWaveBehindAnInterruptedDecisionAsync(runId, teamId, capped); + + var stoppedWave = await AgentWaitsAsync(runId); + stoppedWave.Count.ShouldBe(2, "precondition: turn 1 committed its two-agent wave"); + var finishedWait = stoppedWave.Single(w => w.IterationKey == "sup#turn1#0"); + var finishedAgent = Guid.Parse(finishedWait.Token); + var closedAgent = Guid.Parse(stoppedWave.Single(w => w.IterationKey == "sup#turn1#1").Token); + await SimulateAgentCompletionAsync(finishedAgent); + (await WaitStatusesAsync(new[] { finishedWait })).ShouldBe(new[] { WorkflowWaitStatuses.Resolved }, "precondition: the first unit finished and its wait holds its answer"); + + using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); + await ContinueAsync(runId, teamId); + await RunEngineAsync(runId); // the revived walk replays the in-flight spawn + await stop.Resolve().RunAllAsync(CancellationToken.None); + + var waits = await AgentWaitsAsync(runId); + waits.Select(w => w.IterationKey).ShouldBe(WaveKeys(1), ignoreOrder: true, "one wait per slot of the wave"); + + var kept = waits.Single(w => w.IterationKey == "sup#turn1#0"); + kept.Id.ShouldBe(finishedWait.Id, "the finished slot keeps its own answered wait"); + kept.Status.ShouldBe(WorkflowWaitStatuses.Resolved); + + var restaged = waits.Single(w => w.IterationKey == "sup#turn1#1"); + restaged.Status.ShouldBe(WorkflowWaitStatuses.Pending, "only the slot the stop closed was staged afresh"); + var freshAgent = Guid.Parse(restaged.Token); + freshAgent.ShouldNotBe(closedAgent, "on an agent of its own"); + + (await AgentStatusesAsync(new[] { finishedAgent })).ShouldBe(new[] { AgentRunStatus.Succeeded }, "the finished agent is left as it ended"); + (await AgentIdsAsync(runId)).ShouldBe(new[] { finishedAgent, closedAgent, freshAgent }, ignoreOrder: true, "and its unit is not attempted twice"); + SupervisorOutcome.ReadStagedAgentRunIds((await SpawnDecisionAsync(runId, teamId)).OutcomeJson).ShouldBe(new[] { finishedAgent, freshAgent }, "the decision records the finished agent in its own slot and the fresh one in the closed slot, in spawn order"); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1) : Array.Empty(), ignoreOrder: true, "each slot keeps the one reservation it was first admitted under"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Suspended, "the revived run is parked on the restaged slot"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_turn_that_claimed_its_decision_before_the_stop_begins_nothing_after_the_continue(bool capped) + { + // The overtaken turn's claim committed before the stop; its begin landed after the Continue and flipped the decision + // Running under the revived run — the state a revived walk reads as a crashed walk's execution to recover. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + + var claimed = new DecisionLogHold(DecisionLogStep.AfterClaim); + var overtakenTurn = RunOvertakenTurnInBackground(runId, teamId, capped, claimed.Decorate); + + try + { + await StopContinueSignals.AwaitAsync(claimed.Reached.Task, "the overtaken turn's claim committing"); + + await StopAsync(runId, teamId); + await ContinueAsync(runId, teamId); + } + finally + { + claimed.Release.TrySetResult(); + } + + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenTurn, "the overtaken turn returning"))).ShouldBeOfType("the overtaken turn stood down at its begin"); + (await SpawnDecisionAsync(runId, teamId)).Status.ShouldBe(SupervisorDecisionStatus.Pending, "it began nothing: the decision it claimed before the stop waits, Pending, for the revived walk"); + (await AgentIdsAsync(runId)).ShouldBeEmpty("staged no agent"); + (await ReservationKeysAsync(runId)).ShouldBeEmpty("nor reserved any budget"); + + await RunEngineAsync(runId); // the revived walk finds the decision in flight and finishes it + + await AssertParkedOnARevivedWaveAsync(runId, "sup#turn1#"); + await AssertTheSpawnDecisionRecordsTheParkedWaveAsync(runId, teamId); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1) : Array.Empty(), ignoreOrder: true, "a capped run holds the finished wave's reservations only"); + (await NodeFailuresAsync(runId, "sup")).ShouldBe(0); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_turn_already_assembling_its_wave_when_the_continue_lands_reserves_nothing(bool capped) + { + // The overtaken turn began its spawn before the stop and was still assembling its wave when the Continue landed. Its + // budget admission ran ahead of the wave's fence: a capped run's reservations committed on their own, the fence then + // refused the wave, and the revived walk's replay was refused as a different intent on those very slots — told its + // spawn was over budget. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + + var assembling = new HeldCommand(IsTurnWaveRead); // past the turn's stand-down check, before its wave's transaction + var overtakenTurn = RunOvertakenTurnInBackground(runId, teamId, capped, null, assembling); + + try + { + await StopContinueSignals.AwaitAsync(assembling.Reached.Task, "the overtaken turn reading its wave"); + + await StopAsync(runId, teamId); + await ContinueAsync(runId, teamId); + } + finally + { + assembling.Release.TrySetResult(); + } + + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenTurn, "the overtaken turn returning"))).ShouldBeOfType("the overtaken turn stood down at its wave's fence"); + (await ReservationKeysAsync(runId)).ShouldBeEmpty("it reserved no budget: admission runs behind the fence, in the wave's own transaction"); + (await AgentIdsAsync(runId)).ShouldBeEmpty("staged no agent"); + (await AgentWaitsAsync(runId)).ShouldBeEmpty("nor any wait"); + (await SpawnDecisionAsync(runId, teamId)).Status.ShouldBe(SupervisorDecisionStatus.Running, "the decision it began before the stop is still in flight"); + + await RunEngineAsync(runId); // the revived walk finds it in flight and finishes it + + await AssertParkedOnARevivedWaveAsync(runId, "sup#turn1#"); + await AssertTheSpawnDecisionRecordsTheParkedWaveAsync(runId, teamId); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1) : Array.Empty(), ignoreOrder: true, "a capped run admitted the revived wave, one reservation per unit"); + (await NodeFailuresAsync(runId, "sup")).ShouldBe(0); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_turn_whose_wave_committed_before_the_stop_records_no_terminal_after_the_continue(bool capped) + { + // The overtaken turn committed its wave before the stop and wrote its spawn decision's terminal only after the + // Continue. That terminal named the wave the revive had just closed, so the revived run moved on past it to a next + // turn — or, had the revived walk replayed the decision first, one of the two lost the CAS and failed the step. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + + var terminal = new DecisionLogHold(DecisionLogStep.BeforeTerminal); + var overtakenTurn = RunOvertakenTurnInBackground(runId, teamId, capped, terminal.Decorate); + IReadOnlyList stoppedWave; + ILifetimeScope stop; + + try + { + await StopContinueSignals.AwaitAsync(terminal.Reached.Task, "the overtaken turn's wave committing, its terminal still to write"); + + stoppedWave = await AgentWaitsAsync(runId); + stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); + await ContinueAsync(runId, teamId); + } + finally + { + terminal.Release.TrySetResult(); + } + + using (stop) + { + stoppedWave.Count.ShouldBe(2, "precondition: the overtaken turn committed its two-agent wave before the stop"); + + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenTurn, "the overtaken turn returning"))).ShouldBeOfType("the overtaken turn stood down at its terminal record"); + (await SpawnDecisionAsync(runId, teamId)).Status.ShouldBe(SupervisorDecisionStatus.Running, "it recorded nothing: the decision is still in flight, for the revived walk to finish"); + + await RunEngineAsync(runId); // the revived walk replays the decision: the wave it finds is closed + await stop.Resolve().RunAllAsync(CancellationToken.None); + } + + await AssertParkedOnARevivedWaveAsync(runId, "sup#turn1#", stoppedWave.Select(w => Guid.Parse(w.Token)).ToArray()); + await AssertTheSpawnDecisionRecordsTheParkedWaveAsync(runId, teamId); + (await ReservationKeysAsync(runId)).ShouldBe(capped ? WaveKeys(1) : Array.Empty(), ignoreOrder: true, "the restaged slots kept the overtaken wave's reservations"); + (await NodeFailuresAsync(runId, "sup")).ShouldBe(0, "neither walk recorded the supervisor step failing"); + } + + [Fact] + public async Task A_stop_landing_while_a_wave_is_staged_waits_for_the_whole_wave_and_its_teardown_ends_it() + { + // The wave's fence is a share lock held to the wave's commit. A stop landing mid-staging waits for the whole wave, + // so the agents it captures and ends are all of it; a check that held no lock let the stop commit first, and the + // wave then committed under the stopped run behind its teardown's back. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped: false); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + + var staging = new HeldCommand(text => text.Contains("INSERT INTO workflow_run_wait")); // inside the wave's transaction, its agents created + var turn = RunOvertakenTurnInBackground(runId, teamId, false, null, staging); + + using var stop = _fixture.BeginScope(); + Task stopping; + + try + { + await StopContinueSignals.AwaitAsync(staging.Reached.Task, "the turn's wave, fenced and its agents created, staging its waits"); + + var stopDb = stop.Resolve(); + await stopDb.Database.OpenConnectionAsync(); + var stopPid = await stopDb.Database.SqlQueryRaw("SELECT pg_backend_pid() AS \"Value\"").SingleAsync(); + + stopping = StopHoldingTeardownAsync(stop, runId, teamId); + await StopContinueSignals.WaitForLockWaitAsync(_fixture, stopPid, "the stop's flip to block on the wave's share lock on the run row", stopping); + } + finally + { + staging.Release.TrySetResult(); + } + + (await stopping)!.AgentRunsCancelled.ShouldBe(2, "the stop, let through once the wave committed, captured the whole of it"); + await StopContinueSignals.AwaitAsync(turn, "the turn finishing its wave"); + await stop.Resolve().RunAllAsync(CancellationToken.None); + + var wave = await AgentWaitsAsync(runId); + wave.Select(w => w.IterationKey).ShouldBe(WaveKeys(1), ignoreOrder: true, "the wave the stop waited for"); + wave.ShouldAllBe(w => w.Status == WorkflowWaitStatuses.Discarded, "its waits closed by the teardown"); + (await AgentStatusesAsync(wave.Select(w => Guid.Parse(w.Token)).ToList())).ShouldBe(new[] { AgentRunStatus.Cancelled, AgentRunStatus.Cancelled }, "and its agents ended, none left queued under the stopped run"); + } + + [Fact] + public async Task A_walk_whose_decision_the_revived_walk_finished_first_stands_down_without_failing_the_step() + { + // The old walk had begun turn 0's plan when the run was stopped — on a host the stop never reached — and continued. + // The revived walk found the plan in flight, re-ran it and recorded it; only then did the old walk's executor + // return. Its terminal record read the revived walk's Succeeded, threw it as an illegal Succeeded → Succeeded + // ahead of any fence, and the engine recorded the supervisor step failed in the revived run's journal, where the + // run's next walk read the step as failed. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped: false); + + using var manual = ResolveJobClient().ManualExecution(); + + var recording = ResolveScript().HoldDecisionLog(runId, DecisionLogStep.BeforeTerminal); + var overtakenWalk = WalkOnAnotherHostInBackground(runId); // turn 0 claims, begins and executes its plan, then holds + + try + { + await StopContinueSignals.AwaitAsync(recording.Reached.Task, "the old walk's plan executed, its terminal still to write"); + + await StopAsync(runId, teamId); + await ContinueAsync(runId, teamId); + await RunEngineAsync(runId); // the revived walk finds the plan in flight, re-runs it and records it first + + (await DecisionsAsync(runId, teamId)).ShouldHaveSingleItem("precondition: the plan is the run's one decision").Status + .ShouldBe(SupervisorDecisionStatus.Succeeded, "precondition: the revived walk recorded it first"); + } + finally + { + recording.Release.TrySetResult(); + } + + (await Record.ExceptionAsync(() => StopContinueSignals.AwaitAsync(overtakenWalk, "the overtaken walk returning"))).ShouldBeNull("the overtaken walk stood down at its terminal record"); + (await NodeFailuresAsync(runId, "sup")).ShouldBe(0, "no attempt.failed or node.failed for the supervisor step landed in the revived run"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Suspended, "the revived run is parked on its self-advance, not failed"); + + await ResolveSelfAdvanceAsync(runId); + await RunEngineAsync(runId); // the revived run's next walk + + (await AgentWaitsAsync(runId)).Select(w => w.IterationKey).ShouldBe(WaveKeys(1), ignoreOrder: true, "it ran the supervisor on to turn 1: nothing marked the step failed"); + } + + [Fact] + public async Task A_continue_whose_request_went_away_after_its_commit_still_ends_the_running_agents_and_dispatches_the_run() + { + // The revive's post-commit half ran on the Continue request's own token. A client that went away after the commit + // cancelled it mid-drain: the flipped agent kept its question open, its spend claims live and its harness execution + // open, and the revived run sat undispatched until the reconciler's sweep. + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedSupervisorRunAsync(teamId, userId, capped: false); + + using var manual = ResolveJobClient().ManualExecution(); + + await RunEngineAsync(runId); // turn 0 plans + await ResolveSelfAdvanceAsync(runId); + await RunEngineAsync(runId); // turn 1 spawns both units and parks on them + + var claimedAgent = Guid.Parse((await AgentWaitsAsync(runId)).Single(w => w.IterationKey == "sup#turn1#0").Token); + await MarkRunningAsync(claimedAgent); + var question = await SeedOpenAgentDecisionAsync(teamId, claimedAgent); + + using var stop = await WorkflowsTestSeed.StopWithTeardownHeldAsync(_fixture, runId, teamId); // its teardown still to come + + using var request = new CancellationTokenSource(); + using var command = _fixture.BeginScope(); + + await using (var transaction = await command.Resolve().Database.BeginTransactionAsync()) + { + (await command.Resolve().ContinueRunAsync(runId, teamId, request.Token)).ShouldBeTrue("the stopped run continues in place"); + await transaction.CommitAsync(); + } + + request.Cancel(); // the client went away once the Continue committed + await command.Resolve().RunAllAsync(request.Token); + + (await AgentStatusesAsync(new[] { claimedAgent })).ShouldBe(new[] { AgentRunStatus.Cancelled }, "precondition: the revive flipped the running agent"); + (await DecisionStatusAsync(question)).ShouldBe(ToolCallLedgerStatus.Expired, "its cancel still finished: the agent's open question left the queue"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Enqueued, "and the revived run was still dispatched"); + } + + /// After the late teardown: the revived run is parked on exactly one two-agent wave under , none of it the stopped attempt's, all of it still open and queued. + private async Task AssertParkedOnARevivedWaveAsync(Guid runId, string keyPrefix, params Guid[] stoppedAgents) + { + var pending = (await AgentWaitsAsync(runId)).Where(w => w.Status == WorkflowWaitStatuses.Pending).ToList(); + pending.Select(w => w.IterationKey).ShouldBe(new[] { keyPrefix + "0", keyPrefix + "1" }, ignoreOrder: true, "the revived run is parked on a wave of its own, and the late teardown left it open"); + + var revivedAgents = pending.Select(w => Guid.Parse(w.Token)).ToList(); + revivedAgents.ShouldNotContain(id => stoppedAgents.Contains(id), "the revived wave reuses none of the stopped attempt's agents"); + (await AgentStatusesAsync(revivedAgents)).ShouldBe(new[] { AgentRunStatus.Queued, AgentRunStatus.Queued }, "the late teardown ended none of the revived wave's agents"); + (await RunStatusAsync(runId)).ShouldBe(WorkflowRunStatus.Suspended, "the revived run is parked on its wave"); + } + + /// The run's one spawn decision is finished, and its tape names exactly the wave the run is parked on. + private async Task AssertTheSpawnDecisionRecordsTheParkedWaveAsync(Guid runId, Guid teamId) + { + var parkedOn = (await AgentWaitsAsync(runId)).Where(w => w.Status == WorkflowWaitStatuses.Pending).Select(w => Guid.Parse(w.Token)).ToList(); + var spawn = await SpawnDecisionAsync(runId, teamId); + + spawn.Status.ShouldBe(SupervisorDecisionStatus.Succeeded, "the revived walk finished the decision itself"); + SupervisorOutcome.ReadStagedAgentRunIds(spawn.OutcomeJson).ShouldBe(parkedOn, ignoreOrder: true, "and the tape names the wave the run is parked on"); + } + + /// Turn 1 of the stopped attempt stages its wave, and the stop interrupts it before it records its spawn decision: the wave committed, the decision still in flight. + private async Task StageWaveBehindAnInterruptedDecisionAsync(Guid runId, Guid teamId, bool capped) + { + using var scope = _fixture.BeginScope(b => b.RegisterDecorator((_, _, inner) => new InterruptedTerminalDecisionLog(inner))); + + var interrupted = await Record.ExceptionAsync(() => scope.Resolve().RunTurnAsync(runId, teamId, "sup", Goal, conversationId: null, GoalConfig(capped), CancellationToken.None)); + + interrupted.ShouldBeOfType("precondition: the turn staged its wave, then its terminal write was interrupted"); + } + + /// The overtaken walk's next turn, on another host: the REAL turn service under that walk's claim on the run's current generation, in a scope with the test's hold registered and its commands through . + private Task RunOvertakenTurnInBackground(Guid runId, Guid teamId, bool capped, Action? hold, params IInterceptor[] interceptors) => Task.Run(async () => + { + var generation = await GenerationAsync(runId); + + using var scope = StopContinueSignals.InterceptedScope(_fixture, hold ?? (_ => { }), interceptors); + using var claim = RunGenerationFence.Claim(runId, generation); + + await scope.Resolve().RunTurnAsync(runId, teamId, "sup", Goal, conversationId: null, GoalConfig(capped), CancellationToken.None); + }); + + /// The turn's read of its own wave — the step after its stand-down check and before its wave's transaction. + private static bool IsTurnWaveRead(string commandText) => + commandText.StartsWith("SELECT", StringComparison.Ordinal) && commandText.Contains("FROM workflow_run_wait") && commandText.Contains("'AgentRun'") && commandText.Contains("iteration_key LIKE"); + + /// Stop the run in a transaction of the caller's scope, so the teardown waits in that scope's post-commit actions. + private static async Task StopHoldingTeardownAsync(ILifetimeScope scope, Guid runId, Guid teamId) + { + await using var transaction = await scope.Resolve().Database.BeginTransactionAsync(); + + var outcome = await scope.Resolve().CancelRunAsync(runId, teamId, CancellationToken.None); + await transaction.CommitAsync(); + + return outcome; + } + + private async Task SeedSupervisorRunAsync(Guid teamId, Guid userId, bool capped) + { + Guid workflowId; + using (var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin)) + workflowId = await scope.Resolve().Send(new CreateWorkflowCommand + { + Name = "sup-stop-continue-" + Guid.NewGuid().ToString("N")[..6], + Description = null, + Definition = SupervisorDefinition(capped), + Activations = new List(), + Enabled = true, + }); + + return await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId); + } + + private async Task RunEngineAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + } + + /// A walk on another replica: its own in-process cancellation registry, which a stop landing on this host never trips. + private Task WalkOnAnotherHostInBackground(Guid runId) => Task.Run(async () => + { + using var scope = _fixture.BeginScope(b => b.RegisterType().As().SingleInstance()); + await scope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + }); + + /// Resolve the run's pending self-advance wait through the entry point the engine enqueues, so the next walk runs the next turn. + private async Task ResolveSelfAdvanceAsync(Guid runId) + { + Guid waitId; + using (var verify = _fixture.BeginScope()) + waitId = (await verify.Resolve().WorkflowRunWait.AsNoTracking().SingleAsync(w => w.RunId == runId && w.WaitKind == WorkflowWaitKinds.SupervisorDecision && w.Status == WorkflowWaitStatuses.Pending)).Id; + + using var scope = _fixture.BeginScope(); + await scope.Resolve().ResumeWaitAsync(runId, waitId, null, CancellationToken.None); + } + + private async Task StopAsync(Guid runId, Guid teamId) + { + using var scope = _fixture.BeginScope(); + (await scope.Resolve().CancelRunAsync(runId, teamId, CancellationToken.None))!.Cancelled.ShouldBeTrue("the run was stopped"); + } + + private async Task ContinueAsync(Guid runId, Guid teamId) + { + using var scope = _fixture.BeginScope(); + (await scope.Resolve().ContinueRunAsync(runId, teamId, CancellationToken.None)).ShouldBeTrue("the stopped run continues in place"); + } + + private async Task MarkRunningAsync(Guid agentRunId) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().MarkRunningAsync(agentRunId, CancellationToken.None); + } + + /// Drive the executor's terminal sequence (MarkRunning → Complete → Notify) without the sandboxed CLI, as SupervisorSpawnFlowTests does. + private async Task SimulateAgentCompletionAsync(Guid agentRunId) + { + using var scope = _fixture.BeginScope(); + var runs = scope.Resolve(); + + await runs.MarkRunningAsync(agentRunId, CancellationToken.None); + await runs.CompleteAsync(agentRunId, new AgentRunResult { Status = AgentRunStatus.Succeeded, ExitReason = "completed", Summary = "done" }, CancellationToken.None); + await scope.Resolve().NotifyCompletedAsync(agentRunId, CancellationToken.None); + } + + /// An unanswered, human-required question the agent raised mid-run — the row a stopped agent's cancel expires. + private async Task SeedOpenAgentDecisionAsync(Guid teamId, Guid agentRunId) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + var ledgerId = Guid.NewGuid(); + + db.ToolCallLedger.Add(new ToolCallLedger + { + Id = ledgerId, TeamId = teamId, AgentRunId = agentRunId, ToolKind = DecisionToolKinds.DecisionRequest, + IdempotencyKey = $"decision.request:{ledgerId:N}", InputHash = new string('0', 64), + Status = ToolCallLedgerStatus.AwaitingApproval, ApprovalDeadlineAt = DateTimeOffset.UtcNow.AddHours(1), + CreatedBy = SystemUsers.SeederId, LastModifiedBy = SystemUsers.SeederId, + }); + + await db.SaveChangesAsync(); + return ledgerId; + } + + private async Task DecisionStatusAsync(Guid ledgerId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().ToolCallLedger.AsNoTracking().Where(l => l.Id == ledgerId).Select(l => l.Status).SingleAsync(); + } + + private async Task GenerationAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRun.AsNoTracking().Where(r => r.Id == runId).Select(r => r.Generation).SingleAsync(); + } + + private async Task> AgentWaitsAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunWait.AsNoTracking().Where(w => w.RunId == runId && w.WaitKind == WorkflowWaitKinds.AgentRun).ToListAsync(); + } + + private async Task> WaitStatusesAsync(IEnumerable waits) + { + var ids = waits.Select(w => w.Id).ToList(); + + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunWait.AsNoTracking().Where(w => ids.Contains(w.Id)).Select(w => w.Status).ToListAsync(); + } + + private async Task> AgentIdsAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().AgentRun.AsNoTracking().Where(a => a.WorkflowRunId == runId).Select(a => a.Id).ToListAsync(); + } + + /// The statuses of , in the order given. + private async Task> AgentStatusesAsync(IReadOnlyList agentRunIds) + { + using var scope = _fixture.BeginScope(); + var byId = await scope.Resolve().AgentRun.AsNoTracking().Where(a => agentRunIds.Contains(a.Id)).ToDictionaryAsync(a => a.Id, a => a.Status); + + return agentRunIds.Where(byId.ContainsKey).Select(id => byId[id]).ToList(); + } + + private async Task> ReservationKeysAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().BudgetReservation.AsNoTracking().Where(r => r.WorkflowRunId == runId).Select(r => r.ScopeKey).ToListAsync(); + } + + private async Task NodeFailuresAsync(Guid runId, string nodeId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRunRecord.AsNoTracking().CountAsync(r => r.RunId == runId && r.NodeId == nodeId && (r.RecordType == WorkflowRunRecordTypes.NodeFailed || r.RecordType == WorkflowRunRecordTypes.AttemptFailed)); + } + + private async Task> DecisionsAsync(Guid runId, Guid teamId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().SupervisorDecisionRecord.AsNoTracking().Where(d => d.SupervisorRunId == runId && d.TeamId == teamId).OrderBy(d => d.Sequence).ToListAsync(); + } + + private async Task> DecisionKindsAsync(Guid runId, Guid teamId) => (await DecisionsAsync(runId, teamId)).Select(d => d.DecisionKind).ToList(); + + private async Task> SpawnDecisionsAsync(Guid runId, Guid teamId) => (await DecisionsAsync(runId, teamId)).Where(d => d.DecisionKind == SupervisorDecisionKinds.Spawn).ToList(); + + private async Task SpawnDecisionAsync(Guid runId, Guid teamId) => (await SpawnDecisionsAsync(runId, teamId)).ShouldHaveSingleItem("the run has one spawn decision"); + + private async Task RunStatusAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRun.AsNoTracking().Where(r => r.Id == runId).Select(r => r.Status).SingleAsync(); + } + + private SupervisorDecisionScript ResolveScript() + { + using var scope = _fixture.BeginScope(); + return scope.Resolve(); + } + + private InMemoryBackgroundJobClient ResolveJobClient() + { + using var scope = _fixture.BeginScope(); + return scope.Resolve(); + } + + /// The iteration keys of a two-unit wave at — also each slot's budget reservation scope. + private static string[] WaveKeys(int turn) => new[] { $"sup#turn{turn}#0", $"sup#turn{turn}#1" }; + + private static string SupervisorConfig(bool capped) => capped ? $$"""{"goal":"{{Goal}}","maxCostUsd":{{CapUsd}}}""" : $$"""{"goal":"{{Goal}}"}"""; + + /// The goal config the supervisor node reads off its own config, for a turn driven through the turn service directly. + private static SupervisorGoalConfig? GoalConfig(bool capped) => AgentSupervisorNode.ReadGoalConfig(JsonSerializer.Deserialize>(SupervisorConfig(capped))!); + + // manual → sup (agent.supervisor) → terminal. Shadow completion: the simulated agents mint no delivery evidence, and + // completion arbitration is not this suite's subject (mirrors SupervisorSpawnFlowTests). + private static WorkflowDefinition SupervisorDefinition(bool capped) => new() + { + SchemaVersion = 1, + CompletionMode = WorkflowDefinition.CompletionModeShadow, + Nodes = new List + { + new() { Id = "start", TypeKey = "trigger.manual", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "sup", TypeKey = "agent.supervisor", Config = WorkflowsTestSeed.Json(SupervisorConfig(capped)), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "end", TypeKey = "builtin.terminal", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + }, + Edges = new List + { + new() { From = "start", To = "sup" }, + new() { From = "sup", To = "end" }, + }, + }; + + /// The decision log with its first terminal write interrupted the way a stop's tripped token interrupts it: the wave the executor just committed stays behind a decision still in flight. Every other call, and every later terminal write, reaches the real log. + private sealed class InterruptedTerminalDecisionLog : ISupervisorDecisionLog + { + private readonly ISupervisorDecisionLog _inner; + private int _interrupted; + + public InterruptedTerminalDecisionLog(ISupervisorDecisionLog inner) { _inner = inner; } + + public Task RecordTerminalAsync(Guid decisionId, Guid teamId, SupervisorDecisionStatus status, string? outcomeJson, string? error, CancellationToken cancellationToken) => + Interlocked.Exchange(ref _interrupted, 1) == 0 + ? throw new OperationCanceledException("The stop tripped the walk's token before it recorded the decision.") + : _inner.RecordTerminalAsync(decisionId, teamId, status, outcomeJson, error, cancellationToken); + + public Task TryClaimAsync(SupervisorDecisionClaimRequest request, CancellationToken cancellationToken) => _inner.TryClaimAsync(request, cancellationToken); + + public Task TryBeginExecutionAsync(Guid decisionId, Guid teamId, CancellationToken cancellationToken) => _inner.TryBeginExecutionAsync(decisionId, teamId, cancellationToken); + + public Task> GetForRunAsync(Guid supervisorRunId, Guid teamId, CancellationToken cancellationToken) => _inner.GetForRunAsync(supervisorRunId, teamId, cancellationToken); + + public Task> GetTerminalDecisionsAsync(Guid supervisorRunId, Guid teamId, CancellationToken cancellationToken) => _inner.GetTerminalDecisionsAsync(supervisorRunId, teamId, cancellationToken); + + public Task UpdateOutcomeAsync(Guid decisionId, Guid teamId, string foldedOutcomeJson, CancellationToken cancellationToken) => _inner.UpdateOutcomeAsync(decisionId, teamId, foldedOutcomeJson, cancellationToken); + + public Task ExpireStalePendingAsync(DateTimeOffset olderThan, CancellationToken cancellationToken) => _inner.ExpireStalePendingAsync(olderThan, cancellationToken); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAgentResultsFoldTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAgentResultsFoldTests.cs index 3f06dcc2c..a1e54da96 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAgentResultsFoldTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAgentResultsFoldTests.cs @@ -14,7 +14,8 @@ namespace CodeSpace.UnitTests.Agents; /// (terminal-scoping, DB-gate, persist-once) is proven over real Postgres in SupervisorAgentResultsRehydrateFlowTests; /// this pins the decision logic in isolation. The crown jewels: the fold is ADDITIVE (agentRunIds + agentCount stay /// byte-intact so the E5 spawn-cap / no-progress counters are unperturbed), and a Failed agent whose ResultJson is -/// null still surfaces its ROW error (the exact signal the slice exists to surface). +/// null still surfaces its ROW error (the exact signal the slice exists to surface). The rehydrate's own decision fold is +/// pinned last: written once, it never folds a wave while any of its agents is still live. /// [Trait("Category", "Unit")] public class SupervisorAgentResultsFoldTests @@ -422,4 +423,47 @@ public void HasSettledEvidence_is_true_when_any_agent_in_a_mixed_wave_produced_e SupervisorOutcome.HasSettledEvidence(mixed).ShouldBeTrue("any one settled-evidence agent is progress"); } + + // ── The rehydrate's decision fold: written once, so never over an agent still live ───── + + [Theory] + [InlineData(new[] { "Succeeded", "Cancelled" }, true)] + [InlineData(new[] { "Failed", "TimedOut", "NeedsReview" }, true)] + [InlineData(new[] { "Succeeded", "Running" }, false)] + [InlineData(new[] { "Queued", "Cancelled" }, false)] + public void The_rehydrate_folds_a_spawn_only_once_every_agent_has_ended(string[] statuses, bool folds) + { + // The fold is never revisited, so a status read while an agent is still Queued or Running would stand on the + // tape as its result for good: a Continue's revived turn, re-entering without the barrier, once recorded a + // stopped wave's still-running agent as "Running" permanently. + var ids = statuses.Select(_ => Guid.NewGuid()).ToArray(); + var results = ids.Zip(statuses).ToDictionary(p => p.First, p => new SupervisorAgentResult { AgentRunId = p.First, Status = p.Second }); + + var folded = SupervisorTurnService.FoldAgentResults(TerminalSpawn(ids), results); + + SupervisorOutcome.ReadAgentResults(folded.OutcomeJson).Select(r => r.Status).ShouldBe(folds ? statuses : Array.Empty(), "every agent ended → the whole wave folds, in spawn order; any still live → nothing folds yet"); + SupervisorOutcome.ReadStagedAgentRunIds(folded.OutcomeJson).ShouldBe(ids, "the staged wave itself is untouched either way"); + } + + [Fact] + public void The_rehydrate_folds_an_agent_that_no_longer_resolves_as_a_final_placeholder() + { + var ended = Guid.NewGuid(); + var gone = Guid.NewGuid(); + var results = new Dictionary { [ended] = new() { AgentRunId = ended, Status = "Succeeded" } }; + + var folded = SupervisorOutcome.ReadAgentResults(SupervisorTurnService.FoldAgentResults(TerminalSpawn(ended, gone), results).OutcomeJson); + + folded.Select(r => r.Status).ShouldBe(new[] { "Succeeded", "Unknown" }, "a row that is gone will never end either — it folds as final, not waited on"); + } + + private static SupervisorPriorDecision TerminalSpawn(params Guid[] agentRunIds) => new() + { + Id = Guid.NewGuid(), + Sequence = 1, + DecisionKind = SupervisorDecisionKinds.Spawn, + Status = SupervisorDecisionStatus.Succeeded, + PayloadJson = "{}", + OutcomeJson = SpawnOutcome(agentRunIds), + }; } diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs index 95d85e745..f7953e867 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/FailureTaxonomyTests.cs @@ -32,9 +32,11 @@ public class FailureTaxonomyTests }; /// - /// Throws that are a jump, not a fault. Each is caught by the code that threw it — a suspended run - /// is the SUCCESS path for a wait node, a walk a Continue overtook stands down without writing (the run is - /// fine: it belongs to the revived walk), a stalled sandbox is selected as a status sixty lines from + /// Throws that are a jump, not a fault. Each is caught by the code that owns the jump — a suspended run + /// is the SUCCESS path for a wait node; a walk a Continue overtook stands down without writing, from the engine's + /// park or from anywhere in the supervisor node it runs — the decision ledger, a spawn wave, an ask_human card — + /// and the engine's node runner lets it through as the walk's end, never a node failure (the run is fine: it + /// belongs to the revived walk); a stalled sandbox is selected as a status sixty lines from /// where it is raised, a halted materialization carries one of a closed set of outcomes out of a pipeline that /// then RETURNS it, and a provider rejection crosses nested generic adapters into one typed adoption summary. /// Classifying them would invite someone to render a parked run, or a team that simply already had its own diff --git a/backend/tests/CodeSpace.UnitTests/Architecture/ScopedTransactionInventoryTests.cs b/backend/tests/CodeSpace.UnitTests/Architecture/ScopedTransactionInventoryTests.cs index 787ca5611..e8f631f42 100644 --- a/backend/tests/CodeSpace.UnitTests/Architecture/ScopedTransactionInventoryTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Architecture/ScopedTransactionInventoryTests.cs @@ -42,6 +42,7 @@ public class ScopedTransactionInventoryTests [ "Agents/AgentRunSpoolReaper.cs", "Workflows/Budget/BudgetLedger.cs", + "Workflows/Engine/RunGenerationFence.cs", "Workflows/ModelCalls/WorkflowRunModelCallProjector.cs", "Workflows/ToolCalls/WorkflowRunToolCallProjector.cs", "Workflows/WorkflowService.cs", diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/RunGenerationFenceTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/RunGenerationFenceTests.cs new file mode 100644 index 000000000..b3a94b0d7 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/RunGenerationFenceTests.cs @@ -0,0 +1,89 @@ +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Services.Workflows.Engine; +using Microsoft.EntityFrameworkCore; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// The walk claim carries to every node a walk runs: read back only for the run it was +/// taken for, restored when released, flowing with the async call that took it — and a lock that refuses to run where it +/// could not hold. The lock itself, and a Continue meeting it, are pinned against real Postgres by the stop/continue flow +/// suites; nothing here reaches a database. +/// +[Trait("Category", "Unit")] +public class RunGenerationFenceTests +{ + [Fact] + public void A_claim_is_read_back_only_for_the_run_it_was_taken_for() + { + var runId = Guid.NewGuid(); + + using (RunGenerationFence.Claim(runId, 3)) + { + RunGenerationFence.ClaimedFor(runId).ShouldBe(3, "the walk's own run reads the generation it claimed"); + RunGenerationFence.ClaimedFor(Guid.NewGuid()).ShouldBeNull("any other run — a child the node starts, a run it only reads — is not fenced by this walk's claim"); + } + + RunGenerationFence.ClaimedFor(runId).ShouldBeNull("released, the claim is gone"); + } + + [Fact] + public void Releasing_a_nested_claim_restores_the_one_it_replaced() + { + var outerRun = Guid.NewGuid(); + var innerRun = Guid.NewGuid(); + + using (RunGenerationFence.Claim(outerRun, 1)) + { + using (RunGenerationFence.Claim(innerRun, 7)) + { + RunGenerationFence.ClaimedFor(innerRun).ShouldBe(7); + RunGenerationFence.ClaimedFor(outerRun).ShouldBeNull("while a nested walk holds its claim, only its own run is fenced"); + } + + RunGenerationFence.ClaimedFor(outerRun).ShouldBe(1, "releasing the nested claim restores the outer walk's"); + } + } + + [Fact] + public async Task A_claim_flows_with_the_async_call_that_took_it_and_never_into_its_caller() + { + var runId = Guid.NewGuid(); + + async Task ClaimAcrossAnAwaitAsync() + { + using var claim = RunGenerationFence.Claim(runId, 5); + await Task.Yield(); + return RunGenerationFence.ClaimedFor(runId); + } + + (await ClaimAcrossAnAwaitAsync()).ShouldBe(5, "the claim survives the awaits of the call that took it"); + RunGenerationFence.ClaimedFor(runId).ShouldBeNull("and never leaks into the caller"); + } + + [Fact] + public async Task The_lock_refuses_to_run_outside_a_transaction() + { + await using var db = UnreachableDatabase(); + + var refused = await Should.ThrowAsync(() => RunGenerationFence.TryLockAsync(db, Guid.NewGuid(), 1, CancellationToken.None)); + + refused.Message.ShouldContain("needs the caller's transaction", customMessage: "outside one the share lock would end with its own statement — a check, not a fence"); + } + + [Fact] + public async Task Outside_a_walk_nothing_is_fenced_and_no_database_is_touched() + { + await using var db = UnreachableDatabase(); + var runId = Guid.NewGuid(); + + await RunGenerationFence.EnterAsync(db, runId, CancellationToken.None); + await RunGenerationFence.ThrowIfSupersededAsync(db, runId, CancellationToken.None); + + (await RunGenerationFence.CommitUnderClaimAsync(db, runId, () => Task.FromResult(42), CancellationToken.None)).ShouldBe(42, "a write outside a walk commits as it always did"); + } + + private static CodeSpaceDbContext UnreachableDatabase() => + new(new DbContextOptionsBuilder().UseNpgsql("Host=127.0.0.1;Port=1;Database=unused").UseSnakeCaseNamingConvention().Options); +}