fix: three dispatch-reliability bugs found dogfooding autonomous work - #413
Merged
Conversation
added 3 commits
August 9, 2026 14:41
item::claim() deliberately stores the raw claim owner (<agent>:<instance>) into assignee_agent on acquire, not just the bare agent name — intentional, pinned by existing tests. But resolve_confirmed_agent (supervisor.rs's discovery tick) and resolve_agent's assignee fallback (cli/work.rs) matched that field against the agent registry with an exact string comparison, so once an item had been claimed at least once, its own assignee could never be recognized again on a later dispatch attempt — it would silently get skipped or fall through to other routing rules. Both now reuse item::agent_part (already used internally by claim()'s own handoff-freeze comparison) to strip the instance suffix before matching, instead of duplicating that logic or changing the write side.
item_done unconditionally called mark_completed whenever a claim owner called done and no PR resulted, regardless of whether the claimed branch had any real commits on it. A headless work run that only replies with text (no tool use, no commits) previously exited 0 and sailed straight through to being marked "completed" with zero code changed. Reuses branch_diverged (extracted from push_branch's existing squash-merge guard, now shared between flare-git-core and the main binary) to tell a genuinely empty run apart from one whose push/PR failed for another reason. A genuinely empty run now releases the claim and cleans up the (by-definition clean) worktree so the item is available for retry, but is left unchanged rather than falsely marked completed. Agentflare-Agent: claude-code_2-1-226_agent Agentflare-Branch: task/48 Agentflare-Item: 48
Previously handoff(item_id=X) only ever set assignee_agent — queuing an existing item for the supervisor's discovery tick required a separate item(action=add_label) call every time, unlike a brand-new handed-off item which already got labeled ready-for-work automatically. Extends that to the explicit item_id path, but only when it's actually safe: the item has never been claimed and is still in a genuinely fresh state group (backlog/unstarted/triage). An item that's already claimed, in progress, in review, or completed is left alone, same as the existing reply/continuation path already protects against — re-queuing live work would be wrong regardless of which code path reached it.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR canonicalizes instance-suffixed agent assignments, adds shared branch-divergence checks, changes no-commit completion handling, and refines ready-label behavior for explicit handoffs. ChangesAgentflare workflow behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant item_done
participant Worktree
participant flare_git_core
participant ClaimLedger
Agent->>item_done: submit completion
item_done->>Worktree: check task branch divergence
Worktree->>flare_git_core: compare task branch with target
flare_git_core-->>Worktree: return divergence result
item_done->>ClaimLedger: release claim
item_done->>Worktree: clean up worktree
item_done-->>Agent: return completed or unchanged status
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@crates/flare-git-core/src/worktree.rs`:
- Around line 454-466: Separate commit existence from content divergence: add a
fallible helper in crates/flare-git-core/src/worktree.rs near branch_diverged
that reports whether target_branch..branch contains commits, expose it through
src/worktree.rs, and update item_done in src/mcp_server/item.rs so only a
successful zero-commit check marks the item unchanged; preserve the claim and
worktree on Git-check errors while allowing committed-but-already-applied
branches to complete. Add tests in src/mcp_server/tests/action_tests.rs covering
completion with committed content already on target and claim release for the
true zero-commit case.
In `@src/mcp_server/handoff.rs`:
- Around line 110-115: In the handoff eligibility check around current_owner,
use the fallible durable claim-history lookup to set never_claimed, and treat
lookup errors as not eligible so the item is not requeued. In
src/mcp_server/handoff.rs lines 110-115, replace the active-owner check
accordingly; in lines 490-515, release or expire the seeded claim, move the item
to an eligible state, and assert that explicit handoff does not add
ready-for-work.
- Line 115: Update the ready-label handling in handoff to propagate the Result
from agentflare_backend::item::add_label instead of discarding it with let _.
Ensure any attachment failure is returned through handoff so the operation does
not report success when the item was not queued.
🪄 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: 34a01646-9927-4df1-a4bb-3eaf6cc4839b
📒 Files selected for processing (8)
crates/agentflare-backend/src/item.rscrates/flare-git-core/src/worktree.rssrc/cli/work.rssrc/mcp_server/handoff.rssrc/mcp_server/item.rssrc/mcp_server/tests/action_tests.rssrc/supervisor.rssrc/worktree.rs
Comment on lines
+454
to
+466
| pub fn branch_diverged(repo_root: &Path, branch: &str, target_branch: &str) -> bool { | ||
| match run_git_in( | ||
| repo_root, | ||
| &["rev-list", "--count", &format!("{target_branch}..{branch}")], | ||
| ) { | ||
| Ok(count) if count != "0" => {} | ||
| _ => return false, | ||
| } | ||
| !run_git_in_ok( | ||
| repo_root, | ||
| &["diff", "--quiet", &format!("{target_branch}..{branch}")], | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Separate commit existence from push/PR divergence.
branch_diverged returns false when the branch has zero commits, when target already has identical content after a squash merge, and when a Git query fails. item_done treats all three cases as an empty run. It then cleans the worktree, releases the claim, and leaves the item unchanged. A branch with committed work that is already present on target cannot complete through this path.
crates/flare-git-core/src/worktree.rs#L454-L466: add a fallible helper that reports whethertarget_branch..branchhas commits. Keepbranch_divergedfor push and PR suppression.src/worktree.rs#L25-L34: expose the fallible commit-existence helper.src/mcp_server/item.rs#L645-L684: classify an item as unchanged only when the commit check succeeds and reports zero commits. Preserve the claim and worktree when the Git check fails.src/mcp_server/tests/action_tests.rs#L357-L405: add coverage for a task branch with committed content already applied to target. Assert completion. Also assert that the true zero-commit case releases the claim for a retry.
📍 Affects 4 files
crates/flare-git-core/src/worktree.rs#L454-L466(this comment)src/worktree.rs#L25-L34src/mcp_server/item.rs#L645-L684src/mcp_server/tests/action_tests.rs#L357-L405
🤖 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 `@crates/flare-git-core/src/worktree.rs` around lines 454 - 466, Separate
commit existence from content divergence: add a fallible helper in
crates/flare-git-core/src/worktree.rs near branch_diverged that reports whether
target_branch..branch contains commits, expose it through src/worktree.rs, and
update item_done in src/mcp_server/item.rs so only a successful zero-commit
check marks the item unchanged; preserve the claim and worktree on Git-check
errors while allowing committed-but-already-applied branches to complete. Add
tests in src/mcp_server/tests/action_tests.rs covering completion with committed
content already on target and claim release for the true zero-commit case.
Comment on lines
+110
to
+115
| let never_claimed = agentflare_backend::claim::current_owner(conn, id).is_none(); | ||
| if never_claimed | ||
| && matches!(state.group_name.as_str(), "backlog" | "unstarted" | "triage") | ||
| && let Some(ready_id) = ready_label_id(conn, &project.id) | ||
| { | ||
| let _ = agentflare_backend::item::add_label(conn, id, &ready_id); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use durable claim history for the never_claimed gate.
current_owner only returns an active claim. A released or expired prior claim returns None. If that item later returns to backlog, unstarted, or triage, this code adds ready-for-work despite the requirement that only never-claimed items are requeued.
src/mcp_server/handoff.rs#L110-L115: replacecurrent_ownerwith a fallible historical-claim check. Do not requeue when claim history cannot be read.src/mcp_server/handoff.rs#L490-L515: release or expire the seeded claim, return the item to an eligible state, then assert that the explicit handoff does not addready-for-work.
📍 Affects 1 file
src/mcp_server/handoff.rs#L110-L115(this comment)src/mcp_server/handoff.rs#L490-L515
🤖 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/mcp_server/handoff.rs` around lines 110 - 115, In the handoff eligibility
check around current_owner, use the fallible durable claim-history lookup to set
never_claimed, and treat lookup errors as not eligible so the item is not
requeued. In src/mcp_server/handoff.rs lines 110-115, replace the active-owner
check accordingly; in lines 490-515, release or expire the seeded claim, move
the item to an eligible state, and assert that explicit handoff does not add
ready-for-work.
| && matches!(state.group_name.as_str(), "backlog" | "unstarted" | "triage") | ||
| && let Some(ready_id) = ready_label_id(conn, &project.id) | ||
| { | ||
| let _ = agentflare_backend::item::add_label(conn, id, &ready_id); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Propagate the ready-label attachment error.
When the ready label exists, add_label can still fail, for example after concurrent label deletion or during a database error. Line 115 discards that error, so handoff reports success but does not queue the item.
Proposed fix
- let _ = agentflare_backend::item::add_label(conn, id, &ready_id);
+ agentflare_backend::item::add_label(conn, id, &ready_id)
+ .map_err(map_backend_err)?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let _ = agentflare_backend::item::add_label(conn, id, &ready_id); | |
| agentflare_backend::item::add_label(conn, id, &ready_id) | |
| .map_err(map_backend_err)?; |
🤖 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/mcp_server/handoff.rs` at line 115, Update the ready-label handling in
handoff to propagate the Result from agentflare_backend::item::add_label instead
of discarding it with let _. Ensure any attachment failure is returned through
handoff so the operation does not report success when the item was not queued.
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three real bugs found and fixed while dogfooding agentflare's own autonomous
workdispatch (item #43) against a fresh design/plan cycle.assignee_agentinstance-suffix not recognized on redispatch —item::claimdeliberately stores the raw claim owner (<agent>:<instance>) intoassignee_agenton acquire.resolve_confirmed_agent(supervisor's discovery tick) andresolve_agent's assignee fallback (agentflare work's auto-routing) matched that field with an exact string comparison, so once an item had been claimed even once, its own assignee could never be recognized again on a later dispatch attempt — it would silently get skipped. Both now reuseitem::agent_part(already used internally byclaim()'s own handoff-freeze comparison) to strip the instance suffix before matching.item donemarked an item completed with zero work delivered —item_doneunconditionally calledmark_completedwhenever the claim owner calleddoneand no PR resulted, regardless of whether the claimed branch had any real commits. A headless run that only replied with text (no tool use, no commits) exited 0 and sailed straight through to "completed." Reusesbranch_diverged(extracted frompush_branch's existing squash-merge guard) to tell a genuinely empty run apart from one whose push/PR failed for another reason — a genuinely empty run now releases the claim and cleans up the (by-definition clean) worktree so the item is available for retry, but is left unchanged rather than falsely marked completed.handoff(item_id=X)required a separateadd_labelcall to actually queue dispatch — only a brand-new handed-off item got auto-labeledready-for-work; an explicititem_idhandoff onto an existing item only ever set the assignee. Extends auto-queuing to that path too, but only when it's safe (never claimed, still in a fresh state group) — an already-claimed/in-progress/done item is left alone, same protection the reply/continuation path already had.Test plan
cargo build --workspacecleancargo test --bin agentflare -- --skip fallback_scan— 1150 passed, 0 failed (fallback_scan_finds_matches_and_skips_target_diris a pre-existing, environment-specific failure confirmed to fail identically on unmodifiedmaster)resolve_confirmed_agent_recognizes_an_instance_suffixed_assignee,resolve_agent_falls_back_to_an_instance_suffixed_assignee,item_done_without_new_commits_leaves_the_item_unchanged(renamed + updated from a test that pinned the old buggy behavior),a_reply_to_an_already_claimed_item_is_not_relabeled_ready_for_work,an_explicit_item_id_handoff_labels_a_still_fresh_unclaimed_item~/.cargo/bin/agentflare, restarted the daemon, confirmed the assignee_agent fix works against a real previously-claimed item in the running systemSummary by CodeRabbit
New Features
Bug Fixes