Repository navigation
Conversation
Item #94 exited its job cleanly (agent_jobs.state='exited') while its item_claims row stayed 'claimed' forever, wedging it past all 3 retries. execute_work only released the claim on the intentional bail-out points it explicitly called release_and_comment/item_release for; item_done returning Err fell through none of them, and a panic anywhere after the claim was acquired skipped them all outright. Add ClaimGuard, a Drop-based backstop created right after the claim succeeds and disarmed only once the claim's fate is definitively decided (item_done Ok, or a confirmed-successful explicit release). Drop runs on every remaining exit, including an unwinding panic, so the claim is released immediately instead of waiting out the job's timeout or the claim's own TTL. Agentflare-Agent: claude-code Agentflare-Branch: task/100 Agentflare-Item: 100
📝 WalkthroughWalkthroughThe change adds a panic-safe ChangesClaim lifecycle protection
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/work.rs`:
- Around line 914-923: Update the item_release handling in the mcp claim-release
flow to inspect the successful response payload and disarm claim_guard only when
its released field is true. Do not treat every Ok result as a release; preserve
the guard fallback when released is false or the request fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fe922ba6-5aed-4e4e-8895-70f200bddd3e
📒 Files selected for processing (1)
src/cli/work.rs
| if mcp | ||
| .item_release(ItemRequest { | ||
| action: "release".into(), | ||
| id: Some(item_id.into()), | ||
| ..Default::default() | ||
| }) | ||
| .is_ok() | ||
| { | ||
| claim_guard.disarm(); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# The lean-ctx tools are unavailable in this review context.
# Locate AgentflareMcp::item_release and inspect its semantic response contract.
rg -n -C 8 --type rust '\bfn\s+item_release\b|\bitem_release\s*\(' src
rg -n -C 8 --type rust '"released"|released\s*:|action:\s*"release"' srcRepository: getappz/agentflare
Length of output: 21062
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- item_release implementation ---'
sed -n '619,684p' src/mcp_server/item.rs
printf '%s\n' '--- release backend implementation and result shape ---'
rg -n -C 12 --type rust 'pub\s+(fn|async fn)\s+release\b|fn\s+release\b|struct\s+ItemRequest|item_release\(' .Repository: getappz/agentflare
Length of output: 20771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
server = Path("src/mcp_server/item.rs").read_text()
work = Path("src/cli/work.rs").read_text()
backend = Path("crates/agentflare-db-kit/src/claim.rs").read_text()
release_body = re.search(
r'pub\(crate\) fn item_release\b.*?(?=\n pub\(crate\) fn item_done\b)',
server,
re.S,
).group(0)
assert 'Ok(serde_json::json!({"released": ok, "item_id": item_id}).to_string())' in release_body
assert 'agentflare_backend::claim::release' in release_body
assert 'conn.execute(&sql, params.as_slice())? > 0' in backend
hold = re.search(
r'if let Some\(reason\) = detect_hold_signal.*?(?=\n \})',
work,
re.S,
).group(0)
assert '.item_release(ItemRequest' in hold
assert '.is_ok()' in hold
assert 'claim_guard.disarm()' in hold
print("item_release returns a JSON `released` boolean, while the hold path checks only Result::is_ok().")
print("The backend release result is the affected-row boolean, so Ok(...released:false...) is representable.")
PYRepository: getappz/agentflare
Length of output: 355
Disarm the claim guard only when released is true.
item_release can return Ok({"released": false, ...}) when no claim is deleted. .is_ok() treats this as success and suppresses the guard fallback. Parse the response and disarm only when released == true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/work.rs` around lines 914 - 923, Update the item_release handling in
the mcp claim-release flow to inspect the successful response payload and disarm
claim_guard only when its released field is true. Do not treat every Ok result
as a release; preserve the guard fallback when released is false or the request
fails.
Source: Coding guidelines
Summary
Item #94 exited its job cleanly (
agent_jobs.state='exited') while itsitem_claimsrow stayedclaimedforever, wedging it past all 3 retries — and the same pattern hit #64 and #68.execute_workonly released the claim on the intentional bail-out points it explicitly calledrelease_and_comment/item_releasefor;item_donereturningErrfell through none of them, and a panic anywhere after the claim was acquired skipped them all outright.Adds
ClaimGuard, aDrop-based backstop created right after the claim succeeds and disarmed only once the claim's fate is definitively decided (item_doneOk, or a confirmed-successful explicit release).Dropruns on every remaining exit, including an unwinding panic, so the claim is released immediately instead of waiting out the job's timeout or the claim's own TTL.Test plan
cargo test --bin agentflare -- cli::work::— 44 tests pass, including 3 newClaimGuardtests (drop-releases, disarmed-stays-held, releases-through-panic-unwind)cargo fmt --check,cargo clippycleancargo buildcleanSummary by CodeRabbit
Bug Fixes
Tests