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); +}