feat: add supervisor auto-dispatch loop (item #408) - #376
Merged
Conversation
item(list) had no cap when limit was omitted, unlike search/groom, so a bare call on a large project could overflow the MCP response token limit (155 items / 52k chars observed). Default it to 50 (capped at 500) and return a pagination envelope (total/offset/limit/next_offset/prev_offset) so callers can page forward/backward without re-deriving offsets by hand.
…inding - cargo fmt wrap on the next_offset line (CI fmt check was failing) - prev_offset now clamps via offset.min(total) so an out-of-range offset navigates back into range instead of repeating an empty page, and both next_offset/prev_offset suppress when limit == 0
enforced_rule_reason_for_tool matched on tool_name alone, so the "usetsearch" coaching rule denied every ToolSearch call regardless of what it was searching for -- including WebFetch/EnterPlanMode/ mcp__claude-in-chrome__* and agentflare's own mcp__flare__* tools, contradicting the rule's own stated fallback. It now inspects the query and only blocks genuine leanctx/ctx_* lookups, the one namespace actually missing from the deferred-tool list. Also add a builtin-tool catalog so mcp__flare__tool(action="search") finds agentflare's own first-party tools (item, asset, handoff, ...) before falling back to the public MCP registry -- previously a query like "item" returned unrelated third-party servers because the gateway registry only indexes configured downstream servers.
- Content filter now only applies to the Builtin-tier "usetsearch" rule; a user override (same id, tier Override) enforces on tool name alone again, per the coaching system's "override always wins" contract. - Normalize spaces/hyphens to underscores before the leanctx/ctx_* keyword match, so "ctx read" or "lean ctx" still trigger it, not just the literal underscored form. - Replace the unguaranteed -1.0 score sentinel on builtin tool hits with a documented 0.0 constant that's provably worse than any real bm25 score and better than the registry fallback sentinel. - Strengthen merge_builtin_hits test coverage to assert hit source per index, not just result length.
- search_builtin_tools now accepts and honors MatchMode: All requires every query term to match, Any requires at least one. Previously it always did Any-style matching regardless of the caller's requested mode, so mode="all" (the tool's own default) could still surface a builtin hit like mcp__flare__item on a query such as "item nonexistent" that shouldn't have matched anything. - Updated the "tool" action's description (both the builtin catalog entry and the matching #[tool] attribute) to state that action="search" now returns first-party agentflare tools alongside downstream MCP server tools, not just downstream ones.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds label-scoped item retrieval and a supervisor that periodically scans ready-for-work items, dispatches them to confirmed autonomous agents, or marks them for manual dispatch. The dashboard starts the discovery loop, with in-memory tests covering the new behavior. ChangesSupervisor discovery and dispatch
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DashboardServer
participant Supervisor
participant AgentflareMcp
participant JobQueue
DashboardServer->>Supervisor: start discovery loop
Supervisor->>AgentflareMcp: query ready-for-work items
Supervisor->>AgentflareMcp: validate assignee and update labels/comments
Supervisor->>JobQueue: enqueue agentflare work job
Supervisor-->>DashboardServer: return dispatched/skipped counts
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: 4
🤖 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.rs`:
- Around line 583-592: Update MspServer::for_test_memory to set
backend_project_link_override to a unique temporary project-link path, and use a
fixed test repository key if the constructor supports one. Ensure supervisor
ticks resolve against these test-only values without writing the checkout’s real
.agentflare/project.json or sharing paths across parallel tests.
In `@src/supervisor.rs`:
- Around line 41-57: The discovery flow in run_discovery_tick must preserve and
propagate database, project, and label lookup failures instead of converting
them into an empty result via .ok()? and the zero-count fallback. Replace the
erased errors with an error-returning path, update callers as needed, and have
the scheduler log failures while retaining the existing empty result only for a
valid absence of ready work.
- Around line 100-106: Update both label-transition paths in the supervisor,
including the NEEDS_MANUAL_LABEL flow and the corresponding dispatched flow, to
verify the destination label exists before removing ready-for-work. If the
destination label is missing, retain ready-for-work and report a configuration
error; otherwise add the destination label and remove the source label.
Alternatively, provision the required destination labels during setup.
- Around line 127-150: Update the dispatch flow surrounding queue.enqueue so the
item is reserved or otherwise made ineligible before enqueue is treated as
final, and only proceed when that reservation succeeds. Ensure failures removing
the ready-for-work label cannot leave the item eligible for a duplicate enqueue,
while preserving the existing dispatched-label and comment updates after
successful dispatch.
🪄 Autofix (Beta)
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 Plus
Run ID: 18eba06f-bb9b-48b5-a12f-fcc55945dcb0
📒 Files selected for processing (6)
crates/agentflare-backend/src/item.rssrc/dashboard/server.rssrc/main.rssrc/mcp_server.rssrc/mcp_server/item.rssrc/supervisor.rs
Comment on lines
+583
to
+592
| /// Create an in-memory backend for tests that don't need a real repo/disk. | ||
| /// The other fields (skills, gateway, store, etc.) stay defaulted so they | ||
| /// lazily open `:memory:` SQLite connections or no-ops — no I/O to ~/. | ||
| #[cfg(test)] | ||
| pub(crate) fn for_test_memory() -> Self { | ||
| Self { | ||
| backend_db_override: Some(std::path::PathBuf::from(":memory:")), | ||
| ..Default::default() | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep memory-backed tests from writing the real repository link.
for_test_memory() leaves backend_project_link_override unset, so supervisor ticks call resolve_project() and write the checkout’s .agentflare/project.json. Parallel tests can race and tests leave repository state behind. Set a unique temporary project-link path (and preferably a fixed test repo key) here.
Proposed fix
pub(crate) fn for_test_memory() -> Self {
+ let dir = tempfile::tempdir().expect("create test directory").keep();
Self {
backend_db_override: Some(std::path::PathBuf::from(":memory:")),
+ backend_project_link_override: Some(dir.join("project.json")),
+ backend_repo_key_override: Some("test-memory".to_string()),
..Default::default()
}
}📝 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
| /// Create an in-memory backend for tests that don't need a real repo/disk. | |
| /// The other fields (skills, gateway, store, etc.) stay defaulted so they | |
| /// lazily open `:memory:` SQLite connections or no-ops — no I/O to ~/. | |
| #[cfg(test)] | |
| pub(crate) fn for_test_memory() -> Self { | |
| Self { | |
| backend_db_override: Some(std::path::PathBuf::from(":memory:")), | |
| ..Default::default() | |
| } | |
| } | |
| /// Create an in-memory backend for tests that don't need a real repo/disk. | |
| /// The other fields (skills, gateway, store, etc.) stay defaulted so they | |
| /// lazily open `:memory:` SQLite connections or no-ops — no I/O to ~/. | |
| #[cfg(test)] | |
| pub(crate) fn for_test_memory() -> Self { | |
| let dir = tempfile::tempdir().expect("create test directory").keep(); | |
| Self { | |
| backend_db_override: Some(std::path::PathBuf::from(":memory:")), | |
| backend_project_link_override: Some(dir.join("project.json")), | |
| backend_repo_key_override: Some("test-memory".to_string()), | |
| ..Default::default() | |
| } | |
| } |
🤖 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.rs` around lines 583 - 592, Update MspServer::for_test_memory
to set backend_project_link_override to a unique temporary project-link path,
and use a fixed test repository key if the constructor supports one. Ensure
supervisor ticks resolve against these test-only values without writing the
checkout’s real .agentflare/project.json or sharing paths across parallel tests.
Comment on lines
+41
to
+57
| let fetched = mcp.with_backend_db(|conn| { | ||
| let project = mcp.resolve_project(conn).ok()?; | ||
| let labels = agentflare_backend::label::list_by_project(conn, &project.id).ok()?; | ||
| let mut label_id_by_name = std::collections::HashMap::new(); | ||
| for l in &labels { | ||
| label_id_by_name.insert(l.name.clone(), l.id.clone()); | ||
| } | ||
| let ready_id = label_id_by_name.get(READY_LABEL)?.clone(); | ||
| let items = agentflare_backend::item::list_by_label(conn, &project.id, &ready_id).ok()?; | ||
| Some((items, label_id_by_name)) | ||
| }); | ||
|
|
||
| let Ok(Some((items, label_id_by_name))) = fetched else { | ||
| return result; | ||
| }; | ||
| let Some(ready_id) = label_id_by_name.get(READY_LABEL).cloned() else { | ||
| return result; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not convert discovery failures into an empty tick.
Every fallible database/project/label operation is erased by .ok()?, and the outer match returns a zero-count result. The dashboard therefore cannot distinguish a broken backend from “no ready work,” silently disabling auto-dispatch. Return an error from run_discovery_tick and log it from the scheduler.
🤖 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/supervisor.rs` around lines 41 - 57, The discovery flow in
run_discovery_tick must preserve and propagate database, project, and label
lookup failures instead of converting them into an empty result via .ok()? and
the zero-count fallback. Replace the erased errors with an error-returning path,
update callers as needed, and have the scheduler log failures while retaining
the existing empty result only for a valid absence of ready work.
Comment on lines
+100
to
+106
| if let Some(needs_manual_id) = label_id_by_name.get(NEEDS_MANUAL_LABEL) { | ||
| let _ = mcp.item_add_label(ItemRequest { | ||
| action: "add_label".into(), | ||
| id: Some(item.id.clone()), | ||
| label_id: Some(needs_manual_id.clone()), | ||
| ..Default::default() | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require destination labels before removing ready-for-work.
Both transitions remove the source label unconditionally but add needs-manual-dispatch/dispatched only when those labels already exist. A project with only ready-for-work configured loses the item from discovery without the promised status marker. Provision these labels or retain ready-for-work and report a configuration error.
Also applies to: 137-144
🤖 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/supervisor.rs` around lines 100 - 106, Update both label-transition paths
in the supervisor, including the NEEDS_MANUAL_LABEL flow and the corresponding
dispatched flow, to verify the destination label exists before removing
ready-for-work. If the destination label is missing, retain ready-for-work and
report a configuration error; otherwise add the destination label and remove the
source label. Alternatively, provision the required destination labels during
setup.
Comment on lines
+127
to
+150
| let Ok(info) = queue.enqueue(&job) else { | ||
| return false; | ||
| }; | ||
|
|
||
| let _ = mcp.item_remove_label(ItemRequest { | ||
| action: "remove_label".into(), | ||
| id: Some(item.id.clone()), | ||
| label_id: Some(ready_id.to_string()), | ||
| ..Default::default() | ||
| }); | ||
| if let Some(dispatched_id) = label_id_by_name.get(DISPATCHED_LABEL) { | ||
| let _ = mcp.item_add_label(ItemRequest { | ||
| action: "add_label".into(), | ||
| id: Some(item.id.clone()), | ||
| label_id: Some(dispatched_id.clone()), | ||
| ..Default::default() | ||
| }); | ||
| } | ||
| let _ = mcp.comment_impl(CommentRequest { | ||
| action: "create".into(), | ||
| item_id: Some(item.id.clone()), | ||
| body: Some(format!("## supervisor — dispatched\n\njob: {}", info.id)), | ||
| ..Default::default() | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n## src/supervisor.rs\n'
sed -n '1,240p' src/supervisor.rs
printf '\n## queue-related symbols\n'
rg -n "enqueue\(|struct .*Queue|trait .*Queue|idempot|unique queue|dispatch|ready-for-work|DISPATCHED_LABEL" src -SRepository: getappz/agentflare
Length of output: 19407
🏁 Script executed:
set -euo pipefail
printf '\n## queue implementation\n'
fd -a "queue.rs" . src
fd -a "mod.rs" src/agentflare_jobs src
printf '\n## enqueue and job identity\n'
rg -n "fn enqueue|enqueue\(&job\)|idempot|dedup|duplicate|job.id|struct AgentJob|impl AgentJob|new\(command" src -S
printf '\n## supervisor tests around repeated dispatch\n'
sed -n '240,340p' src/supervisor.rsRepository: getappz/agentflare
Length of output: 16868
🏁 Script executed:
set -euo pipefail
printf '\n## crates/agentflare-jobs/src/queue.rs\n'
sed -n '1,260p' crates/agentflare-jobs/src/queue.rs
printf '\n## AgentJob definition\n'
rg -n "struct AgentJob|impl AgentJob|fn new\(" crates/agentflare-jobs/src -SRepository: getappz/agentflare
Length of output: 10358
Make dispatch idempotent before treating enqueue as final. queue.enqueue() always inserts a new job with a fresh id, and there’s no reservation/idempotency key. If removing ready-for-work fails, the item remains eligible on the next tick and can be enqueued again.
🤖 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/supervisor.rs` around lines 127 - 150, Update the dispatch flow
surrounding queue.enqueue so the item is reserved or otherwise made ineligible
before enqueue is treated as final, and only proceed when that reservation
succeeds. Ensure failures removing the ready-for-work label cannot leave the
item eligible for a duplicate enqueue, while preserving the existing
dispatched-label and comment updates after successful dispatch.
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.
Implements supervisor auto-dispatch: periodically scans
ready-for-workitems and dispatches to confirmed agents (claude-code).Changes
crates/agentflare-backend/src/item.rs— addlist_by_labelquerysrc/supervisor.rs— new module:resolve_confirmed_agent,run_discovery_tick,skip_item,dispatch_itemwith 5 testssrc/main.rs— registersupervisormodulesrc/mcp_server/item.rs— bumpitem_add_label/item_remove_labeltopub(crate)src/mcp_server.rs— addfor_test_memory()constructor &backend_db_override→pub(crate)src/dashboard/server.rs— wirespawn_supervisor_discovery(12s interval) after job cleanupCloses #408
Summary by CodeRabbit