Repository navigation
Conversation
Adds the missing fourth piece from item #419: given a claimed task (labels/kind/size) and the set of installed agents, decide which one should run it. RouteDecision always carries a reason. Precedence: explicit assignment > first matching rule with an installed preference > default (only if installed) > None (caller falls back, e.g. cede). Named TaskRouter/TaskContext rather than reusing optimize::runtime's Router/RouteContext, which is a different, prompt-text-based nudge for per-call model routing -- kept as is to avoid an unrelated rename. Scoped to the decision function only. Loading rules from ~/.agentflare/config.toml is left to #331 (unified config, still in progress) so this doesn't duplicate that surface ahead of time.
Wires agent_registry::route into 'agentflare work' as its first real consumer. --agent still wins when given (fails fast before claiming, same as before). Omit it and the item is claimed first, then routed from its own attributes via a TaskContext (labels, metadata.size, and critically assignee_agent) against the installed-agent set from detect_all_with. RouterConfig::default() has no rules yet, so today this only resolves when the item itself already names an assignee -- work.rs previously ignored that field entirely and always required a manual --agent. Rule- based routing from ~/.agentflare/config.toml activates automatically once #331 lands, with no further change here. Widens mcp_server::item::parsed_size to pub(crate) to reuse the existing (and already metadata-double-encoding-safe) size parser instead of duplicating it.
Adds agent_registry::parse_router_config: reads a config.toml's [router] table (default agent + [[router.rule]] entries with a labels/kind/size 'when' and a use preference list, single string or array) into a RouterConfig. Every other top-level table in the file is ignored -- this only ever reads its own corner, so it composes cleanly once #331 (unified config) actually owns the file. An agent name that doesn't resolve (typo, or unknown to this build's registry) is dropped from wherever it appeared rather than failing the whole parse -- route() already treats a missing preference as 'try the next one', so a bad name behaves like one that's merely not installed. Only malformed TOML is a hard Err. Wires it into 'agentflare work': load_router_config() reads the file (missing file -> the empty config, parse error -> a warning + the empty config, never a crash) and resolve_agent() now takes it instead of a hardcoded RouterConfig::default(). Auto-routing (--agent omitted) can now match a labels/kind/size rule, not just an item's own assignee_agent. Adds the toml crate (0.8, already resolved transitively in the lockfile) as agent-registry's first non-clap/serde dependency -- TOML parsing is exactly the kind of already-solved-by-a-library problem worth reaching for one over a hand-rolled subset parser for.
Adds mcp_server::item::parsed_kind, same metadata-blob convention as
the existing parsed_size ({\kind\: \locate\}) but free-form rather
than a fixed S/M/L set, since task shape isn't a small enumerable
list. Factors the shared double-encoding-safe JSON unwrap the two
now have in common out into parsed_metadata so the workaround lives
in exactly one place.
resolve_agent's TaskContext.kind was hardcoded to None (no item field
mapped to it yet) -- now reads parsed_kind, so a
'[[router.rule]] when.kind = \...\' rule can actually match once
something sets that metadata key. Nothing writes it yet (no kind
UI/tool surface exists), same bootstrapping state size was in before
groom's UI grew a way to set it.
7 new tests (5 for parsed_kind/parsed_size directly, 2 for a
kind-matching rule through resolve_agent); moved the new test module
to the end of item.rs after clippy flagged items-after-test-module
from the pre-existing top-of-file placement.
|
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 (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds TOML-based task routing to ChangesAgent routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkCommand
participant ItemService
participant AgentRouter
participant HeadlessLauncher
WorkCommand->>ItemService: fetch claimed item labels and metadata
ItemService-->>WorkCommand: return assignment and item context
WorkCommand->>AgentRouter: route task context and installed agents
AgentRouter-->>WorkCommand: return resolved agent or no decision
WorkCommand->>HeadlessLauncher: start work with resolved agent
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
🧹 Nitpick comments (6)
crates/agent-registry/src/router.rs (2)
77-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAgent-name resolution is duplicated and inconsistent across three entry points.
agent_by_strincrates/agent-registry/src/router.rsandagent_by_nameinsrc/cli/work.rsare the same exact-id registry lookup, and neither appliesagent_registry::canonicalize. Only the item-assignee path canonicalizes. As a result, the alias"claude"resolves fromassignee_agentbut fails from--agent claudeand fromdefault = "claude"in~/.agentflare/config.toml. Export one canonicalizing resolver fromagent-registryand use it at every site.
crates/agent-registry/src/router.rs#L77-L82: applycrate::registry::canonicalizeinsideagent_by_str, and make the resolverpubso callers do not copy it.src/cli/work.rs#L189-L192: replace the localagent_by_namewith the exported resolver so the explicit--agentvalue accepts the same aliases asassignee_agent, and apply it to the pre-claim check at lines 272-274 as well.🤖 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/agent-registry/src/router.rs` around lines 77 - 82, The agent-name resolver must canonicalize aliases consistently across all entry points. In crates/agent-registry/src/router.rs lines 77-82, make agent_by_str public and apply crate::registry::canonicalize before the registry lookup; in src/cli/work.rs lines 189-192, remove the local agent_by_name and use the exported resolver for explicit --agent values and the pre-claim check at lines 272-274.
179-191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify that the assignment branch bypasses the
installedcheck.The doc states
installedis "the sole authority on availability". The assignment branch returns before anyinstalledcheck, soroutecan return an agent that is not installed. The run then fails later inrun_headlesswithNotFound. Either state this exception in the doc, or gate the assignment oninstalledso all three paths behave the same.🤖 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/agent-registry/src/router.rs` around lines 179 - 191, The route function’s explicit task assignment currently bypasses the installed-agent availability rule. Update the documentation above route to explicitly state that assigned agents are accepted without checking installed, or instead update the assignment branch in route to verify the assigned agent exists in installed and fall through when it does not; keep the behavior consistent with the chosen contract.src/cli/work.rs (3)
344-346: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRemove the
expectby resolving the explicit agent once.Line 346 relies on the pre-claim check at lines 272-274 using identical resolution logic. If either site changes, for example to add
canonicalize, this line panics after the item is already claimed, and the claim is not released. Resolve the explicit name once before the claim and reuse theAgentvalue.🔧 Proposed fix
- if let Some(explicit) = args.agent.as_deref() - && agent_by_name(explicit).is_none() - { - crate::ui::error(&format!( - "unknown agent: {explicit} — use `agentflare agents list`" - )); - return 1; - } + let explicit_agent = match args.agent.as_deref() { + Some(explicit) => match agent_by_name(explicit) { + Some(agent) => Some(agent), + None => { + crate::ui::error(&format!( + "unknown agent: {explicit} — use `agentflare agents list`" + )); + return 1; + } + }, + None => None, + };Then at the resolution site:
- let agent_enum = if let Some(explicit) = args.agent.as_deref() { - agent_by_name(explicit).expect("checked above before claiming") - } else { + let agent_enum = if let Some(agent) = explicit_agent { + agent + } else {🤖 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 344 - 346, Update the agent-resolution flow in the work command to resolve the explicit agent name once before claiming the item, store the resulting Agent value, and reuse it for the post-claim selection in the agent_enum block. Remove the agent_by_name(...).expect(...) call there while preserving route-based resolution when no explicit agent is provided.
158-162: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWarn on read errors that are not "file missing".
The
let Ok(text) = ... elsearm treats every read failure as "no config". A permission error or non-UTF-8 file then produces silent default routing, and the user sees only "pass --agent explicitly". Match onErrorKind::NotFoundfor the silent path and warn for the rest, the same way the parse error path warns.🔧 Proposed fix
- let Ok(text) = std::fs::read_to_string(&path) else { - return agent_registry::RouterConfig::default(); + let text = match std::fs::read_to_string(&path) { + Ok(text) => text, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return agent_registry::RouterConfig::default(); + } + Err(e) => { + crate::ui::warning(&format!( + "{}: could not read config ({e}) — ignoring, pass --agent explicitly", + path.display() + )); + return agent_registry::RouterConfig::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/cli/work.rs` around lines 158 - 162, Update load_router_config to distinguish std::fs::read_to_string errors by ErrorKind: return the default RouterConfig silently only for NotFound, and emit a warning for all other read failures before returning the default, consistent with the existing parse-error handling.
348-374: 🚀 Performance & Scalability | 🔵 TrivialConsider the cold-start cost of
detect_all_withwhile the claim is held, and print the route reason.Two observations, neither blocking:
detect_all_withprobes everyTier::Cliregistry entry on PATH and runs each binary with its version args. The item is already claimed at this point. On a coldversion_cache, one slow agent binary delays the claimed run up to the detection timeout. The cache limits this to first runs, so measure before changing anything.resolve_agentdiscardsRouteDecision::reason. Line 374 prints only the agent name. Returning and printing the reason makes an unexpected pick self-explaining. Dashboard display stays out of scope.🤖 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 348 - 374, Measure the cold-start duration of agent_registry::detect_all_with while the claim is held before changing its placement or behavior. Update the resolve_agent flow to retain the RouteDecision reason alongside the selected agent, then print that reason with the agent in the existing output near headless_args. Keep dashboard display and other routing behavior unchanged.src/mcp_server/item.rs (1)
1039-1043: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a double-encoded
sizetest.
parsed_sizenow reads through the sharedparsed_metadatahelper. Onlyparsed_kindhas double-encoding coverage. Add thesizecase so a future change toparsed_metadatacannot silently break the original workaround.💚 Proposed test
#[test] fn parsed_size_still_rejects_values_outside_s_m_l() { assert_eq!(parsed_size(r#"{"size":"XL"}"#), None); assert_eq!(parsed_size(r#"{"size":"M"}"#), Some("M".to_string())); } + + #[test] + fn parsed_size_survives_double_encoded_metadata() { + let metadata = double_encoded(serde_json::json!({"size": "S"})); + assert_eq!(parsed_size(&metadata), Some("S".to_string())); + }🤖 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/item.rs` around lines 1039 - 1043, Extend the parsed_size test coverage around parsed_size to include a double-encoded size payload, matching the existing parsed_kind workaround coverage. Assert that the helper still extracts the expected size value from the double-encoded input while preserving the current rejection and normal-value assertions.
🤖 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/agent-registry/src/router.rs`:
- Around line 4-8: Correct the module comment in router.rs to describe the
existing free route function and RouterConfig/RouterRule types instead of
referring to a nonexistent TaskRouter type. Preserve the distinction from
optimize::runtime::Router while using only symbols actually defined in the
module.
- Around line 52-59: Update the kind and size checks in the rule-matching
expression to compare configured and task values with case-insensitive equality,
while preserving the existing behavior for unset filters and the labels_ok
logic.
In `@src/cli/work.rs`:
- Around line 14-19: Update the doc comment for the agent field in the work
command so it no longer claims user-configured routing rules are unavailable or
dependent on issue `#331`. Describe automatic routing as using the claimed item’s
assignee_agent and configured router rules, while preserving the existing
--agent behavior.
---
Nitpick comments:
In `@crates/agent-registry/src/router.rs`:
- Around line 77-82: The agent-name resolver must canonicalize aliases
consistently across all entry points. In crates/agent-registry/src/router.rs
lines 77-82, make agent_by_str public and apply crate::registry::canonicalize
before the registry lookup; in src/cli/work.rs lines 189-192, remove the local
agent_by_name and use the exported resolver for explicit --agent values and the
pre-claim check at lines 272-274.
- Around line 179-191: The route function’s explicit task assignment currently
bypasses the installed-agent availability rule. Update the documentation above
route to explicitly state that assigned agents are accepted without checking
installed, or instead update the assignment branch in route to verify the
assigned agent exists in installed and fall through when it does not; keep the
behavior consistent with the chosen contract.
In `@src/cli/work.rs`:
- Around line 344-346: Update the agent-resolution flow in the work command to
resolve the explicit agent name once before claiming the item, store the
resulting Agent value, and reuse it for the post-claim selection in the
agent_enum block. Remove the agent_by_name(...).expect(...) call there while
preserving route-based resolution when no explicit agent is provided.
- Around line 158-162: Update load_router_config to distinguish
std::fs::read_to_string errors by ErrorKind: return the default RouterConfig
silently only for NotFound, and emit a warning for all other read failures
before returning the default, consistent with the existing parse-error handling.
- Around line 348-374: Measure the cold-start duration of
agent_registry::detect_all_with while the claim is held before changing its
placement or behavior. Update the resolve_agent flow to retain the RouteDecision
reason alongside the selected agent, then print that reason with the agent in
the existing output near headless_args. Keep dashboard display and other routing
behavior unchanged.
In `@src/mcp_server/item.rs`:
- Around line 1039-1043: Extend the parsed_size test coverage around parsed_size
to include a double-encoded size payload, matching the existing parsed_kind
workaround coverage. Assert that the helper still extracts the expected size
value from the double-encoded input while preserving the current rejection and
normal-value assertions.
🪄 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: ae78dfea-63f1-4598-9b44-d5b4f766aeb4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
crates/agent-registry/Cargo.tomlcrates/agent-registry/src/lib.rscrates/agent-registry/src/router.rssrc/cli/work.rssrc/mcp_server/item.rs
- Dedupe agent-name resolution: export agent_registry::agent_by_name (canonicalize + registry lookup) as the one place every agent-name string resolves through, and use it in router.rs's TOML parsing and work.rs's --agent/assignee_agent paths. Previously only the assignee_agent path canonicalized, so '--agent claude' and 'default = \claude\' in config.toml silently failed where assignee_agent=\claude\ worked. - Remove the work.rs .expect() after claiming: resolve_agent is now called once, post-claim, for both the explicit and auto-routed paths, instead of a separate pre-claim lookup whose result was assumed (rather than proven) to still hold after the claim. - load_router_config now only treats ErrorKind::NotFound as 'no config yet'; any other read error (permissions, etc.) warns instead of silently degrading to the empty config. - RuleMatch's kind/size comparison is now case-insensitive, matching the labels comparison it sits next to instead of diverging from it. - resolve_agent now returns (Agent, reason) and agentflare work prints the reason alongside the agent name, so an auto-routed pick is self-explaining instead of just a bare id. - Doc fixes: router.rs's module comment no longer references a TaskRouter type that was never actually created; route()'s doc now states explicitly (with the rationale) that explicit assignment is the one path that bypasses the installed check; WorkArgs's --agent doc no longer claims config.toml rules are gated on #331 landing (the loader shipped in this same PR). - Adds a double-encoded-metadata test for parsed_size (parsed_kind already had one) and a test proving explicit assignment survives route() even when the assigned agent isn't in installed. Addresses all 3 actionable comments and the RouteDecision.reason nitpick from CodeRabbit's review; the detect_all_with cold-start-cost observation is left as-is per its own 'measure before changing' caveat, and dashboard display of the reason stays out of scope.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
getappz
pushed a commit
that referenced
this pull request
Aug 11, 2026
…t blocker prior sessions on this item reported. **What I implemented** (case 3 from the scope-broadening note — a dispatched agent correctly finding it's blocked on an unmet dependency, making zero changes, but with no way to say so distinct from "genuinely nothing to do"): - `build_prompt` (`src/cli/work.rs`) now teaches the agent an `AGENTFLARE_HOLD: <reason>` reply marker for exactly that case. - `execute_work`'s success path checks the reply for that marker (`detect_hold_signal`) *before* ever calling `item_done`. If present, it calls `item_release` instead — claim released, item state untouched, no `mark_completed` — posts a distinct "on hold" comment, and exits 0. This sidesteps the deeper root cause I found while investigating: `branch_diverged` diffs the *whole* branch against target, so a branch already carrying commits from an earlier claim attempt reads as "diverged" even when the current run changed nothing, silently routing a legitimate hold into the real-completion path (that's what actually happened to item #91). - Added 4 new tests; also confirmed case 2 (failed commit) is already correctly fixed by this branch's prior commit — `nothing_was_ever_committed`'s release path never touches item state. - Verified: `cargo build` clean, `cargo test --bin agentflare cli::work::` — 39 passed (4 new). `cargo clippy -D warnings` fails workspace-wide on a pre-existing, unrelated lint in `crates/agent-registry/src/router.rs` (last touched in PR #380, untouched here) — confirmed pre-existing on unmodified HEAD too. **Why it's uncommitted:** this session's own `.worktrees/task/92/.git` is mounted read-only (`mount` confirms `ro`), and the `mcp__flare__item done` daemon fallback also failed — `attempt to write a readonly database`. Same dual-blocked state the prior dispatch of this exact item hit. The one changed file sits uncommitted: - `src/cli/work.rs` (+83/-1) A human or unsandboxed process needs to `git add`/`git commit`/`git push` it, or re-run `mcp__flare__item done` for item 92 once the read-only mounts lift. Agentflare-Branch: task/92 Agentflare-Item: 92
getappz
pushed a commit
that referenced
this pull request
Aug 12, 2026
…t blocker prior sessions on this item reported. **What I implemented** (case 3 from the scope-broadening note — a dispatched agent correctly finding it's blocked on an unmet dependency, making zero changes, but with no way to say so distinct from "genuinely nothing to do"): - `build_prompt` (`src/cli/work.rs`) now teaches the agent an `AGENTFLARE_HOLD: <reason>` reply marker for exactly that case. - `execute_work`'s success path checks the reply for that marker (`detect_hold_signal`) *before* ever calling `item_done`. If present, it calls `item_release` instead — claim released, item state untouched, no `mark_completed` — posts a distinct "on hold" comment, and exits 0. This sidesteps the deeper root cause I found while investigating: `branch_diverged` diffs the *whole* branch against target, so a branch already carrying commits from an earlier claim attempt reads as "diverged" even when the current run changed nothing, silently routing a legitimate hold into the real-completion path (that's what actually happened to item #91). - Added 4 new tests; also confirmed case 2 (failed commit) is already correctly fixed by this branch's prior commit — `nothing_was_ever_committed`'s release path never touches item state. - Verified: `cargo build` clean, `cargo test --bin agentflare cli::work::` — 39 passed (4 new). `cargo clippy -D warnings` fails workspace-wide on a pre-existing, unrelated lint in `crates/agent-registry/src/router.rs` (last touched in PR #380, untouched here) — confirmed pre-existing on unmodified HEAD too. **Why it's uncommitted:** this session's own `.worktrees/task/92/.git` is mounted read-only (`mount` confirms `ro`), and the `mcp__flare__item done` daemon fallback also failed — `attempt to write a readonly database`. Same dual-blocked state the prior dispatch of this exact item hit. The one changed file sits uncommitted: - `src/cli/work.rs` (+83/-1) A human or unsandboxed process needs to `git add`/`git commit`/`git push` it, or re-run `mcp__flare__item done` for item 92 once the read-only mounts lift. Agentflare-Branch: task/92 Agentflare-Item: 92
getappz
added a commit
that referenced
this pull request
Aug 12, 2026
…_done (#455) * fix(item): distinguish a failed commit from nothing-to-commit in item_done commit_uncommitted previously swallowed a failed `git commit` (e.g. a read-only .git, a rejected pre-commit hook) with no way for item_done to tell that apart from a genuinely clean tree. A dispatched job could exit 0 with real work correctly staged but never committed, and the item would silently release as if nothing needed doing. commit_uncommitted now reports a commit failure distinctly from "nothing staged"; item_done treats a failed commit as a hard failure (comment + do not release-as-clean) instead of folding it into the nothing-was-ever-committed path. The new regression test moved into its own file (item_commit_failure_tests.rs) rather than growing action_tests.rs past its 1500-line LOC gate. Agentflare-Agent: claude-code Agentflare-Branch: task/92 Agentflare-Item: 92 * Implementation is done and verified, but I've hit the same environment blocker prior sessions on this item reported. **What I implemented** (case 3 from the scope-broadening note — a dispatched agent correctly finding it's blocked on an unmet dependency, making zero changes, but with no way to say so distinct from "genuinely nothing to do"): - `build_prompt` (`src/cli/work.rs`) now teaches the agent an `AGENTFLARE_HOLD: <reason>` reply marker for exactly that case. - `execute_work`'s success path checks the reply for that marker (`detect_hold_signal`) *before* ever calling `item_done`. If present, it calls `item_release` instead — claim released, item state untouched, no `mark_completed` — posts a distinct "on hold" comment, and exits 0. This sidesteps the deeper root cause I found while investigating: `branch_diverged` diffs the *whole* branch against target, so a branch already carrying commits from an earlier claim attempt reads as "diverged" even when the current run changed nothing, silently routing a legitimate hold into the real-completion path (that's what actually happened to item #91). - Added 4 new tests; also confirmed case 2 (failed commit) is already correctly fixed by this branch's prior commit — `nothing_was_ever_committed`'s release path never touches item state. - Verified: `cargo build` clean, `cargo test --bin agentflare cli::work::` — 39 passed (4 new). `cargo clippy -D warnings` fails workspace-wide on a pre-existing, unrelated lint in `crates/agent-registry/src/router.rs` (last touched in PR #380, untouched here) — confirmed pre-existing on unmodified HEAD too. **Why it's uncommitted:** this session's own `.worktrees/task/92/.git` is mounted read-only (`mount` confirms `ro`), and the `mcp__flare__item done` daemon fallback also failed — `attempt to write a readonly database`. Same dual-blocked state the prior dispatch of this exact item hit. The one changed file sits uncommitted: - `src/cli/work.rs` (+83/-1) A human or unsandboxed process needs to `git add`/`git commit`/`git push` it, or re-run `mcp__flare__item done` for item 92 once the read-only mounts lift. Agentflare-Branch: task/92 Agentflare-Item: 92 * fix: rustfmt + avoid world-writable permissions in test cleanup Restore the git dir's exact original permissions after the readonly-simulation test instead of a blanket set_readonly(false), which clippy correctly flags as making the path world-writable on Unix. Agentflare-Agent: claude-code Agentflare-Branch: task/92 Agentflare-Item: 92 * fix(flare-git-core): avoid world-writable permissions in test cleanup Same clippy::permissions_set_readonly_false issue as the mcp_server test fixed in the prior commit -- restore exact original permissions instead of a blanket set_readonly(false) toggle. worktree.rs's test module moved to a sibling worktree_tests.rs (pure code motion, no behavior change) since the fix pushed it past the 1500-line LOC gate -- item #92's own diff had already brought the file to the limit before this fix's few extra lines tipped it over. Agentflare-Agent: claude-code Agentflare-Branch: task/92 Agentflare-Item: 92 * test: skip readonly-git-dir commit-failure tests on Windows FILE_ATTRIBUTE_READONLY on a directory doesn't block writes to files inside it on Windows (unlike Unix permission bits), so the readonly- git-dir simulation these two tests rely on to force a commit failure doesn't hold there -- the commit silently succeeds instead, failing the test's own assertion. Confirmed as the actual cause of PR 455's Windows CI failure (auto-commit succeeded when the test expected an error). Unix-only, matching this repo's existing convention for platform-specific filesystem-permission tests. Agentflare-Agent: claude-code Agentflare-Branch: task/92 Agentflare-Item: 92 --------- Co-authored-by: shiva <shiva@gosysinfo.tech>
4 tasks done
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
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
Closes the missing fourth piece described in item #419: given a claimed task, decide which installed agent should run it.
crates/agent-registry/src/router.rs:TaskContext,RouteDecision,RouterRule/RuleMatch,RouterConfig, androute()— precedence is explicit assignment > first matching rule with an installed preference > installed default >None(caller falls back, e.g. cede). NamedTaskContext/RouteDecisionrather than reusingoptimize::runtime::Router/RouteContext(a different, prompt-text-based per-call model nudge) to avoid an unrelated, riskier rename.agentflare work:--agentis now optional. Given explicitly, behavior is unchanged. Omitted, the item is claimed first and then routed from its own attributes (labels,metadata.size,metadata.kind, and criticallyassignee_agent— previously ignored entirely).agent_registry::parse_router_config: reads the[router]table (default +[[router.rule]]entries) out of~/.agentflare/config.toml, ignoring every other top-level table so it composes cleanly once feat: migrate artifact storage onto agentflare-store documents+blobs + vent: origin routing, throttle, batched filing, judge-prompt hook #331 (unified config) lands. Unknown agent names are dropped rather than failing the whole parse; malformed TOML falls back to the empty config with a warning, never a crash.mcp_server::item::parsed_kind: samemetadataJSON-blob convention as the existingparsed_size, free-form rather than a fixed set. Factored the shared double-encoding-safe unwrap intoparsed_metadata.Deliberately out of scope
TaskContext.repo— no field maps to it; items don't carry an originating-repo concept in single-project-per-repo agentflare today.RouteDecision.reason— no consumer needed it yet.#331config surface — this only reads its own[router]corner.Test plan
cargo test -p agentflare-agent-registry(39 tests) — router + config parsingcargo test --bin agentflare cli::work::(18 tests) — agent resolution / auto-routingcargo test --bin agentflare metadata_field_tests::(5 tests) —parsed_kind/parsed_sizecargo test --bin agentflare item_groom— no regression from the metadata-parsing refactorcargo check --workspacecargo clippy --all-targets -- -A unsafe_code -A clippy::pedantic -D warningsSummary by CodeRabbit
workcommand can automatically select an agent when none is specified.