Conversation
…ation loading
- Created new workspace member with AgentConfig schema (name, mode, description, model, hidden, temperature, top_p, options, permission, prompt)
- Implemented frontmatter parsing with YAML preprocessing to handle `key: value:with:colons` edge cases
- Added directory loader for discovering agent configs from `{agent,agents}/**/*.md` patterns using gitignore-aware scanning
- Added comprehensive error types (Io, MissingFrontmatter, InvalidYaml, SchemaValidation) with thiserror
- Included 32 tests covering frontmatter parsing edge cases and loader discovery patterns
- Added Rule struct with private fields and getters (permission, pattern, action) - Added Ruleset with from_config, evaluate, and allowed_tools methods - Implemented wildcard matching (pub(crate)) with last-match-wins semantics - Added SubagentRegistry with list(mode), get(name), filter_accessible methods - Permission key matching uses exact equality (case-insensitive) - filter_accessible excludes Primary mode agents - Default deny behavior when no rule matches - Added comprehensive tests for permission evaluation and registry filtering - Updated module exports and documentation
…ssion-based access control - Added TaskRunner trait for framework-agnostic subagent execution - Added TaskToolCore enforcing access validation before delegation - Added TaskInput/TaskOutput/TaskError types for task execution - Added TASK constant to tool_names in core library - Added rig TaskTool adapter implementing rig::tool::Tool - Added serdesAI TaskTool adapter implementing serdes_ai::tools::Tool - Tool filtering passes allowed_tools to runner with case-insensitive matching - Error order: UnknownAgent → NotInvocable → AccessDenied - Updated subagents README with Task Tool documentation and usage examples - Added test verifying allowed_tools preserves original tool name casing
Document the problem (YAML misparsing colons), show before/after examples for both transformed and preserved cases, and explain why each case is handled differently.
WIP: Improve Frontmatter Parsing Performance
…and Clippy fixes - Changed signature from `&str` to `String` for in-place mutation - Added full-file CRLF→LF normalization upfront - Single-pass YAML block scalar conversion (no pre-scan) - Replaced FrontmatterSlices with FrontmatterOffsets for byte index tracking - Added extract_body_inplace() using split_off + in-place trim to avoid reallocation - Removed memchr dependency - Fixed Clippy warnings: sort_by → sort_by_key with Reverse (glob.rs:90, grep.rs:171) - Updated benchmarks to use iter_batched
- Explain function purpose: checks for unquoted colons in YAML values - Document return values: Some((key, value)) for conversion, None if safe - List all cases where None is returned (no conversion needed)
…order is not required - Changed `options` field in AgentConfig from IndexMap to HashMap (order not semantically meaningful) - Changed `load_agents` return type and internal map from IndexMap to HashMap - Changed `agents` field in SubagentRegistry from IndexMap to HashMap - IndexMap preserved for `permission` fields where last-match-wins semantics require ordering - All 76 tests pass
- Clarify that star wildcard matches zero or more characters - Explain that backtracking lets star consume one additional character - Document retry behavior for remaining pattern matching
- Added AgentLoader builder for loading from files, directories, and in-memory configs - Added AgentSource private enum to track sources (Directory, File, Config) - Implemented load() and load_into_registry() methods for SubagentRegistry - Added helper methods: add_directory(), add_file(), add_file_named(), add_config() - Refactored load_agents() to use new load_agents_registry() and SubagentRegistry::into_map() - Added reserve() and into_map() to SubagentRegistry - Re-exported load_agents_registry and AgentLoader in lib.rs - Added tests for file name derivation, override semantics, and loading into existing registry
- Updated crate-level docs to highlight AgentLoader's flexible source composition - Updated example to use AgentLoader and SubagentRegistry API - Added note that load_agents remains a convenience for directory-only scans - Confirmed exports include AgentLoader, load_agents, load_agents_registry, and SubagentRegistry - Added test for loading explicit file without agent prefix - Added test for directory scanning with agent patterns - Added test for overriding existing registry entries
Make AgentLoader the sole public entry point so agent configs load consistently across files, strings, and bytes with clear, path-aware error context. Align docs, benchmarks, and verification to the new loader/parser split.
…ectories - Save original working directory before changing to project root - Use trap to restore original directory even if script fails - Ensure cargo commands execute from correct workspace location
- Made AgentLoader stateless with internal source storage removed - Changed all loader methods to accept &mut SubagentRegistry for immediate insertion - Removed load() and load_into_registry() methods - Removed SubagentRegistry::reserve() method (no longer needed) - Updated all tests, docs, and benchmarks to use registry-first API
- Split task.rs into task/mod.rs (implementation) and task/tests.rs (tests) in both rig and serdesAI adapters - Reserved task/runner.rs for future TaskRunner implementation - Preserved public API: TaskTool and TaskArgs exports unchanged - All 288 tests pass, clippy clean
- Updated template-version.txt from 1.0.1 to 1.1.1 - Enhanced Code & Performance Guidelines section with memory optimization details: - Use arrays instead of maps when size is known ahead of time - Optimize for memory (preallocate, trim, minimize memory use) - Use smaller integers/types where appropriate - Apply other CPU/memory efficiency tricks
- Added AgentCatalog type in catalog.rs for config-only storage (no placeholder error agents) - Added catalog loading methods in loader.rs (add_directory_to_catalog, add_file_to_catalog, add_config_to_catalog, add_from_str_to_catalog, add_from_bytes_to_catalog) - Exported AgentCatalog from lib.rs - Refactored loader.rs to use shared helpers between registry and catalog (load_directory_with, parse_agent_config, load_agent_file) - Catalog uses strict validation (no placeholder error agents), registry preserves existing behavior - All 87 tests pass in agents crate (449 total across workspace)
- Add missing article 'the' in README documentation - Add exit code check in PowerShell verification script - Add #[source] attribute for error chaining in Io variant - Fix directory restoration on success in PowerShell script using try/finally - Add --check flag to cargo fmt for verification instead of modifying files - Fix throughput calculation for CRLF variant in benchmarks - Fix tilde expansion issue in README example (use literal path) - Return Option from derive_agent_name_from_rel to handle empty names - Propagate parse errors in add_from_str instead of silently inserting error config - Validate key-name consistency in from_map by rebuilding the map - Change inline helper rule from FAIL to WARNING with better guidance All verification checks pass: build, test, clippy, fmt, docs
…tion - Added ToolCatalogEntry cloneable enum to rig framework with name(), context(), and builder registration methods - Added ToolCatalogEntry cloneable enum to serdesAI framework with name(), context(), and builder registration methods - Implemented default_tools() function parameterized by line_numbers, resolver, and todo_state - Builder registration helpers support both fresh (register_on/register_on_simple) and existing builders - Default catalog excludes Task tool to avoid registry circularity - All tools in a single mode (absolute or allowed, never both) - Tests validate 9 expected tool names, uniqueness, and Task exclusion - All verification checks pass (build, tests, clippy, docs, formatting)
- Move async-trait to [dev-dependencies] in rig Cargo.toml - Add --check flag to cargo fmt in verify.ps1 for verification-only mode - Fix UTF-8 error handling in loader.rs to return proper validation error
- Created registry.rs with AgentDefaults, AgentRegistry, AgentRegistryBuilder, and RegistryAgent trait for framework-agnostic agent storage - Rewrote task/mod.rs to use registry directly for agent lookup, removing TaskRunner and TaskToolCore dependencies - Added register_on_with_prompt() and register_on_simple_with_prompt() to tool_catalog.rs for system prompt context tracking - Updated task/tests.rs with 7 tests using MockAgent for access validation, description building, and context message formatting - Moved async-trait from dev-dependencies to dependencies in Cargo.toml for RegistryAgent trait implementation - Exported AgentDefaults, AgentRegistry, AgentRegistryBuilder, and AgentRegistryEntry from lib.rs Fix CodeRabbit findings: - Changed default PermissionAction from Allow to Deny for safer security posture - Updated add_from_bytes docs to reflect actual error behavior (schema validation error) - Fixed model resolution to handle empty config.model by filtering before fallback - Clarified AGENTS.md redirect with proper migration note - Added working_directory() call to README Quick Start example - Implemented std::fmt::Display and std::error::Error for RegistryAgentError - Aligned empty name validation between add_from_str and add_from_str_to_catalog
…tool integration - Created registry.rs with AgentDefaults, AgentRegistry, AgentRegistryBuilder, RegistryAgent trait, RegistryAgentError, and AgentRegistryEntry for framework-agnostic agent storage - Refactored Task tool to use registry directly, removing TaskRunner and TaskToolCore dependencies - Added from_entries method to AgentCatalog for config-only to registry conversion - Tool filtering by agent permissions during registry construction - Added register_with_prompt to ToolCatalogEntry for system prompt context tracking during agent building - Implemented comprehensive tests for hidden agents, permission filtering, task message formatting, and error handling - Added indexmap dependency for permission config support
- Add TaskTool usage example to README Usage section - Fix benchmark loop variables to properly benchmark both LF and CRLF variants - Add missing objectives_path documentation to Inputs section - Fix typo in catalog.rs doc comment: [AgentConfig) -> [AgentConfig] - Update rig-core version from 0.29 to 0.28.0 (0.29 does not exist on crates.io) - Replace expect() panic with proper BuildFailed error handling in registry.rs - Return error for non-directory paths that exist in loader.rs
…flow - Removed TaskRunner, TaskToolCore, TaskError from agents crate (now framework-specific) - Deleted SubagentRegistry module (registry.rs) entirely - Updated AgentLoader to work exclusively with AgentCatalog - Simplified task.rs to only export TaskInput and TaskOutput types - Removed async-trait dependency and tokio dev-dependency from agents - Rewrote agents/README with registry-driven flow documentation and migration guide - Updated rig README with new Task tool setup and examples - Updated serdesAI README with new Task tool setup and examples - Added note to core README about registry-driven Task tools - Updated benchmark to use AgentCatalog instead of SubagentRegistry - Net code reduction: 856 lines - All verification checks pass (306 tests, builds, clippy, docs) Fix CodeRabbit review findings: - Removed internal development references (created in PROMPT-06) from README files - Standardized severity levels for code style issues in quality gate template - Defined FORBIDDEN_TESTS_FOUND handling in decision logic and test status - Updated TaskOutput::format() to include metadata field - Capitalized SerdesAI consistently in documentation
…orks - Added llm-coding-tools-rig/examples/registry-driven-task-rig.rs - Added llm-coding-tools-serdesai/examples/registry-driven-task-serdesai.rs - Both examples demonstrate complete registry-driven Task flow - Examples support absolute and allowed flows via OPENCODE_USE_ALLOWED Fix CodeRabbit review findings: - Fixed inconsistent casing in lib.rs: "serdesAI" → "SerdesAI" - Fixed PowerShell script to restore RUSTDOCFLAGS after execution - Fixed registry-driven-task-rig.rs to read API key from environment with fallback - Updated README.md model format to show representative example with provider prefix and optional tag - Added Mode Options section in README explaining subagent/primary/primary-only modes
- Renamed operations module to tools for clarity - Updated all imports in core, serdesai, and agents crates - Bumped versions to 0.2.0 for core and serdesai - Resolved task module location conflict - All tests pass (330 total)
![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
🤖 Fix all issues with AI agents
In `@src/AGENTS.md`:
- Line 3: The sentence "This is a headless library, there is no TUI interaction
model here, so interactive `ask` approval flows and autocomplete-style agent UX
are out of scope." contains a comma splice; rewrite it as two sentences or use a
semicolon—for example split into "This is a headless library. There is no TUI
interaction model here, so interactive `ask` approval flows and
autocomplete-style agent UX are out of scope." or combine with a semicolon after
"library" to fix the grammar.
In `@src/llm-coding-tools-serdesai/src/task/mod.rs`:
- Around line 207-215: resolve_runtime_rules currently returns an empty Ruleset
when a RegistryCaller name is missing, which silently denies access; update
resolve_runtime_rules (and its callers) to surface this misconfiguration by
returning a Result<Cow<'a, Ruleset>, TaskError> (or similar) instead of always
returning Cow::Owned(Ruleset::new()), and when AgentRegistry::get(caller_name)
returns None return an Err that includes the missing caller_name and context;
alternatively (if changing the signature is unacceptable) log a clear warning
including caller_name before returning the owned empty Ruleset so failures are
diagnosable. Ensure references to resolve_runtime_rules,
TaskCallerAuthority::RegistryCaller, AgentRegistry::get, and Ruleset::new() are
updated accordingly.
🧹 Nitpick comments (3)
src/llm-coding-tools-serdesai/src/task/mod.rs (3)
17-39:TaskArgsduplicatesTaskInput— consider deserializing directly into [TaskInput].
TaskInputalready derivesDeserialize(seetask.rsline 13) with identical fields. TheTaskArgsstruct and itsFromimpl are a 1:1 copy that adds maintenance burden without benefit. If the wire format ever needs to diverge, this indirection can be reintroduced then.♻️ Suggested simplification in
call()- let args: TaskArgs = serde_json::from_value(args) - .map_err(|e| ToolError::validation_error(tool_names::TASK, None, e.to_string()))?; - - let input: TaskInput = args.into(); + let input: TaskInput = serde_json::from_value(args) + .map_err(|e| ToolError::validation_error(tool_names::TASK, None, e.to_string()))?;
225-253: O(n²) lookup indefinition()— sort targets directly instead of re-scanning.The code collects all names into a sorted
Vec, then for each name does a linear.find()back intoself.definition_snapshot.targets. Since the number of agents is typically small this won't cause real performance problems, but the intent is clearer if you sort the targets directly:♻️ Suggested simplification
fn definition(&self) -> ToolDefinition { - let mut names: Vec<_> = self + let mut targets: Vec<_> = self .definition_snapshot .targets .iter() - .map(|target| target.name.as_str()) + .filter(|t| t.is_invocable()) + .filter(|t| { + self.definition_rules().evaluate(tool_names::TASK, &t.name) + == PermissionAction::Allow + }) .collect(); - names.sort_unstable(); + targets.sort_unstable_by(|a, b| a.name.cmp(&b.name)); - let mut lines = Vec::with_capacity(names.len()); - for name in names { - let target = match self - .definition_snapshot - .targets - .iter() - .find(|target| target.name == name) - { - Some(target) => target, - None => continue, - }; - if !target.is_invocable() { - continue; - } - if self.definition_rules().evaluate(tool_names::TASK, name) != PermissionAction::Allow { - continue; - } - lines.push(format!("- {}: {}", name, target.tool_names.join(", "))); - } + let mut lines = Vec::with_capacity(targets.len()); + for target in targets { + lines.push(format!("- {}: {}", target.name, target.tool_names.join(", "))); + }
191-205: Add#[inline]to small helper methods.
definition_rules()andresolve_registry()are small, hot-path-adjacent helpers. Per coding guidelines, small functions on hot paths should be annotated with#[inline].♻️ Suggested change
+ #[inline] fn definition_rules(&self) -> &Ruleset { match &self.authority { TaskCallerAuthority::StaticRules(ruleset) => ruleset, TaskCallerAuthority::RegistryCaller { build_rules, .. } => build_rules, } } + #[inline] fn resolve_registry(&self) -> Result<&AgentRegistry<A>, ToolError> {As per coding guidelines,
Use #[inline] on small, hot-path functions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
src/AGENTS.mdsrc/llm-coding-tools-core/Cargo.tomlsrc/llm-coding-tools-core/src/lib.rssrc/llm-coding-tools-core/src/tools/glob.rssrc/llm-coding-tools-core/src/tools/grep.rssrc/llm-coding-tools-core/src/tools/mod.rssrc/llm-coding-tools-core/src/tools/task.rssrc/llm-coding-tools-serdesai/Cargo.tomlsrc/llm-coding-tools-serdesai/src/task/mod.rs
✅ Files skipped from review due to trivial changes (1)
- src/llm-coding-tools-core/src/tools/glob.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/llm-coding-tools-core/Cargo.toml
- src/llm-coding-tools-core/src/lib.rs
- src/llm-coding-tools-serdesai/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.rs: Preallocate collections when size is known or estimable usingString::with_capacity(estimated_len),Vec::with_capacity(count), orBufReader::with_capacity(size, reader)
Prefer&str/&[T]returns over owned types when lifetime allows
UseCow<'_, str>for conditional ownership (e.g.,String::from_utf8_lossy)
Use&'static strfor compile-time constant strings
Reuse buffers by calling.clear()and reusingVec/Stringinstead of reallocating
Use const generics for compile-time branching (e.g.,<const LINE_NUMBERS: bool>)
Use#[inline]on small, hot-path functions
Prefercoreoverstdwhere possible (e.g.,core::memoverstd::mem)
Usememchrcrate for fast byte searching over manual iteration
Keep modules under 500 lines (excluding tests); split if larger
Placeusestatements inside functions only for#[cfg]conditional compilation
Document public items with///and add examples in docs where helpful
Use//!for module-level documentation
Use [TypeName] rustdoc links instead of backticks in documentation
Files:
src/llm-coding-tools-core/src/tools/mod.rssrc/llm-coding-tools-core/src/tools/grep.rssrc/llm-coding-tools-core/src/tools/task.rssrc/llm-coding-tools-serdesai/src/task/mod.rs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`
Applied to files:
src/llm-coding-tools-core/src/tools/mod.rssrc/llm-coding-tools-core/src/tools/task.rssrc/llm-coding-tools-serdesai/src/task/mod.rssrc/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations
Applied to files:
src/llm-coding-tools-core/src/tools/mod.rssrc/llm-coding-tools-core/src/tools/task.rssrc/llm-coding-tools-serdesai/src/task/mod.rssrc/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Use `//!` for module-level documentation
Applied to files:
src/llm-coding-tools-core/src/tools/mod.rssrc/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Prefer `core` over `std` where possible (e.g., `core::mem` over `std::mem`)
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/Cargo.toml : Prefer performance-oriented crates: `parking_lot` over `std::sync`, `memchr` for byte search
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Document public items with `///` and add examples in docs where helpful
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Prefer `&str` / `&[T]` returns over owned types when lifetime allows
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Reuse buffers by calling `.clear()` and reusing `Vec`/`String` instead of reallocating
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Keep modules under 500 lines (excluding tests); split if larger
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Use `#[inline]` on small, hot-path functions
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: After making changes to source code, run `.cargo/verify.sh` (or `.cargo/verify.ps1` on Windows) before returning to the user
Applied to files:
src/AGENTS.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Use [`TypeName`] rustdoc links instead of backticks in documentation
Applied to files:
src/AGENTS.md
🧬 Code graph analysis (1)
src/llm-coding-tools-serdesai/src/task/mod.rs (5)
src/llm-coding-tools-serdesai/src/convert.rs (1)
to_serdes_result(51-58)src/llm-coding-tools-serdesai/src/tool_catalog.rs (2)
context(101-122)name(77-95)src/llm-coding-tools-serdesai/src/registry.rs (4)
is_invocable(149-151)new(88-92)new(237-243)get(181-183)src/llm-coding-tools-agents/src/permission.rs (2)
new(25-35)new(70-72)src/llm-coding-tools-agents/src/config.rs (1)
default(42-44)
🪛 LanguageTool
src/AGENTS.md
[grammar] ~1-~1: Use a hyphen to join words.
Context: Basic coding oriented tools for LLM agents. This is ...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (9)
src/llm-coding-tools-core/src/tools/grep.rs (1)
171-171: LGTM!Renaming the closure parameter from
btofileimproves readability and makes the sort intent clearer.src/AGENTS.md (3)
21-22: Looks good.
Project structure update is clear and consistent.
27-28: Looks good.
Performance guidance is concise and actionable.
70-72: Looks good.
Verification guidance is explicit and helpful.src/llm-coding-tools-core/src/tools/task.rs (1)
1-62: LGTM — clean DTO definitions with proper documentation and builder pattern.The module is well-structured: DTOs are appropriately owned for deserialization, builder methods are
#[inline]-annotated, and all public items have///doc comments with rustdoc links. Module-level//!docs are present.src/llm-coding-tools-core/src/tools/mod.rs (1)
13-13: LGTM — consistent module declaration and re-export.Follows the existing alphabetical ordering and re-export pattern of sibling modules.
Also applies to: 22-22
src/llm-coding-tools-serdesai/src/task/mod.rs (3)
341-359: Good use ofString::with_capacityfor the message buffer.Pre-allocating with
input.prompt.len() + 128is a reasonable estimate for the structured message envelope. Aligns with coding guidelines on preallocating collections.
296-296:expecton schema build is acceptable here since input is compile-time deterministic.The schema definition is static and any failure would be a programming error caught at first use, so
expectis the right choice over propagating an error.
300-373: Runtime permission checks are thorough — existence, invocability, and access rules all validated before dispatch.The
callmethod correctly validates in the right order: deserialization → registry lookup → mode check → permission check → dispatch. Error messages are specific and actionable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| fn resolve_runtime_rules<'a>(&'a self, registry: &'a AgentRegistry<A>) -> Cow<'a, Ruleset> { | ||
| match &self.authority { | ||
| TaskCallerAuthority::StaticRules(ruleset) => Cow::Borrowed(ruleset), | ||
| TaskCallerAuthority::RegistryCaller { caller_name, .. } => registry | ||
| .get(caller_name) | ||
| .map(|entry| Cow::Borrowed(&entry.ruleset)) | ||
| .unwrap_or_else(|| Cow::Owned(Ruleset::new())), | ||
| } | ||
| } |
There was a problem hiding this comment.
Falling back to an empty Ruleset when caller is not in the registry silently denies all access.
When the RegistryCaller variant's caller_name isn't found in the registry, resolve_runtime_rules returns Cow::Owned(Ruleset::new()). A fresh/empty Ruleset will presumably deny all agent invocations, meaning a misconfigured caller name silently blocks task delegation with a generic "Access denied" error. Consider logging a warning or returning a descriptive error instead, so misconfigurations are easier to diagnose.
🤖 Prompt for AI Agents
In `@src/llm-coding-tools-serdesai/src/task/mod.rs` around lines 207 - 215,
resolve_runtime_rules currently returns an empty Ruleset when a RegistryCaller
name is missing, which silently denies access; update resolve_runtime_rules (and
its callers) to surface this misconfiguration by returning a Result<Cow<'a,
Ruleset>, TaskError> (or similar) instead of always returning
Cow::Owned(Ruleset::new()), and when AgentRegistry::get(caller_name) returns
None return an Err that includes the missing caller_name and context;
alternatively (if changing the signature is unacceptable) log a clear warning
including caller_name before returning the owned empty Ruleset so failures are
diagnosable. Ensure references to resolve_runtime_rules,
TaskCallerAuthority::RegistryCaller, AgentRegistry::get, and Ruleset::new() are
updated accordingly.
…plementations Split fs.rs into modular implementation with separate blocking and tokio variants selected via compile-time feature flags. Changes: - Split fs.rs into fs/mod.rs, fs/blocking_impl.rs, fs/tokio_impl.rs - Renamed async_impl.rs to tokio_impl.rs for consistency - Added compile_error guards for mutually exclusive tokio/blocking features - Updated all tool implementations to use new fs module APIs - Added file.flush() in tests to ensure data persistence - Fixed potential integer truncation in webfetch using usize::try_from - Cleaned up Cargo.toml dependencies Benefits: - Clear separation of sync/async filesystem implementations - Compile-time feature selection prevents runtime ambiguity - Safer integer conversions prevent truncation bugs - More consistent naming across async implementations
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/llm-coding-tools-core/src/lib.rs (1)
35-36:⚠️ Potential issue | 🟠 MajorChange feature guard from
asynctotokioto prevent compilation failure.The re-export at line 35 uses
#[cfg(any(feature = "async", feature = "blocking"))], butfetch_urlis only defined inwebfetch/mod.rsunder#[cfg(feature = "tokio")]or#[cfg(feature = "blocking")]. Since theasyncfeature is just a marker with no dependencies, a user could enableasyncwithouttokio, causing the re-export to fail compilation. Change the guard to#[cfg(any(feature = "tokio", feature = "blocking"))]to align with the actual module-level gates.
🧹 Nitpick comments (6)
src/llm-coding-tools-core/src/output.rs (1)
13-13: Prefercore::ops::Not::notoverstd::ops::Not::not.Per coding guidelines, prefer
coreoverstdwhere possible.Suggested fix
- #[serde(skip_serializing_if = "std::ops::Not::not")] + #[serde(skip_serializing_if = "core::ops::Not::not")]src/llm-coding-tools-serdesai/Cargo.toml (2)
25-34: Consider feature-gating provider support to reduce compile footprint.Enabling all 7 provider features unconditionally increases compile time and binary size for consumers who may only need one or two providers. You could expose these as optional features of this crate so downstream users opt in selectively.
49-50: Explicitly declare theserdefeature forindexmapto improve clarity, even though it's transitively available viallm-coding-tools-agents:-indexmap = "2.7" +indexmap = { version = "2.7", features = ["serde"] }This makes the dependency explicit and ensures the crate remains self-sufficient if upstream dependencies change.
src/llm-coding-tools-serdesai/src/task/mod.rs (1)
225-253: O(n²) lookup indefinition()can be simplified.The method collects target names, sorts them, then for each name does a linear
.find()back intoself.definition_snapshot.targets. Since agent counts are typically small this is not a real performance concern, but it could be simplified by sorting the targets directly.♻️ Suggested simplification
fn definition(&self) -> ToolDefinition { - let mut names: Vec<_> = self + let mut targets: Vec<_> = self .definition_snapshot .targets .iter() - .map(|target| target.name.as_str()) .collect(); - names.sort_unstable(); + targets.sort_unstable_by(|a, b| a.name.cmp(&b.name)); - let mut lines = Vec::with_capacity(names.len()); - for name in names { - let target = match self - .definition_snapshot - .targets - .iter() - .find(|target| target.name == name) - { - Some(target) => target, - None => continue, - }; + let mut lines = Vec::with_capacity(targets.len()); + for target in &targets { if !target.is_invocable() { continue; } - if self.definition_rules().evaluate(tool_names::TASK, name) != PermissionAction::Allow { + if self.definition_rules().evaluate(tool_names::TASK, &target.name) != PermissionAction::Allow { continue; } - lines.push(format!("- {}: {}", name, target.tool_names.join(", "))); + lines.push(format!("- {}: {}", target.name, target.tool_names.join(", "))); }src/llm-coding-tools-core/src/fs/blocking_impl.rs (1)
7-9: Consider#[inline]on these thin wrapper functions.These are trivially thin wrappers around
std::fscalls. While I/O dominates runtime cost,#[inline]could let the compiler eliminate the wrapper overhead at call sites — consistent with the coding guideline to use#[inline]on small, hot-path functions. This applies to all four functions in this module.♻️ Example for read_to_string (apply similarly to others)
/// Reads a file to string. +#[inline] pub fn read_to_string(path: impl AsRef<Path>) -> ToolResult<String> { Ok(std::fs::read_to_string(path)?) }As per coding guidelines,
src/**/*.rs: "Use#[inline]on small, hot-path functions."src/llm-coding-tools-core/src/fs/mod.rs (1)
23-44: Test coverage gap:create_dir_allandopen_bufferedare untested.The tests cover
read_to_stringandwrite, butcreate_dir_allandopen_buffered(both exposed from the blocking and tokio impls) have no corresponding tests. Consider adding tests for completeness —create_dir_allcan usetempdirandopen_bufferedcan verify readable content via the returnedBufReader.♻️ Suggested additional tests
+ #[maybe_async::test(feature = "blocking", async(feature = "tokio", tokio::test))] + async fn create_dir_all_works() { + let dir = tempfile::tempdir().unwrap(); + let nested = dir.path().join("a").join("b").join("c"); + create_dir_all(&nested).await.unwrap(); + assert!(nested.is_dir()); + } + + #[maybe_async::test(feature = "blocking", async(feature = "tokio", tokio::test))] + async fn open_buffered_works() { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(b"buffered content").unwrap(); + file.flush().unwrap(); + let reader = open_buffered(file.path(), 8192).await.unwrap(); + use std::io::Read as _; + let mut buf = String::new(); + // For blocking: use std::io::Read directly + // For tokio: the BufReader implements tokio::io::AsyncRead + // This test may need cfg-specific reading logic + #[cfg(feature = "blocking")] + { + let mut reader = reader; + reader.read_to_string(&mut buf).unwrap(); + assert_eq!(buf, "buffered content"); + } + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
src/llm-coding-tools-core/Cargo.tomlsrc/llm-coding-tools-core/README.mdsrc/llm-coding-tools-core/src/fs.rssrc/llm-coding-tools-core/src/fs/blocking_impl.rssrc/llm-coding-tools-core/src/fs/mod.rssrc/llm-coding-tools-core/src/fs/tokio_impl.rssrc/llm-coding-tools-core/src/lib.rssrc/llm-coding-tools-core/src/output.rssrc/llm-coding-tools-core/src/tools/bash/mod.rssrc/llm-coding-tools-core/src/tools/bash/tokio_impl.rssrc/llm-coding-tools-core/src/tools/edit.rssrc/llm-coding-tools-core/src/tools/read.rssrc/llm-coding-tools-core/src/tools/webfetch/mod.rssrc/llm-coding-tools-core/src/tools/webfetch/tokio_impl.rssrc/llm-coding-tools-core/src/tools/write.rssrc/llm-coding-tools-serdesai/Cargo.tomlsrc/llm-coding-tools-serdesai/src/task/mod.rs
💤 Files with no reviewable changes (1)
- src/llm-coding-tools-core/src/fs.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/llm-coding-tools-core/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.rs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.rs: Preallocate collections when size is known or estimable usingString::with_capacity(estimated_len),Vec::with_capacity(count), orBufReader::with_capacity(size, reader)
Prefer&str/&[T]returns over owned types when lifetime allows
UseCow<'_, str>for conditional ownership (e.g.,String::from_utf8_lossy)
Use&'static strfor compile-time constant strings
Reuse buffers by calling.clear()and reusingVec/Stringinstead of reallocating
Use const generics for compile-time branching (e.g.,<const LINE_NUMBERS: bool>)
Use#[inline]on small, hot-path functions
Prefercoreoverstdwhere possible (e.g.,core::memoverstd::mem)
Usememchrcrate for fast byte searching over manual iteration
Keep modules under 500 lines (excluding tests); split if larger
Placeusestatements inside functions only for#[cfg]conditional compilation
Document public items with///and add examples in docs where helpful
Use//!for module-level documentation
Use [TypeName] rustdoc links instead of backticks in documentation
Files:
src/llm-coding-tools-core/src/tools/edit.rssrc/llm-coding-tools-core/src/tools/webfetch/tokio_impl.rssrc/llm-coding-tools-core/src/tools/bash/tokio_impl.rssrc/llm-coding-tools-core/src/tools/bash/mod.rssrc/llm-coding-tools-core/src/fs/blocking_impl.rssrc/llm-coding-tools-core/src/lib.rssrc/llm-coding-tools-core/src/tools/read.rssrc/llm-coding-tools-core/src/fs/mod.rssrc/llm-coding-tools-core/src/tools/write.rssrc/llm-coding-tools-core/src/fs/tokio_impl.rssrc/llm-coding-tools-core/src/output.rssrc/llm-coding-tools-core/src/tools/webfetch/mod.rssrc/llm-coding-tools-serdesai/src/task/mod.rs
src/**/Cargo.toml
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/Cargo.toml: Enabletokiofeature flag (default) for async mode with tokio runtime, orblockingfor sync/blocking mode. Theasyncandblockingfeatures are mutually exclusive.
Prefer performance-oriented crates:parking_lotoverstd::sync,memchrfor byte search
Files:
src/llm-coding-tools-serdesai/Cargo.toml
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/Cargo.toml : Enable `tokio` feature flag (default) for async mode with tokio runtime, or `blocking` for sync/blocking mode. The `async` and `blocking` features are mutually exclusive.
Applied to files:
src/llm-coding-tools-core/src/tools/edit.rssrc/llm-coding-tools-core/src/tools/bash/tokio_impl.rssrc/llm-coding-tools-core/src/tools/bash/mod.rssrc/llm-coding-tools-core/src/lib.rssrc/llm-coding-tools-core/src/tools/read.rssrc/llm-coding-tools-core/src/fs/mod.rssrc/llm-coding-tools-core/src/tools/write.rssrc/llm-coding-tools-core/src/fs/tokio_impl.rssrc/llm-coding-tools-core/src/tools/webfetch/mod.rssrc/llm-coding-tools-core/README.md
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Use `//!` for module-level documentation
Applied to files:
src/llm-coding-tools-core/src/tools/bash/tokio_impl.rssrc/llm-coding-tools-core/src/fs/mod.rs
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Document public items with `///` and add examples in docs where helpful
Applied to files:
src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`
Applied to files:
src/llm-coding-tools-core/src/fs/blocking_impl.rssrc/llm-coding-tools-core/src/lib.rssrc/llm-coding-tools-core/src/fs/mod.rssrc/llm-coding-tools-core/src/fs/tokio_impl.rssrc/llm-coding-tools-core/README.mdsrc/llm-coding-tools-serdesai/Cargo.toml
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Place `use` statements inside functions only for `#[cfg]` conditional compilation
Applied to files:
src/llm-coding-tools-core/src/tools/read.rssrc/llm-coding-tools-core/src/fs/mod.rs
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations
Applied to files:
src/llm-coding-tools-core/src/fs/mod.rssrc/llm-coding-tools-core/src/fs/tokio_impl.rssrc/llm-coding-tools-serdesai/src/task/mod.rssrc/llm-coding-tools-serdesai/Cargo.toml
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Prefer `core` over `std` where possible (e.g., `core::mem` over `std::mem`)
Applied to files:
src/llm-coding-tools-core/src/fs/mod.rs
🧬 Code graph analysis (5)
src/llm-coding-tools-core/src/tools/bash/mod.rs (2)
src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs (1)
execute_command(16-113)src/llm-coding-tools-core/src/tools/bash/blocking_impl.rs (1)
execute_command(17-125)
src/llm-coding-tools-core/src/fs/blocking_impl.rs (1)
src/llm-coding-tools-core/src/fs/tokio_impl.rs (4)
read_to_string(7-9)write(12-14)create_dir_all(17-19)open_buffered(22-28)
src/llm-coding-tools-core/src/fs/mod.rs (2)
src/llm-coding-tools-core/src/fs/blocking_impl.rs (2)
read_to_string(7-9)write(12-14)src/llm-coding-tools-core/src/fs/tokio_impl.rs (2)
read_to_string(7-9)write(12-14)
src/llm-coding-tools-core/src/fs/tokio_impl.rs (1)
src/llm-coding-tools-core/src/fs/blocking_impl.rs (4)
read_to_string(7-9)write(12-14)create_dir_all(17-19)open_buffered(22-28)
src/llm-coding-tools-core/src/tools/webfetch/mod.rs (1)
src/llm-coding-tools-core/src/tools/webfetch/tokio_impl.rs (1)
fetch_url(12-65)
🔇 Additional comments (21)
src/llm-coding-tools-core/src/tools/webfetch/tokio_impl.rs (1)
37-37: Good defensive conversion fromu64tousize.Using
usize::try_from(len).ok()gracefully handles the edge case on 32-bit platforms where au64content-length could exceedusize::MAX, falling back to streaming-only size checks instead of silently truncating.src/llm-coding-tools-core/src/tools/write.rs (1)
41-41: LGTM — consistent explicit feature gating.Switching from
not(feature = "blocking")tofeature = "tokio"is the right call for mutually exclusive features; it avoids silently selecting the async path when neither feature is enabled.Also applies to: 55-55
src/llm-coding-tools-core/src/tools/edit.rs (1)
92-92: LGTM — same consistent gating update as the other modules.Also applies to: 112-112
src/llm-coding-tools-core/src/tools/read.rs (2)
57-60: LGTM — cfg-gated imports correctly placed inside the function body, consistent with the new explicit tokio gating.Both the conditional
useplacement and thefeature = "tokio"gate align with the codebase conventions. As per coding guidelines,usestatements inside functions are appropriate for#[cfg]conditional compilation.
180-180: LGTM — test macros updated consistently.Also applies to: 188-188, 196-196
src/llm-coding-tools-core/src/output.rs (1)
3-4: LGTM — cfg gates are consistent across import, impl, and test.The feature gating is correctly and consistently applied to the
useimport, theFrom<WebFetchOutput>impl, and the corresponding test.Also applies to: 49-57, 96-107
src/llm-coding-tools-serdesai/Cargo.toml (2)
17-21: LGTM on new internal crate dependencies.Both
llm-coding-tools-agentsandllm-coding-tools-models-devare properly declared with path references, aligning with the PR's registry-driven task tool objectives.
56-57: Good move offuturesto dev-dependencies.If
futuresis only used in tests/examples, keeping it out of the main dependency tree is the right call.src/llm-coding-tools-core/README.md (2)
12-19: LGTM!The additions documenting
TaskInput/TaskOutputand the registry-driven Task tool flow are clear and consistent with the code changes moving these types into core.
21-26: LGTM!Simplified features section is consistent with the
tokio/blockingmutual exclusivity enforced inlib.rs.src/llm-coding-tools-core/src/tools/webfetch/mod.rs (1)
86-94: LGTM!Clean transition from
async_impltotokio_implwith explicittokiofeature gating, consistent with the same pattern inbash/mod.rs.src/llm-coding-tools-core/src/fs/tokio_impl.rs (1)
1-28: LGTM!Clean, symmetric tokio wrappers that mirror the
blocking_impl.rsAPI surface. Error propagation via?is correct.src/llm-coding-tools-serdesai/src/task/mod.rs (2)
300-373: LGTM! Solid validation chain incall().Proper defense-in-depth: argument deserialization → agent existence → invocability check → permission check → dispatch. Message construction is clean with reasonable capacity pre-allocation.
68-106: LGTM!TaskRegistryHandlewithOnceLockis well-suited for two-phase initialization.The lazy initialization pattern cleanly solves the chicken-and-egg problem of recursive agent wiring.
src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs (1)
1-1: LGTM!Doc comment update is accurate — clarifies the tokio-specific nature of this implementation.
src/llm-coding-tools-core/src/tools/bash/mod.rs (1)
59-62: LGTM!Consistent
tokiofeature gating matching the pattern inwebfetch/mod.rs.src/llm-coding-tools-core/src/lib.rs (2)
8-9: LGTM — good compile-time safety net.Requiring at least one runtime mode ensures consumers can't accidentally get a crate with no usable I/O surface.
31-31: LGTM!
TaskInputandTaskOutputre-exports from core are consistent with the PR objective of centralizing framework-agnostic operation types.src/llm-coding-tools-core/src/fs/blocking_impl.rs (1)
1-28: Clean, consistent blocking mirror of the async API.The implementations correctly mirror the tokio counterparts with matching signatures and error propagation. Module documentation and public item docs are present. No issues found.
src/llm-coding-tools-core/src/fs/mod.rs (2)
1-21: Well-structured feature-gated module with proper mutual-exclusivity guards.The
compile_error!guards correctly enforce that exactly one oftokioorblockingis enabled, and the conditional module inclusion + glob re-exports cleanly present a unified API surface. This is a solid pattern.
3-5: Documentation is accurate. The Cargo.toml correctly specifiesdefault = ["tokio"], andblockingis not included in defaults, matching the module documentation's description.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Relocate the permission evaluation system from llm-coding-tools-agents to llm-coding-tools-core where it more appropriately belongs as a framework-agnostic primitive. Changes: - Moved permission.rs from agents crate to core crate as permissions.rs - Moved PermissionAction and PermissionRule enums to appropriate crates - Core permissions module contains: PermissionAction, Rule, Ruleset, wildcard matching - Serialization format (PermissionRule) remains in agents crate - Updated all imports across serdesai crate and tests - Added llm-coding-tools-core dependency to agents crate Benefits: - Better separation of concerns: core evaluation logic vs serialization - Permission system is now framework-agnostic and reusable - Agents crate focuses on config loading and agent management - Clearer API boundaries between crates
Replaced verbose separate README and crate docs in llm-coding-tools-agents with a single concise source using include_str! to load README.md into crate documentation. The new docs focus on core purpose, compatibility, and quick start. Changes: - Unified README.md and lib.rs docs via include_str! macro - Reduced README from 151 lines to 65 lines (57% reduction) - Restructured to: What it provides, Compatibility notes, Quick start, Agent file format, Integration - Added OpenCode documentation links for schema fields (mode, model, permissions, task-permissions, hidden) - Updated model example to synthetic/hf:moonshotai/Kimi-K2.5 - Converted to British English (catalogue, summarises, behaviour) - Clarified compatibility: rejects permission.task: ask (requires interactive UX), ignores hidden - Removed redundant Task tool examples (point to serdesai crate instead) - Removed Migration section (legacy APIs already removed from codebase) Benefits: - Single source of truth for documentation (README drives both GitHub and docs.rs) - Faster for users to understand what the crate does - Clear compatibility expectations for OpenCode drop-in usage - Reduced maintenance burden by eliminating doc duplication
Eliminate the #![warn(missing_docs)] attribute from llm-coding-tools-agents, llm-coding-tools-core, and llm-coding-tools-serdesai. Changes: - Removed #![warn(missing_docs)] from llm-coding-tools-agents/src/lib.rs - Removed #![warn(missing_docs)] from llm-coding-tools-core/src/lib.rs - Removed #![warn(missing_docs)] from llm-coding-tools-serdesai/src/lib.rs Benefits: - Reduces compile-time warnings - Simplifies crate attributes - All crates already have complete documentation, so the lint is redundant
… crate
These types are now imported directly from llm_coding_tools_core::permissions
to provide clearer dependency boundaries and avoid duplicate exports.
Changes:
- Removed pub use llm_coding_tools_core::permissions::{PermissionAction, Rule, Ruleset}
- Updated doctest in extensions.rs to import Ruleset from core
- Updated test file to import from core directly
- Updated README examples to import from core directly
- Added reference-style link for Ruleset in README
Benefits:
- Clearer dependency graph (consumers import directly from core)
- Avoids confusion about which crate owns these types
- Consistent with single-responsibility principle
The Substitute trait and its String implementation were not being used anywhere in the codebase except for their own unit tests. Removing dead code reduces maintenance burden and clarifies the public API. Changes: - Removed Substitute trait definition and impl for String - Removed substitute_replaces_single_placeholder test - Removed substitute_leaves_unmatched_placeholders test - Removed substitute_handles_empty_value test - Removed substitute_all_replaces_multiple test - Removed substitute_no_placeholder_returns_unchanged test - Removed Substitute from public exports in core and serdesai crates Benefits: - Cleaner public API with only actively used types - Reduced code complexity and maintenance overhead - 5 fewer tests to maintain for unused functionality
…ssions Refresh `llm-coding-tools-core` documentation for both crates.io and module-level docs on docs.rs. Changes: - Reworked `llm-coding-tools-core/README.md` with a clearer flow (table of contents, install/features upfront, and focused sections) - Added concise examples for path resolver modes, ToolContext wrapper mapping, and serdesAI-style integration - Clarified feature-gated behavior (WebFetch), const-generic read usage, and last-match-wins permissions behavior - Expanded `permissions` module-level docs with mapping and evaluation examples - Removed stale async-only wording from `tools/todo.rs` module docs Benefits: - Improves first-time onboarding and scanability on crates.io - Keeps crate docs and module docs aligned with actual behavior - Reduces ambiguity for wrapper authors integrating resolvers and permissions
Replaced placeholder header with comprehensive module documentation that explains the agent configuration format and shows realistic examples. Changes: - Added module-level documentation explaining YAML frontmatter parsing - Documented permission rule semantics (simple actions and pattern-based) - Clarified that pattern matching only works for task tool delegation - Added complete example showing all supported fields - Used real model format (synthetic/hf:moonshotai/Kimi-K2.5) and default temperature Benefits: - Developers can understand the config format without reading source - Example shows correct usage avoiding misleading patterns - Accurate representation of what features are actually implemented
Split the 3-parameter add_directory into two methods for better ergonomics: - add_directory(catalog, directory) - simple version without callback - add_directory_with_errors(catalog, directory, on_error) - with error callback Changes: - Added add_directory_with_errors() with the full callback signature - Changed add_directory() to delegate to add_directory_with_errors with None callback - Updated all 15+ test call sites to use simpler add_directory() API - Updated README.md example to remove explicit None::<fn()> type annotation - Expanded module-level and struct-level documentation with comprehensive examples Benefits: - Eliminates need for explicit None::<fn(&Path, &AgentLoadError)> at call sites - Provides cleaner API for common case (ignore file errors silently) - Maintains flexibility for advanced use cases via add_directory_with_errors - Better developer experience with simpler default interface
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/llm-coding-tools-agents/src/loader.rs`:
- Around line 48-58: The doc example unnecessarily declares loader as mutable;
change `let mut loader = AgentLoader::new();` to `let loader =
AgentLoader::new();` because all AgentLoader methods (add_directory, add_file)
take &self; update the snippet in the module docs to match the module-level
example that uses `let loader` and ensure no other doc examples for AgentLoader
declare it as mut.
🧹 Nitpick comments (4)
src/llm-coding-tools-agents/src/config.rs (1)
65-88: Consider documenting whydescriptionis required (no#[serde(default)]) inRawFrontmatter.This is the schema enforcement point —
descriptionbeing required here whileAgentConfighas#[serde(default)]on it could confuse future contributors. A brief doc comment on thedescriptionfield (e.g.,/// Required in frontmatter schema) would clarify intent.src/llm-coding-tools-agents/src/loader.rs (3)
155-182: Minor:add_file_namedpasses an empty string toload_agent_filethen overwrites.Line 178 passes
String::new()as the default name toload_agent_file, which flows intofrom_rawwhere it may become the config name if the frontmatter also lacks anamefield. The immediate override on line 179 makes this safe, but the intent could be clearer. Not a bug—just a readability nit.
237-249: UTF-8 errors reported asSchemaValidationis slightly misleading.
std::str::from_utf8failure is an encoding issue, not a schema validation failure. Consider usingAgentLoadError::ioor introducing a dedicated variant if you want precise error categorization. This is minor since the error message itself ("invalid UTF-8: ...") is clear.
252-312:Err(_) => continueon walker entries silently drops walk-level I/O errors.Line 279 discards errors from the
ignorecrate walker (e.g., permission denied on subdirectories). While acceptable for a best-effort directory scan, consider forwarding these to theon_errorcallback inadd_directory_with_errorsfor observability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/llm-coding-tools-agents/README.mdsrc/llm-coding-tools-agents/src/config.rssrc/llm-coding-tools-agents/src/extensions.rssrc/llm-coding-tools-agents/src/loader.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/llm-coding-tools-agents/src/extensions.rs
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.rs: Preallocate collections when size is known or estimable usingString::with_capacity(estimated_len),Vec::with_capacity(count), orBufReader::with_capacity(size, reader)
Prefer&str/&[T]returns over owned types when lifetime allows
UseCow<'_, str>for conditional ownership (e.g.,String::from_utf8_lossy)
Use&'static strfor compile-time constant strings
Reuse buffers by calling.clear()and reusingVec/Stringinstead of reallocating
Use const generics for compile-time branching (e.g.,<const LINE_NUMBERS: bool>)
Use#[inline]on small, hot-path functions
Prefercoreoverstdwhere possible (e.g.,core::memoverstd::mem)
Usememchrcrate for fast byte searching over manual iteration
Keep modules under 500 lines (excluding tests); split if larger
Placeusestatements inside functions only for#[cfg]conditional compilation
Document public items with///and add examples in docs where helpful
Use//!for module-level documentation
Use [TypeName] rustdoc links instead of backticks in documentation
Files:
src/llm-coding-tools-agents/src/config.rssrc/llm-coding-tools-agents/src/loader.rs
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`
Applied to files:
src/llm-coding-tools-agents/README.mdsrc/llm-coding-tools-agents/src/config.rs
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations
Applied to files:
src/llm-coding-tools-agents/README.mdsrc/llm-coding-tools-agents/src/config.rs
📚 Learning: 2026-02-07T22:53:26.056Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.056Z
Learning: Applies to src/**/*.rs : Document public items with `///` and add examples in docs where helpful
Applied to files:
src/llm-coding-tools-agents/README.md
🧬 Code graph analysis (1)
src/llm-coding-tools-agents/src/loader.rs (4)
src/llm-coding-tools-agents/src/parser/mod.rs (4)
parse_agent(49-79)parse_agent(293-293)parse_agent(417-417)parse_agent(436-436)src/llm-coding-tools-agents/src/catalog.rs (3)
new(24-28)by_name(45-47)iter(34-36)src/llm-coding-tools-agents/src/error.rs (3)
schema_validation(82-87)io(72-74)parse(77-79)src/llm-coding-tools-agents/src/config.rs (1)
from_raw(128-141)
🔇 Additional comments (10)
src/llm-coding-tools-agents/README.md (1)
1-61: LGTM!The README is clear, well-structured, and provides good quick-start examples with appropriate compatibility notes.
One minor note: the rustdoc-style links (e.g.,
[`AgentLoader`],[`Ruleset`]) won't resolve when rendered on GitHub or crates.io—they only work insiderustdoc. Consider using regular markdown links to the docs.rs pages if discoverability outsidecargo docmatters to you.src/llm-coding-tools-agents/src/config.rs (3)
1-34: LGTM!Module documentation is thorough with a realistic YAML frontmatter example. The imports are clean and appropriate.
49-63: LGTM onPermissionRule.
#[serde(untagged)]correctly triesAction(scalar) beforePattern(map), which aligns with the YAML format where simpleallow/denystrings and pattern maps are both valid.
90-142: LGTM onAgentConfigandfrom_raw.The struct cleanly separates the public API shape from the intermediate
RawFrontmatter, andfrom_rawcorrectly prefers the frontmatternameover the derived default.src/llm-coding-tools-agents/src/loader.rs (6)
1-28: LGTM!Module-level documentation is clear with a useful example covering directory, file, and string loading.
62-126: LGTM!The
add_directory/add_directory_with_errorssplit is clean. The callback-based error reporting for per-file failures while propagating directory-level errors is a good design that addresses the prior concern about silent error swallowing.
128-153: LGTM!Name derivation from file stem with empty-name validation is correct.
314-359: LGTM!The parse helpers are well-factored:
parse_agent_confighandles the raw→config conversion,map_parse_errorpromotes schema validation errors correctly, andconfig_from_str_strictadds the empty-name guard.
361-392: LGTM!
matches_agent_patternandderive_agent_name_from_relare simple and well-tested. Theunwrap_orfallbacks handle edge cases where the prefix/suffix is absent gracefully (though in practicematches_agent_patternensures they're always present beforederive_agent_name_from_relis called).
394-1162: Thorough test coverage.Tests cover pattern matching, name derivation, directory scanning (single/nested/multiple dirs), non-md filtering, permission parsing (including flow syntax), mode defaults, in-memory overrides, string/bytes loading, UTF-8 validation, empty name rejection,
askpermission rejection, and hidden field acceptance. Well done.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…features enabled When both tokio and blocking features were enabled (e.g. via --all-features), the execute_command and fetch_url functions were exported twice from their respective modules, causing E0252 compile errors. Changes: - Changed bash module blocking imports to require 'not(feature = "tokio")' - Changed webfetch module blocking imports to require 'not(feature = "tokio")' Benefits: - CI passes with --all-features flag - Maintains tokio precedence when both features enabled - Aligns with documented mutual exclusivity of async/blocking features
The doc example at line 53 incorrectly declared loader as mutable even though AgentLoader methods take &self, not &mut self. Changes: - Changed let mut loader to let loader in the doc example - Aligns with the module-level example that already uses immutable binding Benefits: - Makes the documentation example more accurate - Follows Rust best practices for immutable bindings when mutation not needed
Split the monolithic build-and-test job into separate async and blocking jobs to properly handle mutually exclusive features, and adopt the upstream action's new packages field for cleaner package selection. Changes: - Split build-and-test into build-and-test-async and build-and-test-blocking - Use isolated caches for each job to prevent feature conflicts - Replace --exclude arguments with upstream packages field for blocking tests - Update publish-crate to depend on both test jobs Benefits: - Cleaner workflow using upstream's native package selection - Properly isolates async/blocking feature sets - Eliminates cache pollution between mutually exclusive features
Removed unnecessary OPENCODE_USE_ALLOWED environment variable toggle. Examples should demonstrate the safer pattern (allowed paths) by default without requiring env vars. Changes: - Default to AllowedPathResolver instead of conditional env var check - Rename serdesai-agents.md to file-reader.md for clarity - Update include_str! reference to match new filename - Simplify comments to reflect the new always-allowed approach Benefits: - Cleaner, more focused example code - Demonstrates secure-by-default patterns - More intuitive file naming
Extract token limits (context/output) from models.dev API payloads and expose them via new lookup APIs on ModelsDevCatalog. Supports both global model lookups and provider-scoped lookups with conflict detection when limits differ. Changes: - Added ModelLimits struct with context and output fields, publicly exported - Added ProviderModelsSnapshot enum for backward-compatible snapshot parsing (Legacy Vec<String> and Detailed HashMap variants) - Updated ModelsDevCatalog with provider-scoped limit storage and model-level convenience index - Added public lookup APIs: get_model_limits() and get_model_limits_for_provider() - Implemented conflict detection for models with different limits across providers - Updated snapshot ingestion from full API payloads to extract limits - Updated models-dev-update.rs binary to output detailed snapshots with limits - Added comprehensive tests for lookup, provider-scoped access, conflict handling, backward compatibility - Created tests/public_api.rs to verify ModelLimits is publicly importable - Updated README.md with ModelLimits usage examples - Regenerated data/models.dev.min.json with new detailed format containing limit metadata Benefits: - Consumers can now access token limits for model context window planning - Backward compatible with legacy snapshots without limit data - Provider-scoped lookups handle cases where the same model has different limits per provider - Conflict detection prevents returning ambiguous limits when providers disagree
Implemented support for `<provider>/<model>` specs alongside existing `provider:model` format. Added runtime fields to ResolvedModel for preserving original spec while tracking provider-inferred values. Changes: - Added `ParsedModelSpec` struct and `parse_model_spec()` for position-based delimiter detection - Added `runtime_provider`, `runtime_model_id`, `runtime_spec` fields to ResolvedModel - Added `infer_runtime_from_raw_spec()` for fallback provider inference from raw spec - Updated resolve_provider_model() to accept requested_spec parameter and populate runtime fields - Removed canonical `openai:` rewriting of user-facing specs in resolved.spec - Updated registry to use runtime fields instead of reparsing resolved.spec - Added debug_assert! for runtime field validation in registry - Updated tests to verify runtime fields and mixed delimiter scenarios - Updated README with OpenCode model spec documentation Benefits: - Supports OpenCode-style `<provider>/<model>` specs naturally - Preserves original user-facing spec while tracking runtime-inferred values separately - Eliminates need for registry to reparse resolved model specs - Better alignment between user intent and runtime behavior
Refactored AgentDefaults to accept any ModelResolver implementation through a shared trait object, enabling custom resolver injection while preserving default models.dev behavior via fallback construction. Changes: - Added SharedModelResolver type alias (Arc<dyn ModelResolver + Send + Sync>) - Refactored AgentDefaults to use Option<SharedModelResolver> instead of concrete ModelsDevResolver - Added manual Debug impl for AgentDefaults to mask sensitive data (api_key redaction) - Updated registry builder to accept abstract resolver via trait object parameter - Preserved default models.dev behavior via fallback construction in create_resolver_from_defaults - Added custom resolver injection test (TestCustomResolver) proving extensibility - Added Send bound verification test (agent_registry_builder_is_send) - Updated public API with SharedModelResolver re-export in lib.rs - Updated README with custom resolver injection documentation and example Benefits: - Enables pluggable resolver backends for different model catalogs or APIs - Maintains backward compatibility with existing models.dev-based workflows - Sensitive data masking prevents accidental credential exposure in logs - Send bound ensures registry builder is thread-safe for async contexts
…ey handling The example was failing because: 1. The file-reader agent config was missing `task: allow` permission, so it only had 2 tools instead of the Task tool needed for subagent delegation 2. The API key was not being passed through ProviderOverrides (AgentDefaults.api_key only works for OpenAI provider) Changes: - Added `task: allow` to agents/file-reader.md to enable Task tool delegation - Added SYNTHETIC_API_KEY const and get_synthetic_api_key() function - Updated provider_overrides_from_env() to pass API key via ProviderOverride - Updated AgentDefaults to use model_resolver: None (uses default models.dev resolver) - Updated README.md quick start and Task tool sections to match new patterns Benefits: - Example now works correctly with subagent delegation - API key can be set via const instead of only environment variable - Documentation reflects current best practices
These files were duplicates of src/types/config.rs and src/types/error.rs and were not referenced in lib.rs. The types/ versions are the active ones.
1 participant
This PR introduces a new registry-driven Task tool system that allows agents to invoke other agents with permission-based access control, using configuration files. This enables flexible, multi-agent workflows where primary agents can delegate tasks to specialized subagents.
Changes
New Features
New
llm-coding-tools-agentscrate:AgentLoader: Loads agent configurations from markdown files with YAML frontmatter (supportsagent/**/*.mdandagents/**/*.mdpatterns)AgentCatalog: Stores loaded agent configurations with lookup supportsubagent(supportive),primary(can spawn others),primary-only(can only run as main)TaskInput/TaskOutputtypes for task execution APISerdesAI Integration:
AgentRegistryBuilder: Builds framework-specific registries fromAgentCatalogTaskTool: Registry-driven task execution with permission validationToolCatalog: Precomputes tool availability per agent for performanceserdesai-agents.rsdemonstrating full registry-driven workflowConfiguration Format
It is the OpenCode format, you should generally find that agent files from OpenCode should be drop-in.
Documentation Updates
AGENTS.mdat root level (references src/AGENTS.md)Implementation Details
model: provider/model:tag)