Conversation
handoff_impl only special-cased "github" -- any other recipient string, including a typo or thematic word, fell through and became assignee_agent with no validation, silently orphaning the item since nothing could ever claim or dispatch it (reproduced live: a handoff to "gastown" did exactly this). Reuses agent_registry::agent_by_name, the same lookup cli/work.rs's --agent flag already validates against, and rejects with the list of valid recipients instead of silently creating dead work. Agentflare-Agent: claude-code Agentflare-Branch: task/52 Agentflare-Item: 52
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 35 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 handoff implementation now validates recipients after GitHub handling. Unknown recipients produce invalid-parameter errors that list registered agents and ChangesHandoff recipient validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🤖 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/mcp_server/handoff.rs`:
- Around line 89-101: Update the recipient validation in the handoff flow to
retain the agent returned by agent_registry::agent_by_name(&recipient) and use
its canonical id for assignee_agent, metadata, response, and dispatch-related
fields. Preserve the existing invalid-recipient error and special handling for
"github", while ensuring aliases are never persisted or returned.
- Around line 691-695: Update the test around handoff_impl to assert that the
returned err has code ErrorCode::INVALID_PARAMS, while preserving the existing
message-content assertions.
🪄 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: b3579b23-8d66-4c90-ac09-16b8cf839919
📒 Files selected for processing (1)
src/mcp_server/handoff.rs
| if agent_registry::agent_by_name(&recipient).is_none() { | ||
| let mut valid: Vec<&str> = | ||
| agent_registry::REGISTRY.iter().map(|s| s.id.as_str()).collect(); | ||
| valid.push("github"); | ||
| valid.sort_unstable(); | ||
| return Err(ErrorData::invalid_params( | ||
| format!( | ||
| "recipient '{recipient}' is not a known agent -- valid recipients: {}", | ||
| valid.join(", ") | ||
| ), | ||
| None, | ||
| )); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(handoff\.rs|agent_registry|.*registry.*)\b' || true
printf '%s\n' '--- symbols and recipient references ---'
rg -n -C 4 'agent_by_name|recipient|reuse|assignee|dispatch' src/mcp_server/handoff.rs src 2>/dev/null | head -n 500
printf '%s\n' '--- handoff outline ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline src/mcp_server/handoff.rs
else
echo 'ast-grep unavailable'
fiRepository: getappz/agentflare
Length of output: 38565
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- handoff implementation ---'
sed -n '1,270p' src/mcp_server/handoff.rs
printf '%s\n' '--- agent registry implementation ---'
cat -n crates/agent-registry/src/lib.rs
printf '%s\n' '--- registry and router implementations ---'
cat -n crates/agent-registry/src/registry.rs
cat -n crates/agent-registry/src/router.rs
printf '%s\n' '--- canonicalize and agent_by_name call sites ---'
rg -n -C 8 'canonicalize|agent_by_name|assignee_agent|list_by_assignee_agent|dispatch_item' src crates --glob '*.rs' | head -n 1000Repository: getappz/agentflare
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- remaining handoff response path ---'
sed -n '240,360p' src/mcp_server/handoff.rs
printf '%s\n' '--- exact assignee lookup implementations ---'
rg -n -l 'fn list_by_assignee_agent|list_by_assignee_agent' crates src --glob '*.rs' | while read -r f; do
echo "--- $f"
rg -n -C 18 'fn list_by_assignee_agent|list_by_assignee_agent' "$f"
done
printf '%s\n' '--- exact dispatch and claim assignee resolution ---'
rg -n -C 14 'assignee_agent|resolve_confirmed_agent|dispatch_item|current_owner' src crates --glob '*.rs' \
| rg -B 14 -A 14 'assignee_agent|resolve_confirmed_agent|dispatch_item|current_owner' \
| head -n 1200
printf '%s\n' '--- handoff tests around recipient validation and reuse ---'
sed -n '680,940p' src/mcp_server/handoff.rsRepository: getappz/agentflare
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- supervisor resolution ---'
rg -n -C 25 'fn resolve_confirmed_agent|resolve_confirmed_agent\(' src/supervisor.rs src/supervisor --glob '*.rs' 2>/dev/null || true
printf '%s\n' '--- supervisor assignee filters and dispatch entry points ---'
rg -n -C 12 'assignee_agent|dispatch_item|list_by_label|READY_LABEL|run_discovery_tick' src/supervisor.rs src/supervisor --glob '*.rs' 2>/dev/null | head -n 800
printf '%s\n' '--- claim matching functions ---'
rg -n -C 20 'fn (agent_of|claim|current_owner)|agent_of\(|owner.*assignee|assignee.*owner' src crates --glob '*.rs' | head -n 800
printf '%s\n' '--- canonicalization data-flow verifier ---'
python3 - <<'PY'
from pathlib import Path
registry = Path("crates/agent-registry/src/registry.rs").read_text()
handoff = Path("src/mcp_server/handoff.rs").read_text()
backend = Path("crates/agentflare-backend/src/item.rs").read_text()
assert '"claude" | "claude code" | "claude-code-cli"' in registry
assert 'agent_registry::agent_by_name(&recipient).is_none()' in handoff
assert 'let canonical_recipient = agent_registry::canonicalize(&recipient);' in handoff
assert 'assignee_agent: Some(recipient.clone())' in handoff
assert 'i.assignee_agent = ?2' in backend
print("alias accepted: claude -> claude-code")
print("stored recipient expression: original recipient")
print("reuse lookup expression: canonical_recipient")
print("backend comparison: exact equality")
print("result: an existing item stored as claude-code is not reusable after handoff recipient claude")
PYRepository: getappz/agentflare
Length of output: 50374
Persist the canonical agent identifier.
agent_by_name(&recipient) accepts aliases such as "claude" but the returned agent is discarded. The code stores the alias in assignee_agent, metadata, and the response, while reuse searches for the canonical "claude-code" value. This creates duplicate items. Autonomous dispatch also rejects the alias. Use the resolved agent identifier for all persisted and returned recipient fields.
🤖 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 89 - 101, Update the recipient
validation in the handoff flow to retain the agent returned by
agent_registry::agent_by_name(&recipient) and use its canonical id for
assignee_agent, metadata, response, and dispatch-related fields. Preserve the
existing invalid-recipient error and special handling for "github", while
ensuring aliases are never persisted or returned.
| let err = mcp.handoff_impl(req).unwrap_err(); | ||
| let msg = err.to_string(); | ||
| assert!(msg.contains("gastown"), "{msg}"); | ||
| assert!(msg.contains("claude-code"), "{msg}"); | ||
| assert!(msg.contains("github"), "{msg}"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant file locations ---'
git ls-files | rg '(^|/)(handoff\.rs|Cargo\.toml|Cargo\.lock)$|mcp_server|agent_registry'
printf '%s\n' '--- handoff.rs outline and size ---'
wc -l src/mcp_server/handoff.rs
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline src/mcp_server/handoff.rs | sed -n '1,220p'
else
echo 'ast-grep unavailable'
fi
printf '%s\n' '--- handoff validation and test context ---'
sed -n '60,120p' src/mcp_server/handoff.rs
sed -n '620,720p' src/mcp_server/handoff.rs
printf '%s\n' '--- relevant symbols and rmcp APIs ---'
rg -n -C 3 'handoff_impl|agent_by_name|invalid.?param|ErrorCode|test database|sqlite|persist|items' src Cargo.toml Cargo.lock 2>/dev/null | sed -n '1,320p'Repository: getappz/agentflare
Length of output: 26300
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- dependency versions ---'
rg -n -C 4 '^rmcp|rmcp =' Cargo.toml Cargo.lock | sed -n '1,100p'
printf '%s\n' '--- handoff implementation ---'
sed -n '1,220p' src/mcp_server/handoff.rs
sed -n '220,430p' src/mcp_server/handoff.rs
printf '%s\n' '--- test helpers and relevant tests ---'
sed -n '540,710p' src/mcp_server/handoff.rs
printf '%s\n' '--- backend item APIs and test DB access ---'
rg -n -C 5 'pub (fn|struct)|fn (get|list|create)|backend_db_override|test_mcp|with_backend_db' crates src/mcp_server.rs src/mcp_server/tests | rg 'item|test_mcp|backend_db|with_backend_db|src/mcp_server/handoff.rs' -n | sed -n '1,260p'
printf '%s\n' '--- current diff summary and relevant diff ---'
git diff --stat
git diff -- src/mcp_server/handoff.rs | sed -n '1,260p'Repository: getappz/agentflare
Length of output: 50374
🌐 Web query:
rmcp 1.8.0 ErrorData ErrorCode invalid_params code Rust API
💡 Result:
In the rmcp crate (the official Rust SDK for the Model Context Protocol), ErrorData is a structure used to represent JSON-RPC error responses [1][2]. The invalid_params code corresponds to the standard JSON-RPC error code -32602, which indicates an invalid method parameter [2][3]. You can trigger this error using the constructor method ErrorData::invalid_params(message, data) [1][2]. When implementing an MCP server, it is important to distinguish between protocol errors and tool-level errors [4][5]: 1. Protocol Errors (Use ErrorData): These are returned as Err(ErrorData) and should be used when the server cannot route or process the request at all (e.g., malformed input, missing capabilities, or internal infrastructure errors) [4][5]. Clients typically render these errors opaquely, meaning the end-user will likely not see the specific message you provide [4][5]. 2. Tool-Level Errors (Use CallToolResult::error): These should be returned as Ok(CallToolResult::error(...)) when the request is valid and successfully routed to your tool, but the tool fails to produce a result for reasons relevant to the user (e.g., a database query returns no rows, or an external API returns a 500 status) [4][5]. This approach ensures your custom message is passed to and rendered by the client for the user [4][5]. Example usage for an invalid parameter check: if args.query.is_empty { return Err(ErrorData::invalid_params("query must be non-empty", None)); } [4][5] Additionally, note that for protocol versions 2026-07-28 and newer, some errors that were previously classified as RESOURCE_NOT_FOUND (-32002) are now promoted to INVALID_PARAMS (-32602) to align with updated specifications [1][6].
Citations:
- 1: https://docs.rs/rmcp/latest/rmcp/model/struct.ErrorData.html
- 2: https://github.com/modelcontextprotocol/rust-sdk/blob/e9029ccc/crates/rmcp/src/model.rs
- 3: https://docs.rs/rmcp/latest/rmcp/model/struct.ErrorCode.html
- 4: https://docs.rs/rmcp/latest/rmcp/model/struct.CallToolResult.html
- 5: https://raw.githubusercontent.com/modelcontextprotocol/rust-sdk/main/README.md
- 6: https://github.com/modelcontextprotocol/rust-sdk/blob/80a74795/crates/rmcp/tests/test_resource_not_found_version.rs
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import urllib.request
urls = [
"https://docs.rs/crate/rmcp/1.8.0/source/src/model.rs",
"https://raw.githubusercontent.com/modelcontextprotocol/rust-sdk/rmcp-v1.8.0/crates/rmcp/src/model.rs",
]
for url in urls:
print(f"--- {url} ---")
try:
text = urllib.request.urlopen(url, timeout=10).read().decode()
except Exception as exc:
print(type(exc).__name__, exc)
continue
lines = text.splitlines()
for i, line in enumerate(lines):
if "struct ErrorData" in line or "enum ErrorCode" in line or "invalid_params" in line:
start = max(0, i - 8)
end = min(len(lines), i + 35)
print("\n".join(f"{n+1}:{lines[n]}" for n in range(start, end)))
print()
PYRepository: getappz/agentflare
Length of output: 614
🏁 Script executed:
#!/bin/bash
set -e
for url in \
'https://docs.rs/crate/rmcp/1.8.0/source/src/model.rs' \
'https://raw.githubusercontent.com/modelcontextprotocol/rust-sdk/rmcp-v1.8.0/crates/rmcp/src/model.rs'
do
echo "--- $url ---"
curl -k -L --fail --silent "$url" |
rg -n -C 12 'struct ErrorData|enum ErrorCode|fn invalid_params|INVALID_PARAMS|InvalidParams' |
sed -n '1,180p' || true
doneRepository: getappz/agentflare
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -e
curl -k -L --fail --silent \
'https://docs.rs/crate/rmcp/1.8.0/source/src/model.rs' |
python3 -c '
import sys, re, html
s = html.unescape(sys.stdin.read())
s = re.sub(r"<[^>]+>", "", s)
s = s.replace("\r", "")
lines = s.splitlines()
for i, line in enumerate(lines):
if "pub struct ErrorData" in line:
print("\n".join(f"{n+1}:{lines[n]}" for n in range(max(0,i-5), min(len(lines),i+18))))
break
'Repository: getappz/agentflare
Length of output: 1280
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
text = Path("src/mcp_server/handoff.rs").read_text()
start = text.index("pub fn handoff_impl(")
end = text.index("\n /// Publishes", start)
body = text[start:end]
invalid = body.index("if agent_registry::agent_by_name")
branch_end = body.index("\n let ext =", invalid)
prefix = body[:branch_end]
print("invalid_recipient_validation_before_db_access:",
"ErrorData::invalid_params" in prefix and "with_backend_db" not in prefix)
print("first_with_backend_db_offset:", body.index("with_backend_db"))
print("invalid_recipient_offset:", invalid)
test_start = text.index("fn recipient_not_in_the_agent_registry_is_rejected")
test_end = text.index("\n #[test]", test_start)
test = text[test_start:test_end]
print("test_asserts_error_code:", ".code" in test or "ErrorCode" in test)
print("test_asserts_message_fragments:",
all(f'msg.contains("{x}")' in test for x in ("gastown", "claude-code", "github")))
PYRepository: getappz/agentflare
Length of output: 342
Assert the protocol error code.
The message checks do not prove that handoff_impl returns ErrorCode::INVALID_PARAMS. Assert err.code == ErrorCode::INVALID_PARAMS; validation already occurs before any database write.
🤖 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 691 - 695, Update the test around
handoff_impl to assert that the returned err has code ErrorCode::INVALID_PARAMS,
while preserving the existing message-content assertions.
getappz
changed the title
Agentflare-Agent: claude-code Agentflare-Branch: task/52 Agentflare-Item: 52
Agentflare-Agent: claude-code Agentflare-Branch: task/52 Agentflare-Item: 52
Agentflare-Agent: claude-code Agentflare-Branch: task/52 Agentflare-Item: 52
1 participant
Auto-opened on
item donefor OHqzZViG_1FS_6WHOy9xK.Summary by CodeRabbit