fix: guard heartbeat requeue against stale hold races

docs/CONFIGURATION.md

@@ -82,7 +82,7 @@ roles:
 
 ### Workflow States
 
-The workflow section defines the state machine for issue lifecycle — states, transitions, review policy, and the optional test phase.
+The workflow section defines the state machine for issue lifecycle — states, transitions, review policy, the optional test phase, and optional delivery policies for promotion and acceptance.
 
 See **[Workflow Reference](WORKFLOW.md)** for the full state machine documentation, including state types, built-in actions, review policy options, and how to enable the test phase.
 

docs/ROADMAP.md

@@ -19,7 +19,7 @@ Planning → To Do → Doing → To Review → [PR approved → auto-merge] →
 To Research → Researching → Planning (architect posts findings)
 ```
 
-States have types (`queue`, `active`, `hold`, `terminal`), transitions with actions (`gitPull`, `detectPr`, `mergePr`, `closeIssue`, `reopenIssue`), and review checks (`prMerged`, `prApproved`). The test phase (toTest, testing) can be enabled via `workflow.yaml` — see [Workflow](WORKFLOW.md#test-phase-optional).
+States have types (`queue`, `active`, `hold`, `terminal`), transitions with actions (`gitPull`, `detectPr`, `mergePr`, `closeIssue`, `reopenIssue`), and review checks (`prMerged`, `prApproved`). The test phase (toTest, testing) and delivery phases (toPromote/promoting, toAccept/accepting) can be enabled or skipped via `workflow.yaml` — see [Workflow](WORKFLOW.md#test-phase-optional).
 
 ### Three-Layer Configuration
 

docs/WORKFLOW.md

@@ -1,6 +1,6 @@
 # DevClaw — Workflow Reference
 
-The issue lifecycle in DevClaw is a configurable state machine defined in `workflow.yaml`. This document covers the default pipeline, all state types, review policies, and the optional test phase.
+The issue lifecycle in DevClaw is a configurable state machine defined in `workflow.yaml`. This document covers the default pipeline, all state types, review policies, the optional test phase, and the optional delivery phases for candidate promotion and acceptance.
 
 For config file format and location, see [Configuration](CONFIGURATION.md).
 
@@ -12,7 +12,7 @@ For config file format and location, see [Configuration](CONFIGURATION.md).
 Planning → To Do → Doing → To Review → PR approved → Done (auto-merge + close)
 ```
 
-Human review, no test phase. Approved PRs are auto-merged and the issue is closed.
+Human review, no test phase, and delivery phases skipped by default. Approved PRs are auto-merged, test is auto-skipped, promotion is auto-skipped, acceptance is auto-skipped, and the issue is closed.
 
 ```mermaid
 stateDiagram-v2

lib/config/loader.ts

@@ -178,6 +178,16 @@ function resolve(config: DevClawConfig): ResolvedConfig {
     initial: config.workflow?.initial ?? DEFAULT_WORKFLOW.initial,
     reviewPolicy: config.workflow?.reviewPolicy ?? DEFAULT_WORKFLOW.reviewPolicy,
     testPolicy: config.workflow?.testPolicy ?? DEFAULT_WORKFLOW.testPolicy,
+    delivery: {
+      promotion: {
+        ...DEFAULT_WORKFLOW.delivery?.promotion,
+        ...config.workflow?.delivery?.promotion,
+      },
+      acceptance: {
+        ...DEFAULT_WORKFLOW.delivery?.acceptance,
+        ...config.workflow?.delivery?.acceptance,
+      },
+    },
     roleExecution: config.workflow?.roleExecution ?? DEFAULT_WORKFLOW.roleExecution,
     states: { ...DEFAULT_WORKFLOW.states, ...config.workflow?.states },
   };

lib/config/merge.ts

@@ -48,6 +48,18 @@ export function mergeConfig(
       initial: overlay.workflow?.initial ?? base.workflow?.initial,
       reviewPolicy: overlay.workflow?.reviewPolicy ?? base.workflow?.reviewPolicy,
       testPolicy: overlay.workflow?.testPolicy ?? base.workflow?.testPolicy,
+      delivery: base.workflow?.delivery || overlay.workflow?.delivery
+        ? {
+            promotion: {
+              ...base.workflow?.delivery?.promotion,
+              ...overlay.workflow?.delivery?.promotion,
+            },
+            acceptance: {
+              ...base.workflow?.delivery?.acceptance,
+              ...overlay.workflow?.delivery?.acceptance,
+            },
+          }
+        : undefined,
       roleExecution: overlay.workflow?.roleExecution ?? base.workflow?.roleExecution,
       maxWorkersPerLevel: overlay.workflow?.maxWorkersPerLevel ?? base.workflow?.maxWorkersPerLevel,
       states: {

lib/config/schema.test.ts

@@ -0,0 +1,28 @@
+import { describe, it } from "node:test";
+import assert from "node:assert";
+import { validateWorkflowIntegrity } from "./schema.js";
+import { DEFAULT_WORKFLOW } from "../workflow/index.js";
+
+describe("validateWorkflowIntegrity delivery role validation", () => {
+  it("rejects promotion states that are not reviewer-owned", () => {
+    const workflow = structuredClone(DEFAULT_WORKFLOW);
+    workflow.delivery!.promotion!.queueState = "toTest";
+    workflow.delivery!.promotion!.activeState = "testing";
+
+    const errors = validateWorkflowIntegrity(workflow);
+
+    assert.ok(errors.includes("workflow.delivery.promotion.queueState must reference a reviewer-owned state"));
+    assert.ok(errors.includes("workflow.delivery.promotion.activeState must reference a reviewer-owned state"));
+  });
+
+  it("rejects acceptance states that are not tester-owned", () => {
+    const workflow = structuredClone(DEFAULT_WORKFLOW);
+    workflow.delivery!.acceptance!.queueState = "toReview";
+    workflow.delivery!.acceptance!.activeState = "promoting";
+
+    const errors = validateWorkflowIntegrity(workflow);
+
+    assert.ok(errors.includes("workflow.delivery.acceptance.queueState must reference a tester-owned state"));
+    assert.ok(errors.includes("workflow.delivery.acceptance.activeState must reference a tester-owned state"));
+  });
+});

lib/config/schema.ts

@@ -30,10 +30,20 @@ const StateConfigSchema = z.object({
   on: z.record(z.string(), TransitionTargetSchema).optional(),
 });
 
+const DeliveryPhaseSchema = z.object({
+  policy: z.enum(["human", "agent", "skip"]).optional(),
+  queueState: z.string().optional(),
+  activeState: z.string().optional(),
+}).optional();
+
 const WorkflowConfigSchema = z.object({
   initial: z.string(),
   reviewPolicy: z.enum(["human", "agent", "skip"]).optional(),
   testPolicy: z.enum(["skip", "agent"]).optional(),
+  delivery: z.object({
+    promotion: DeliveryPhaseSchema,
+    acceptance: DeliveryPhaseSchema,
+  }).optional(),
   roleExecution: z.enum(["parallel", "sequential"]).optional(),
   maxWorkersPerLevel: z.number().int().positive().optional(),
   states: z.record(z.string(), StateConfigSchema),
@@ -95,7 +105,7 @@ export function validateConfig(raw: unknown): void {
  * - Terminal states have no outgoing transitions
  */
 export function validateWorkflowIntegrity(
-  workflow: { initial: string; states: Record<string, { type: string; role?: string; on?: Record<string, unknown> }> },
+  workflow: { initial: string; delivery?: { promotion?: { queueState?: string; activeState?: string }; acceptance?: { queueState?: string; activeState?: string } }; states: Record<string, { type: string; role?: string; on?: Record<string, unknown> }> },
 ): string[] {
   const errors: string[] = [];
   const stateKeys = new Set(Object.keys(workflow.states));
@@ -104,6 +114,28 @@ export function validateWorkflowIntegrity(
     errors.push(`Initial state "${workflow.initial}" does not exist in states`);
   }
 
+  const validateDeliveryRef = (phase: "promotion" | "acceptance", stateKind: "queueState" | "activeState", value?: string) => {
+    if (!value) return;
+    if (!stateKeys.has(value)) {
+      errors.push(`workflow.delivery.${phase}.${stateKind} references non-existent state "${value}"`);
+      return;
+    }
+    const state = workflow.states[value];
+    const expectedType = stateKind === "queueState" ? StateType.QUEUE : StateType.ACTIVE;
+    const expectedRole = phase === "promotion" ? "reviewer" : "tester";
+    if (state?.type !== expectedType) {
+      errors.push(`workflow.delivery.${phase}.${stateKind} must reference a ${expectedType} state`);
+    }
+    if (state?.role !== expectedRole) {
+      errors.push(`workflow.delivery.${phase}.${stateKind} must reference a ${expectedRole}-owned state`);
+    }
+  };
+
+  validateDeliveryRef("promotion", "queueState", workflow.delivery?.promotion?.queueState);
+  validateDeliveryRef("promotion", "activeState", workflow.delivery?.promotion?.activeState);
+  validateDeliveryRef("acceptance", "queueState", workflow.delivery?.acceptance?.queueState);
+  validateDeliveryRef("acceptance", "activeState", workflow.delivery?.acceptance?.activeState);
+
   for (const [key, state] of Object.entries(workflow.states)) {
     if (state.type === StateType.QUEUE && !state.role) {
       errors.push(`Queue state "${key}" must have a role assigned`);

lib/dispatch/index.ts

@@ -18,7 +18,7 @@ import {
 import { resolveModel } from "../roles/index.js";
 import { notify, getNotificationConfig } from "./notify.js";
 import { loadConfig, type ResolvedRoleConfig } from "../config/index.js";
-import { ReviewPolicy, TestPolicy, resolveReviewRouting, resolveTestRouting, resolveNotifyChannel, isFeedbackState, hasReviewCheck, producesReviewableWork, hasTestPhase, detectOwner, getOwnerLabel, OWNER_LABEL_COLOR, getRoleLabelColor, STEP_ROUTING_COLOR, getStateLabels } from "../workflow/index.js";
+import { ReviewPolicy, TestPolicy, DeliveryPolicy, resolveReviewRouting, resolveTestRouting, resolveDeliveryRouting, resolveNotifyChannel, isFeedbackState, hasReviewCheck, producesReviewableWork, hasTestPhase, hasDeliveryPhase, detectOwner, getOwnerLabel, OWNER_LABEL_COLOR, getRoleLabelColor, STEP_ROUTING_COLOR, getStateLabels } from "../workflow/index.js";
 import { fetchPrFeedback, fetchPrContext, type PrFeedback, type PrContext } from "./pr-context.js";
 import { formatAttachmentsForTask } from "./attachments.js";
 import { loadRoleInstructions } from "./bootstrap-hook.js";
@@ -253,6 +253,26 @@ export async function dispatchTask(
       await provider.addLabel(issueId, testLabel);
     }
 
+    if (hasDeliveryPhase(workflow, "promotion")) {
+      const promotionPolicy = workflow.delivery?.promotion?.policy ?? DeliveryPolicy.SKIP;
+      const promotionLabel = resolveDeliveryRouting(promotionPolicy, "promotion");
+      const oldPromotionRouting = issue.labels.filter((l) => l.startsWith("promotion:"));
+      const safePromotionRouting = filterNonStateLabels(oldPromotionRouting, stateLabels);
+      if (safePromotionRouting.length > 0) await provider.removeLabels(issueId, safePromotionRouting);
+      await provider.ensureLabel(promotionLabel, STEP_ROUTING_COLOR);
+      await provider.addLabel(issueId, promotionLabel);
+    }
+
+    if (hasDeliveryPhase(workflow, "acceptance")) {
+      const acceptancePolicy = workflow.delivery?.acceptance?.policy ?? DeliveryPolicy.SKIP;
+      const acceptanceLabel = resolveDeliveryRouting(acceptancePolicy, "acceptance");
+      const oldAcceptanceRouting = issue.labels.filter((l) => l.startsWith("acceptance:"));
+      const safeAcceptanceRouting = filterNonStateLabels(oldAcceptanceRouting, stateLabels);
+      if (safeAcceptanceRouting.length > 0) await provider.removeLabels(issueId, safeAcceptanceRouting);
+      await provider.ensureLabel(acceptanceLabel, STEP_ROUTING_COLOR);
+      await provider.addLabel(issueId, acceptanceLabel);
+    }
+
     // Apply owner label if issue is unclaimed (auto-claim on pickup)
     if (opts.instanceName && !detectOwner(issue.labels)) {
       const ownerLabel = getOwnerLabel(opts.instanceName);

lib/orchestrator-intervention/engine.test.ts

@@ -48,7 +48,7 @@ describe("orchestrator intervention engine", () => {
     }
   });
 
-  it("requeues a refining issue when a hold policy matches", async () => {
+  it("does not auto-requeue a refining issue when a hold policy matches", async () => {
     const h = await createTestHarness();
     try {
       const issue = h.provider.seedIssue({ iid: 77, title: "Blocked", labels: ["Refining"] });
@@ -79,12 +79,54 @@ describe("orchestrator intervention engine", () => {
         source: "worker",
       });
 
-      assert.equal(executions[0]?.executed, true);
+      assert.equal(executions[0]?.executed, false);
+      assert.match(executions[0]?.error ?? "", /not allowed from HOLD state/i);
       const updated = await h.provider.getIssue(77);
-      assert.ok(updated.labels.includes("To Do"));
+      assert.ok(updated.labels.includes("Refining"));
+      assert.ok(!updated.labels.includes("To Do"));
       const comments = await h.provider.listComments(77);
-      assert.equal(comments.length, 1);
-      assert.match(comments[0]!.body, /Requeued after blocked/);
+      assert.equal(comments.length, 0);
+    } finally {
+      await h.cleanup();
+    }
+  });
+
+  it("does not auto-queue a refining issue via queue_issue after a blocked hold event", async () => {
+    const h = await createTestHarness();
+    try {
+      const issue = h.provider.seedIssue({ iid: 78, title: "Blocked", labels: ["Refining"] });
+      await upsertInterventionPolicy(h.workspaceDir, h.project.slug, {
+        id: "queue-blocked",
+        title: "Queue blocked issues",
+        mode: "auto",
+        issueId: 78,
+        event: { type: "workflow.hold", result: "blocked" },
+        action: { type: "queue_issue", issueId: 78 },
+      });
+
+      const executions = await recordAndApplyInterventionEvent({
+        workspaceDir: h.workspaceDir,
+        channelId: h.channelId,
+        agentId: "main",
+        project: h.project,
+        workflow: h.workflow,
+        provider: h.provider,
+        issue,
+        runCommand: h.runCommand,
+      }, {
+        eventType: "workflow.hold",
+        issueId: 78,
+        result: "blocked",
+        fromState: "Doing",
+        toState: "Refining",
+        source: "worker",
+      });
+
+      assert.equal(executions[0]?.executed, false);
+      assert.match(executions[0]?.error ?? "", /automatic queue_issue is not allowed from HOLD state/i);
+      const updated = await h.provider.getIssue(78);
+      assert.ok(updated.labels.includes("Refining"));
+      assert.ok(!updated.labels.includes("To Do"));
     } finally {
       await h.cleanup();
     }

lib/orchestrator-intervention/engine.ts

@@ -172,6 +172,9 @@ async function executePolicyAction(
       if (!currentLabel) throw new Error("issue has no recognized workflow label");
       const currentState = findStateByLabel(ctx.workflow, currentLabel);
       if (!currentState) throw new Error(`unknown state for ${currentLabel}`);
+      if (currentState.type === StateType.HOLD) {
+        throw new Error(`automatic requeue is not allowed from HOLD state ${currentLabel}; require explicit human restart via task_start(confirmHoldRestart=true)`);
+      }
       const target = resolveTarget(ctx.workflow, currentLabel, currentState);
       if (target.transitioned) {
         await ctx.provider.transitionLabel(issue.iid, currentLabel, target.targetLabel);
@@ -192,6 +195,9 @@ async function executePolicyAction(
       if (!currentLabel) throw new Error(`issue #${targetIssueId} has no recognized workflow label`);
       const currentState = findStateByLabel(ctx.workflow, currentLabel);
       if (!currentState) throw new Error(`unknown state for ${currentLabel}`);
+      if (currentState.type === StateType.HOLD) {
+        throw new Error(`automatic queue_issue is not allowed from HOLD state ${currentLabel}; require explicit human restart via task_start(confirmHoldRestart=true)`);
+      }
       const target = resolveTarget(ctx.workflow, currentLabel, currentState);
       if (target.transitioned) {
         await ctx.provider.transitionLabel(targetIssueId, currentLabel, target.targetLabel);