Conversation
run_scope_check compared claims by agent_of(owner) (agent TYPE only,
e.g. "claude-code"), not the full owner_id() (agent:instance).
claims::owner_id() is deliberately instance-scoped, and claim
release/done require an exact owner_id() match -- the mismatch meant
a live claim from a DIFFERENT session of the same agent type (a
sibling session, or one orphaned by a crashed prior session) got
misclassified as the invoker's own claim, producing a spurious
OutOfTree denial ("you hold claim X -- do this work in its isolated
worktree") for a claim the invoker cannot actually release.
Extracted the split into partition_claims_by_owner, now matching on
the exact owner_id(), with regression tests covering both the
false-positive (different instance treated as mine) and the
second-order bug (sibling-instance claims silently exempted from
scope enforcement).
Agentflare-Agent: claude-code_2-1-227_agent
Agentflare-Branch: task/444
Agentflare-Item: 444
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CLI now classifies claims by full ChangesOwner scope partitioning
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/git.rs (1)
1235-1250: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the scope-enforcement result.
This test only verifies that the sibling claim enters
others. It does not verify that a changed path insidecrates/foo/producesscope::ScopeVerdict::Overlapping.Call
scope::classify_scopeswithothersand assert the overlapping verdict. This validates the regression described by the test name and PR objective.🤖 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/git.rs` around lines 1235 - 1250, Extend partition_claims_by_owner_enforces_scope_between_sibling_instances() by calling scope::classify_scopes with the returned others claims and a changed path under crates/foo/. Assert that the result is scope::ScopeVerdict::Overlapping while preserving the existing assertions that the sibling claim is placed in others.
🤖 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.
Nitpick comments:
In `@src/cli/git.rs`:
- Around line 1235-1250: Extend
partition_claims_by_owner_enforces_scope_between_sibling_instances() by calling
scope::classify_scopes with the returned others claims and a changed path under
crates/foo/. Assert that the result is scope::ScopeVerdict::Overlapping while
preserving the existing assertions that the sibling claim is placed in others.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf74ed68-3145-4c39-9901-1383f8a8486a
📒 Files selected for processing (1)
src/cli/git.rs
…e verdict CodeRabbit nitpick on PR 458: the sibling-instance test proved the claim lands in others but never proved that translates into real enforcement. Add the classify_scopes call and assert Overlapping. Agentflare-Agent: claude-code_2-1-227_agent Agentflare-Branch: task/444 Agentflare-Item: 444
Agentflare-Agent: claude-code_2-1-227_agent Agentflare-Branch: task/444 Agentflare-Item: 444
run_scope_check(src/cli/git.rs) was comparing live claims byclaims::agent_of(owner)— agent TYPE only (e.g. "claude-code"), stripping the:instancesuffix — to decide which claim is "mine" for the OutOfTree check. Butclaims::owner_id()is deliberately instance-scoped, andclaim(action="release"|"done")require an exactowner_id()match. This mismatch meant a live claim from a DIFFERENT session of the same agent type (a concurrent sibling session, or one orphaned by a crashed prior session) got misclassified as the invoker's own claim, producing a spuriousOutOfTreedenial ("you hold claim X -- do this work in its isolated worktree, not the canonical checkout") for a claim the invoker doesn't actually own and has no way to release — a real deadlock captured live via a vent report (orphaned claimitem#JgotedejAG-2Mopg6zisSowned by a different instance blockinggit commitwith no escape hatch).Fix: extracted the split into
partition_claims_by_owner(live, owner), matching on the fullowner_id()string instead ofagent_of(). Added 3 regression tests: (1) a different instance of the same agent type must not be treated as mine, (2) the exact same instance still matches, (3) a sibling instance's scoped claim correctly lands inothersfor scope enforcement (second-order bug: same-agent-type-different-instance claims were previously excluded fromothersentirely too, so scope enforcement between two concurrent sessions of the same agent type never fired).Verified:
cargo build --bin agentflareclean, all 3 new tests pass plus the fullclaims::test suite (16 tests, no regressions).Summary by CodeRabbit