fix: release item claim on finalize success

crates/agentflare-backend/src/claim.rs

@@ -86,6 +86,30 @@ pub fn current_owner(conn: &Connection, item_id: &str) -> Option<String> {
         .map(|c| c.owner)
 }
 
+/// A live (non-stale) `claimed` lease on an item, if any.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct LiveClaimOnItem {
+    pub owner: String,
+    pub age_secs: i64,
+}
+
+/// Returns the live claim holder on `item_id`, if one exists.
+pub fn live_claim_on_item(
+    conn: &Connection,
+    item_id: &str,
+    now: i64,
+    ttl_secs: i64,
+) -> rusqlite::Result<Option<LiveClaimOnItem>> {
+    let claims = LEDGER.list(conn, false, now, ttl_secs)?;
+    Ok(claims
+        .iter()
+        .find(|c| c.key == [item_id])
+        .map(|c| LiveClaimOnItem {
+            owner: c.owner.clone(),
+            age_secs: now - c.heartbeat_at,
+        }))
+}
+
 /// Returns true if there is an active (live, non-stale) claim on this item
 /// whose owner differs from `owner`. Used by the comment edit/delete gates
 /// to prevent modifying a comment when another agent has started work.

src/mcp_server/item.rs

@@ -1150,6 +1150,8 @@ impl AgentflareMcp {
             return Err(ErrorData::invalid_params("id is required", None));
         }
         let assignee_agent = req.assignee_agent.clone();
+        let now = crate::claims::now();
+        let ttl = crate::mcp_server::types::backend_claim_ttl_secs();
         self.with_backend_db(|conn| {
             let item_id = self.resolve_item_id(conn, &raw)?;
             let outcome = agentflare_backend::item::redispatch(
@@ -1159,14 +1161,34 @@ impl AgentflareMcp {
             )
             .map_err(map_backend_err)?;
             match outcome {
-                agentflare_backend::item::RedispatchOutcome::Ready { assignee_agent } => Ok(
-                    serde_json::json!({
+                agentflare_backend::item::RedispatchOutcome::Ready { assignee_agent } => {
+                    let effective_ttl =
+                        agentflare_backend::claim::effective_ttl_secs(conn, &item_id, ttl);
+                    let live = agentflare_backend::claim::live_claim_on_item(
+                        conn,
+                        &item_id,
+                        now,
+                        effective_ttl,
+                    )
+                    .map_err(|e| ErrorData::internal_error(e.to_string(), None))?;
+                    let blocked_by = live.filter(|c| c.owner != assignee_agent);
+                    let dispatchable = blocked_by.is_none();
+                    let mut resp = serde_json::json!({
                         "item_id": item_id,
                         "ready": true,
+                        "dispatchable": dispatchable,
                         "assignee_agent": assignee_agent,
-                    })
-                    .to_string(),
-                ),
+                    });
+                    if let Some(live) = blocked_by {
+                        resp["blocked_by_live_claim"] = serde_json::json!({
+                            "owner": live.owner,
+                            "age_secs": live.age_secs,
+                            "ttl_secs": effective_ttl,
+                            "reason": "another agent already holds a live claim on this item",
+                        });
+                    }
+                    Ok(resp.to_string())
+                },
                 agentflare_backend::item::RedispatchOutcome::NoAssignee => {
                     Err(ErrorData::invalid_params(
                         format!(

src/mcp_server/tests/action_tests.rs

@@ -1047,6 +1047,46 @@ fn item_release_errors_when_a_different_owner_holds_a_live_claim() {
     );
 }
 
+#[test]
+fn item_redispatch_reports_dispatch_blocked_when_a_live_claim_remains() {
+    let (_tmp, s) = harness();
+    let created: serde_json::Value =
+        serde_json::from_str(&s.item(Parameters(empty_item_create("Test"))).unwrap()).unwrap();
+    let item_id = created["id"].as_str().unwrap().to_string();
+
+    s.item(Parameters(ItemRequest {
+        action: "update".into(),
+        id: Some(item_id.clone()),
+        assignee_agent: Some("opencode".into()),
+        ..Default::default()
+    }))
+    .unwrap();
+
+    seed_claim(&s, &item_id, "opencode:dead-job", 60);
+
+    let resp: serde_json::Value = serde_json::from_str(
+        &s.item(Parameters(ItemRequest {
+            action: "redispatch".into(),
+            id: Some(item_id.clone()),
+            ..Default::default()
+        }))
+        .unwrap(),
+    )
+    .unwrap();
+
+    assert_eq!(resp["ready"], serde_json::Value::Bool(true));
+    assert_eq!(resp["dispatchable"], serde_json::Value::Bool(false));
+    assert_eq!(
+        resp["blocked_by_live_claim"]["owner"].as_str(),
+        Some("opencode:dead-job")
+    );
+    assert_eq!(
+        resp["blocked_by_live_claim"]["reason"].as_str(),
+        Some("another agent already holds a live claim on this item")
+    );
+    assert!(resp["blocked_by_live_claim"]["age_secs"].as_i64().unwrap() >= 60);
+}
+
 #[test]
 fn item_release_reclaims_and_releases_a_stale_claim_from_an_abandoned_owner() {
     let (_tmp, s) = harness();

src/work_item_pipeline.rs

@@ -468,6 +468,8 @@ pub(crate) fn build_sdd_loop_step(
 ///    human with a comment instead of opening a PR on unreviewed code, since
 ///    this step has no access to `supervisor`'s label-id lookups for a real
 ///    relabel (that stays the supervisor's job on its next discovery tick).
+///    The job is finished either way — release the claim so redispatch /
+///    supervisor discovery can pick the item back up.
 /// 4. Otherwise — the success path: `item_done`, then the same
 ///    `cap_reply_for_comment`/`format_success_comment`/comment/notify
 ///    sequence `execute_work` runs today.
@@ -477,6 +479,24 @@ pub(crate) fn build_sdd_loop_step(
 /// way `coder`/`review_or_fix`'s agent dispatch can, and unlike those two,
 /// a failure here has already done the real work and just needs to land the
 /// result.
+///
+/// Best-effort claim release on every terminal success except when
+/// `item_done` deliberately left the lease held for an open PR (`in_review`).
+fn finalize_release_claim_best_effort(
+    mcp: &crate::mcp_server::AgentflareMcp,
+    item_id: &str,
+    leave_claim_held: bool,
+) {
+    if leave_claim_held {
+        return;
+    }
+    let _ = mcp.item_release(ItemRequest {
+        action: "release".into(),
+        id: Some(item_id.to_string()),
+        ..Default::default()
+    });
+}
+
 pub(crate) fn build_finalize_step(
     mcp: std::sync::Arc<AgentflareMcp>,
     item_id: String,
@@ -492,11 +512,7 @@ pub(crate) fn build_finalize_step(
             Box::pin(async move {
                 crate::claims::with_owner_override(owner, || {
                     if let Some(reason) = ctx.data.hold_reason.clone() {
-                        let _ = mcp.item_release(ItemRequest {
-                            action: "release".into(),
-                            id: Some(item_id.clone()),
-                            ..Default::default()
-                        });
+                        finalize_release_claim_best_effort(&mcp, &item_id, false);
                         let body = format!("## agentflare work — on hold\n\n{reason}");
                         let _ = mcp.comment_impl(CommentRequest {
                             action: "create".into(),
@@ -520,11 +536,7 @@ pub(crate) fn build_finalize_step(
                         } else {
                             ctx.data.review_findings.join("\n\n---\n\n")
                         };
-                        let _ = mcp.item_release(ItemRequest {
-                            action: "release".into(),
-                            id: Some(item_id.clone()),
-                            ..Default::default()
-                        });
+                        finalize_release_claim_best_effort(&mcp, &item_id, false);
                         let body = format!("## agentflare work — review findings\n\n{findings}");
                         let _ = mcp.comment_impl(CommentRequest {
                             action: "create".into(),
@@ -540,6 +552,7 @@ pub(crate) fn build_finalize_step(
 
                     if ctx.data.review_issues.is_some() {
                         let issues = ctx.data.review_issues.clone().unwrap_or_default();
+                        finalize_release_claim_best_effort(&mcp, &item_id, false);
                         let _ = mcp.comment_impl(CommentRequest {
                             action: "create".into(),
                             item_id: Some(item_id.clone()),
@@ -567,6 +580,8 @@ pub(crate) fn build_finalize_step(
                     let done_val: serde_json::Value =
                         serde_json::from_str(&done_resp).unwrap_or(serde_json::Value::Null);
                     ctx.data.pr_url = done_val["pr_url"].as_str().map(str::to_string);
+                    let leave_claim_held = done_val["status"].as_str() == Some("in_review");
+                    finalize_release_claim_best_effort(&mcp, &item_id, leave_claim_held);
 
                     let comment_reply = crate::cli::work::cap_reply_for_comment(
                         &mcp,

src/work_item_pipeline/tests.rs

@@ -292,6 +292,98 @@ async fn finalize_step_uses_accumulated_review_findings_when_last_report_was_cle
     panic!("finalize step did not complete");
 }
 
+#[tokio::test]
+async fn finalize_step_releases_claim_when_human_review_gate_is_hit() {
+    let (mcp, _backend_tmp, _repo_tmp, item_id, _project_id, _worktree_path) =
+        crate::mcp_server::tests::mcp_with_claimed_item("Human-review finalize test item");
+    let mcp = Arc::new(mcp);
+    let owner = crate::claims::owner_id();
+
+    let data = WorkItemData {
+        review_issues: Some("- still broken".into()),
+        ..Default::default()
+    };
+    let step = build_finalize_step(mcp.clone(), item_id.clone(), None, owner.clone());
+    let wf = WorkflowDefinition::new(WORKFLOW_ID, "work item").add_step(step);
+    let engine = WorkflowEngine::<WorkItemData, InMemoryStore<WorkItemData>>::new();
+    engine.register_workflow(wf).unwrap();
+    let run_id = engine
+        .start_workflow(WorkflowId::new(WORKFLOW_ID), data, String::new())
+        .await
+        .unwrap();
+
+    for _ in 0..50 {
+        let state = engine.get_status(run_id).await.unwrap();
+        if state.status == flare_workflow::WorkflowStatus::Completed {
+            let still_claimed = mcp
+                .with_backend_db(|conn| {
+                    agentflare_backend::claim::is_owner(conn, &item_id, &owner)
+                        .map_err(|e| e.to_string())
+                })
+                .unwrap()
+                .unwrap();
+            assert!(
+                !still_claimed,
+                "finalize must release the claim when gating for human review"
+            );
+            return;
+        }
+        if state.status == flare_workflow::WorkflowStatus::Failed {
+            panic!(
+                "finalize must succeed for human-review gate: {:?}",
+                state.error
+            );
+        }
+        tokio::time::sleep(std::time::Duration::from_millis(10)).await;
+    }
+    panic!("finalize step did not complete");
+}
+
+#[tokio::test]
+async fn finalize_step_releases_claim_after_review_only_success() {
+    let (mcp, _backend_tmp, _repo_tmp, item_id, _project_id, _worktree_path) =
+        crate::mcp_server::tests::mcp_with_claimed_item("Review-only claim release test item");
+    let mcp = Arc::new(mcp);
+    let owner = crate::claims::owner_id();
+
+    let data = WorkItemData {
+        review_only: true,
+        last_report: Some("Found an issue.".to_string()),
+        ..Default::default()
+    };
+    let step = build_finalize_step(mcp.clone(), item_id.clone(), None, owner.clone());
+    let wf = WorkflowDefinition::new(WORKFLOW_ID, "work item").add_step(step);
+    let engine = WorkflowEngine::<WorkItemData, InMemoryStore<WorkItemData>>::new();
+    engine.register_workflow(wf).unwrap();
+    let run_id = engine
+        .start_workflow(WorkflowId::new(WORKFLOW_ID), data, String::new())
+        .await
+        .unwrap();
+
+    for _ in 0..50 {
+        let state = engine.get_status(run_id).await.unwrap();
+        if state.status == flare_workflow::WorkflowStatus::Completed {
+            let still_claimed = mcp
+                .with_backend_db(|conn| {
+                    agentflare_backend::claim::is_owner(conn, &item_id, &owner)
+                        .map_err(|e| e.to_string())
+                })
+                .unwrap()
+                .unwrap();
+            assert!(
+                !still_claimed,
+                "finalize must release the claim after a review-only run"
+            );
+            return;
+        }
+        if state.status == flare_workflow::WorkflowStatus::Failed {
+            panic!("finalize must not fail for review-only: {:?}", state.error);
+        }
+        tokio::time::sleep(std::time::Duration::from_millis(10)).await;
+    }
+    panic!("finalize step did not complete");
+}
+
 // requires a real headless agent binary; run manually / in an
 // environment with one installed — the mock-sender variant right below
 // covers the same metadata-persistence assertion unconditionally.