Orchestrator restart terminally fails in-flight items instead of requeueing them; pause does not drain

docs/development/manual-uat/persistence-and-recovery.md

@@ -25,7 +25,8 @@ repositories only.
 3. Send `kill -9` to the orchestrator process.
 4. Restart CodeyBox with the same `state.db`.
 5. Verify startup replay maps the item to the expected durable state:
-   `Working` without a preempt checkpoint becomes `Failed`, `Auditing` and
+   `Working` without a preempt checkpoint becomes `Queued` (recovery budget
+   untouched), `Auditing` and
    `Reworking` become `WorkComplete`, `Merging` becomes `AuditPassed`, and
    `UpstreamPushing` becomes `Merged`.
 6. Repeat the interrupted recovery until `MaxRecoveryAttempts` is exceeded and

docs/operating/recovery.md

@@ -46,7 +46,18 @@ Each active worker fires an `UPDATE worker_registry SET last_heartbeat_at = $now
    quiescing.
 4. For each claimed row whose `current_work_item_id IS NOT NULL`:
    - Look up the work item.
-   - If it is in a recoverable worker-owned state (see table below), increment `RecoveryAttempts` and transition it.
+   - If it is a checkpoint-less `Working` item, requeue it preserving the
+     work branch **without incrementing `RecoveryAttempts`**: losing the
+     worker with no durable evidence is infrastructure, not item failure, so
+     a restart never erodes the item's recovery budget and never transitions
+     it to `Failed` or `AbandonedAfterRecoveryAttempts`. Each such requeue
+     increments a separate consecutive-infrastructure counter (reset when a
+     phase completes or an operator retries); past
+     `MaxConsecutiveInfrastructureRecoveries` (default **20**) the item parks
+     at `NeedsOperatorInput` for triage instead of requeueing, so a poison
+     input that kills every worker cannot retry forever.
+   - Otherwise, if it is in a recoverable worker-owned state (see table below),
+     increment `RecoveryAttempts` and transition it.
    - If it is in a durable phase-boundary state, re-dispatch it without changing state, still consuming a recovery attempt.
    - If `RecoveryAttempts` exceeds `MaxRecoveryAttempts` (default **10**): transition to `AbandonedAfterRecoveryAttempts` with `LastError = "exceeded MaxRecoveryAttempts"`.
    - Fire a `work_item.recovered` webhook event for recovery handoffs, including same-state phase-boundary redispatches.
@@ -70,7 +81,7 @@ The mechanics, the retained-VM fallback for Incus, and the attempt caps are in
 
 | State when worker died | Recovered to | Why |
 |---|---|---|
-| `Working` | `Working` with a typed Git or retained-sandbox recovery boundary; otherwise `Failed` | Valid recovery evidence preserves the interrupted turn for bounded resume. Without it there is no durable mid-turn evidence, so explicit retry is required. |
+| `Working` | `Queued` preserving the work branch (`PreserveWorkBranchOnQueuedPickup`), or `Working` with the preempt checkpoint when one exists | No durable mid-turn evidence means the worker loss is purely infrastructure: requeue for a fresh run **without consuming `RecoveryAttempts`** and never `Failed`. Past `MaxConsecutiveInfrastructureRecoveries` (default **20**) consecutive infrastructure requeues without a phase completing, the item parks at `NeedsOperatorInput` for triage instead of requeueing. A preempt checkpoint resumes the exact interrupted turn. The one exception is a suspended sandbox whose resume was attempted and failed (VM gone, provider error, timeout): the suspended state itself is unrecoverable, so that item is marked `Failed` — a genuine resume failure, not a worker loss. |
 | `Planning` | `Queued` | Planning edits are discarded; rerun the planning-only turn from a clean sandbox |
 | `PlanReview` | `PlanReview` | A plan artifact already exists; rerun the auditor-backed plan-review loop, including plan rework if reviewers still block |
 | `PlanApproved` | `PlanApproved` | Re-dispatch implementation from the approved-plan boundary and count the recovery handoff |
@@ -94,6 +105,7 @@ All options live under `CodeyBox:DeadWorker`:
 | `DeadWorkerThreshold` | `00:01:30` | Workers not seen in this window are presumed dead |
 | `CheckInterval` | `00:01:00` | How often the reaper periodic sweep runs |
 | `MaxRecoveryAttempts` | `10` | Cap on automatic recovery transitions before the item is abandoned for operator triage |
+| `MaxConsecutiveInfrastructureRecoveries` | `20` | Cap on consecutive worker-loss-without-checkpoint requeues before the item parks at `NeedsOperatorInput`; never consumes `MaxRecoveryAttempts`. Set to `0` to disable (not recommended) |
 
 **Constraint**: `DeadWorkerThreshold` must be ≥ 3 × `HeartbeatInterval`. Startup validation throws if the constraint is violated.
 
@@ -204,4 +216,7 @@ To rehearse the window: `kill -SIGTERM` the API, wait for the port to free,
 leave it down for 30 s, start it again, and check that work-item count is
 unchanged, that your poller's next tick succeeds, and that GitHub's "Recent
 deliveries" panel shows a successful retry. The new process logs its recovery
-banner as the reaper resets in-flight items to their safe restart point.
+banner as the reaper returns in-flight items to a runnable state. To avoid
+interrupting running work at all, drain first with `POST /queue/drain` (see
+[`running.md`](running.md#restarting-the-orchestrator-safely)) — pausing with
+`POST /queue/pause` alone does not wait for in-flight items.

docs/operating/running.md

@@ -59,11 +59,42 @@ by default). Put a reverse proxy in front of it for TLS and auth.
 ## Restarting the orchestrator safely
 
 Items in flight during a shutdown are **not** cancelled. They stay in their
-mid-flight state and the reaper resets each one to a safe restart point on the
-next startup, incrementing `recoveryAttempts`. After
-`CodeyBox:DeadWorker:MaxRecoveryAttempts` recoveries (default 10) without
-reaching a terminal state, an item lands in `AbandonedAfterRecoveryAttempts` and
-waits for `POST /workitems/{id}/retry`.
+mid-flight state and the reaper returns each one to a runnable state on the
+next startup. Losing the worker is treated as infrastructure, not item
+failure: a `Working` item interrupted without a preempt checkpoint is
+re-queued preserving its work branch **without** consuming its recovery
+budget, so routine restarts never push it toward `Failed` or
+`AbandonedAfterRecoveryAttempts`. (Repeated worker losses without any phase
+completing are bounded separately: past
+`CodeyBox:DeadWorker:MaxConsecutiveInfrastructureRecoveries` consecutive
+infrastructure requeues the item parks at `NeedsOperatorInput` for triage.)
+Other mid-flight states still count their
+recovery handoff against `CodeyBox:DeadWorker:MaxRecoveryAttempts`
+(default 10); after that many recoveries without reaching a terminal state,
+an item lands in `AbandonedAfterRecoveryAttempts` and waits for
+`POST /workitems/{id}/retry`.
+
+For a clean restart with nothing interrupted, drain first — pausing alone is
+not enough, because pause only blocks *new* pickup while in-flight work keeps
+running:
+
+```bash
+# 1. Pause new pickup AND wait for running workers to finish (up to 300 s).
+curl -X POST localhost:5000/queue/drain \
+  -H 'Content-Type: application/json' \
+  -d '{"reason":"deploy restart","timeoutSeconds":300}'
+# → {"state":"Paused","drained":true,"currentlyRunning":0,...}
+
+# 2. Restart the process.
+
+# 3. Resume pickup.
+curl -X POST localhost:5000/queue/resume -H 'Content-Type: application/json' -d '{}'
+```
+
+If `drained` comes back `false`, some workers were still running when the
+deadline elapsed: wait and drain again, or restart anyway and let recovery
+re-queue the interrupted items. `POST /queue/pause` gives no such guarantee —
+it returns immediately with in-flight work still running.
 
 Per-state resume points, the reaper's fencing rules, and the caller-facing
 downtime window are in [`recovery.md`](recovery.md).

docs/operating/worker-pool.md

@@ -354,12 +354,34 @@ forget they left it paused.
 Pausing is **not** the same as cancelling. Items blocked by the pause gate
 remain Queued and are picked up automatically on resume.
 
+### Draining before a restart
+
+Pause alone does **not** wait: it returns immediately while in-flight workers
+keep running, so restarting right after pausing still interrupts running work
+(which recovery then re-queues). `POST /queue/drain` closes that gap — it
+pauses new pickup and then blocks until no workers are running or the deadline
+elapses:
+
+| | `POST /queue/pause` | `POST /queue/drain` |
+|---|---|---|
+| New item pickup | Blocked | Blocked (pauses first if still running) |
+| In-flight workers | Unaffected, keeps running | Waits until none are running |
+| Returns | Immediately | When quiescent or `timeoutSeconds` elapses |
+| Queue state after | Paused | Paused (resume it, or restart, when ready) |
+
+Safe-restart sequence: drain → restart → resume. When `drained` is `true`,
+every worker has reached a safe boundary and the restart disturbs nothing.
+When it is `false`, some workers were still running at the deadline — drain
+again or restart anyway and let recovery re-queue the interrupted items.
+
 ### API
 
 ```
 GET  /queue/status          → { state, pausedAt, pausedReason, refactorGates }
 POST /queue/pause           body: { "reason": "..." }  → { state, pausedAt }
 POST /queue/resume          → { state }
+POST /queue/drain           body: { "reason": "...", "timeoutSeconds": 300 }
+                            → { state, drained, currentlyRunning, pausedAt, pausedReason }
 ```
 
 Operators must supply a non-empty reason when pausing. The reason is stored

docs/reference/api.md

@@ -1266,6 +1266,26 @@ Clear a per-project queue pause. No-op if the project is not paused.
 * Returns `200 OK` with `{ "projectId", "paused": false, "pausedAt": null, "pausedReason": null }`.
 * Returns `404 Not Found` if the project does not exist.
 
+### `POST /queue/drain`
+
+Pause-and-wait drain for graceful restarts. Pauses the global queue when it
+is still running, then blocks until no workers are running or `timeoutSeconds`
+elapses — unlike `POST /queue/pause`, which returns immediately with in-flight
+work still running. The queue stays paused afterwards; resume it (or restart,
+then resume) when ready.
+
+```json
+{ "reason": "deploy restart", "timeoutSeconds": 300 }
+```
+
+* `reason` — required, ≤ 500 chars, no control characters.
+* `timeoutSeconds` — required, 1–3600.
+* Returns `200 OK` with `{ "state", "drained", "currentlyRunning", "pausedAt", "pausedReason" }`.
+  `drained: true` means every worker reached a safe boundary; `false` means
+  the deadline elapsed first (drain again, or restart and let recovery
+  re-queue the interrupted items).
+* Returns `400 Bad Request` if reason or timeout is missing or invalid.
+
 ### `GET /agents/paused`
 
 List agent kinds and pooled instances currently paused for new dispatch.

src/CodeyBox.Api/WorkItemEndpoints.cs

@@ -51,6 +51,7 @@ public static void Map(WebApplication app)
         app.MapGet("/queue/status", GetQueueStatusAsync);
         app.MapPost("/queue/pause", PauseQueueAsync);
         app.MapPost("/queue/resume", ResumeQueueAsync);
+        app.MapPost("/queue/drain", DrainQueueAsync);
     }
 
     private static async Task<IResult> GetWorkerStatusAsync(
@@ -1125,6 +1126,7 @@ private static async Task<IResult> UncancelAsync(
         {
             RecoveryAttempts = 0,
             RecoveryAttemptSourceState = null,
+            ConsecutiveInfrastructureRecoveries = 0,
         };
         var updated = await store.TryUpdateIfStateAsync(requeued, WorkItemState.Cancelled, ct);
         if (!updated)
@@ -2094,18 +2096,40 @@ private static async Task<IResult> GetQueueStatusAsync(
         });
     }
 
+    /// <summary>Maximum length of a queue pause/drain reason (characters).</summary>
+    public const int MaxQueueReasonLength = 500;
+
+    /// <summary>Minimum drain wait (seconds) accepted by the drain endpoint.</summary>
+    public const int MinDrainTimeoutSeconds = 1;
+
+    /// <summary>Maximum drain wait (seconds) accepted by the drain endpoint.</summary>
+    public const int MaxDrainTimeoutSeconds = 3600;
+
+    /// <summary>
+    /// Shared required-reason guard for the queue pause/drain endpoints: the
+    /// reason must be present, contain no control characters, and fit within
+    /// <see cref="MaxQueueReasonLength"/> characters. Returns a BadRequest
+    /// result when invalid, null when the reason is acceptable.
+    /// </summary>
+    private static IResult? ValidateQueueReason(string? reason)
+    {
+        if (string.IsNullOrWhiteSpace(reason))
+            return Results.BadRequest(new { error = "reason is required" });
+        if (reason.Any(char.IsControl))
+            return Results.BadRequest(new { error = "reason must not contain control characters" });
+        if (reason.Length > MaxQueueReasonLength)
+            return Results.BadRequest(new { error = $"reason must be <= {MaxQueueReasonLength} chars" });
+        return null;
+    }
+
     private static async Task<IResult> PauseQueueAsync(
         PauseQueueRequest body,
         IQueueController queueController,
         IWebhookDispatcher webhooks,
         CancellationToken ct)
     {
-        if (string.IsNullOrWhiteSpace(body.Reason))
-            return Results.BadRequest(new { error = "reason is required" });
-        if (body.Reason.Any(char.IsControl))
-            return Results.BadRequest(new { error = "reason must not contain control characters" });
-        if (body.Reason.Length > 500)
-            return Results.BadRequest(new { error = "reason must be <= 500 chars" });
+        if (ValidateQueueReason(body.Reason) is { } reasonError)
+            return reasonError;
 
         await queueController.PauseAsync(body.Reason, ct);
         _ = webhooks.PublishAsync(new WebhookEvent
@@ -2144,6 +2168,55 @@ private static async Task<IResult> ResumeQueueAsync(
         });
     }
 
+    /// <summary>
+    /// Pause-and-wait drain for graceful restarts. Pauses the queue when it is
+    /// still running, then blocks until no workers are running or
+    /// <c>timeoutSeconds</c> elapses. Unlike <c>POST /queue/pause</c> — which
+    /// returns immediately and leaves in-flight work running — drain lets an
+    /// operator restart without interrupting running work: when
+    /// <c>drained</c> is true every worker has reached a safe boundary. The
+    /// queue stays paused afterwards; resume it (or restart, then resume)
+    /// when ready.
+    /// </summary>
+    private static async Task<IResult> DrainQueueAsync(
+        DrainQueueRequest body,
+        IQueueController queueController,
+        OrchestratorService orchestrator,
+        IWebhookDispatcher webhooks,
+        CancellationToken ct)
+    {
+        if (ValidateQueueReason(body.Reason) is { } drainReasonError)
+            return drainReasonError;
+        if (body.TimeoutSeconds is not { } timeoutSeconds
+            || timeoutSeconds < MinDrainTimeoutSeconds
+            || timeoutSeconds > MaxDrainTimeoutSeconds)
+            return Results.BadRequest(new { error = $"timeoutSeconds is required and must be between {MinDrainTimeoutSeconds} and {MaxDrainTimeoutSeconds}" });
+
+        if (queueController.State == QueueState.Running)
+        {
+            await queueController.PauseAsync(body.Reason, ct);
+            _ = webhooks.PublishAsync(new WebhookEvent
+            {
+                Event = "queue.paused",
+                Details = new { pausedAt = queueController.PausedAt, reason = queueController.PausedReason, pausedBy = "api" },
+            }, CancellationToken.None);
+        }
+
+        var drained = await QueueDrain.WaitForQuiescenceAsync(
+            async innerCt => (await orchestrator.GetStatusAsync(innerCt)).CurrentlyRunning,
+            TimeSpan.FromSeconds(timeoutSeconds),
+            ct);
+        var status = await orchestrator.GetStatusAsync(ct);
+        return Results.Ok(new
+        {
+            state = queueController.State.ToString(),
+            drained,
+            currentlyRunning = status.CurrentlyRunning,
+            pausedAt = queueController.PausedAt,
+            pausedReason = queueController.PausedReason,
+        });
+    }
+
     // ── Budget usage ──────────────────────────────────────────────────────────
 
     private static async Task<IResult> GetBudgetUsageAsync(
@@ -3064,6 +3137,17 @@ public sealed record ReorderWorkItemsRequest(string[]? Ids = null);
 
 public sealed record PauseQueueRequest(string Reason = "");
 
+/// <summary>
+/// Pause-and-wait drain request. <c>Reason</c> follows the shared queue-reason
+/// guard (required, no control characters, at most
+/// <c>WorkItemEndpoints.MaxQueueReasonLength</c> characters).
+/// <c>TimeoutSeconds</c> bounds how long the endpoint waits for in-flight work
+/// to reach a safe boundary (<c>WorkItemEndpoints.MinDrainTimeoutSeconds</c> to
+/// <c>WorkItemEndpoints.MaxDrainTimeoutSeconds</c>); on expiry the endpoint
+/// reports <c>drained: false</c> and the queue stays paused.
+/// </summary>
+public sealed record DrainQueueRequest(string Reason = "", int? TimeoutSeconds = null);
+
 public sealed record WorkItemTimelineResponse(string WorkItemId, IReadOnlyList<TimelineEntry> Entries);
 
 public sealed record WorkItemAgentHistoryResponse(

src/CodeyBox.Core/SandboxAbstractions.cs

@@ -910,8 +910,8 @@ Task ResumeSandboxAsync(ManagedSandboxInfo sandbox, CancellationToken ct)
     /// preempt-checkpoint git ref on origin so the orchestrator's standard
     /// recovery flow (see <c>DeadWorkerReaper.RecoverWorkItemAsync</c>) can
     /// re-enqueue the work item with a non-null
-    /// <see cref="WorkItem.PreemptCheckpoint"/> instead of marking it Failed
-    /// for "Working without a preempt checkpoint".
+    /// <see cref="WorkItem.PreemptCheckpoint"/> for a clean resume (rather
+    /// than the checkpoint-less requeue).
     ///
     /// <para>Operation, executed inside the resumed VM:</para>
     /// <list type="number">

src/CodeyBox.Core/WorkItem.cs

@@ -270,6 +270,21 @@ public sealed record WorkItem
     /// </summary>
     public WorkItemState? RecoveryAttemptSourceState { get; init; }
 
+    /// <summary>
+    /// Consecutive infrastructure-caused requeues (worker death without a
+    /// preempt checkpoint) since a phase last completed successfully or an
+    /// operator last retried/resumed the item. Tracked separately from
+    /// <see cref="RecoveryAttempts"/> so routine restarts never erode the
+    /// genuine-failure budget: infrastructure loss carries no evidence the
+    /// item itself is at fault. When it exceeds the configured
+    /// consecutive-infrastructure cap the item parks at
+    /// <see cref="WorkItemState.NeedsOperatorInput"/> instead of requeueing,
+    /// which bounds poison inputs that deterministically kill every worker
+    /// that picks the item up. Reset alongside <see cref="RecoveryAttempts"/>
+    /// on real progress and on manual retry/resume.
+    /// </summary>
+    public int ConsecutiveInfrastructureRecoveries { get; init; }
+
     /// <summary>Number of attempts that have been made on the upstream-push phase.</summary>
     public int UpstreamPushAttempts { get; init; }
 

src/CodeyBox.Orchestrator/DeadWorkerOptions.cs

@@ -33,6 +33,22 @@ public sealed class DeadWorkerOptions
     /// </summary>
     public int MaxRecoveryAttempts { get; set; } = 10;
 
+    /// <summary>
+    /// Maximum number of CONSECUTIVE infrastructure-caused requeues (worker
+    /// death without a preempt checkpoint) for a single work item before the
+    /// reaper parks it at <c>NeedsOperatorInput</c> for triage instead of
+    /// requeueing. Default 20. Infrastructure requeues never consume
+    /// <see cref="MaxRecoveryAttempts"/>, so without this separate bound a
+    /// poison input that deterministically kills every worker would retry
+    /// forever; the counter resets whenever a phase completes or an operator
+    /// retries/resumes the item, so routine restarts never approach the cap.
+    /// Pairs with <see cref="OrchestratorOptions.MaxConsecutiveInfrastructureRecoveries"/>
+    /// (the startup-replay / shutdown-recovery counterpart).
+    /// Set to 0 (or any negative value) to disable the bound and requeue
+    /// indefinitely (not recommended in production).
+    /// </summary>
+    public int MaxConsecutiveInfrastructureRecoveries { get; set; } = 20;
+
     /// <summary>
     /// Validates that the threshold is large enough to avoid false positives.
     /// Throws <see cref="InvalidOperationException"/> on misconfiguration.