mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
fix(agent): harden Kimi profile tool contracts
This commit is contained in:
Bryan Helmkamp
parent
cbd257c016
commit
eddee10b35
25 changed files with 1026 additions and 453 deletions
|
|
@ -11333,7 +11333,7 @@ components:
|
|||
|
||||
TodoListKind:
|
||||
type: string
|
||||
enum: [openai_plan, anthropic_tasks]
|
||||
enum: [openai_plan, anthropic_tasks, kimi_todos]
|
||||
description: |-
|
||||
Tool surface a todo list belongs to. Determines the `list_id`
|
||||
prefix and scoping convention.
|
||||
|
|
|
|||
|
|
@ -331,11 +331,14 @@ mod tests {
|
|||
fn native_tool_options_have_expected_profile_defaults() {
|
||||
let openai = NativeToolOptions::for_profile(AgentProfileKind::OpenAi);
|
||||
let anthropic = NativeToolOptions::for_profile(AgentProfileKind::Anthropic);
|
||||
let kimi = NativeToolOptions::for_profile(AgentProfileKind::Kimi);
|
||||
|
||||
assert_eq!(openai.default_command_timeout_ms, 10_000);
|
||||
assert_eq!(openai.max_command_timeout_ms, 600_000);
|
||||
assert_eq!(anthropic.default_command_timeout_ms, 120_000);
|
||||
assert_eq!(anthropic.max_command_timeout_ms, 600_000);
|
||||
assert_eq!(kimi.default_command_timeout_ms, 60_000);
|
||||
assert_eq!(kimi.max_command_timeout_ms, 600_000);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ use fabro_types::{
|
|||
};
|
||||
|
||||
use crate::memory::MemoryDocument;
|
||||
use crate::native_tool::NativeTool;
|
||||
use crate::native_tool::ToolVocabulary;
|
||||
use crate::skills::{Skill, format_skills_prompt_section};
|
||||
use crate::tool_registry::{ToolDefinitionWithSource, ToolSource};
|
||||
|
||||
|
|
@ -23,6 +23,7 @@ pub(crate) struct ContextWindowInput<'a> {
|
|||
pub system_prompt: &'a str,
|
||||
pub memory: &'a [MemoryDocument],
|
||||
pub skills: &'a [Skill],
|
||||
pub tool_vocabulary: ToolVocabulary,
|
||||
pub activated_skill_context_observed: bool,
|
||||
pub provider: &'a str,
|
||||
pub model: &'a str,
|
||||
|
|
@ -159,7 +160,7 @@ fn add_message_breakdown(
|
|||
input: &ContextWindowInput<'_>,
|
||||
) {
|
||||
let memory_text = memory_prompt_suffix(input.memory);
|
||||
let skills_text = skills_prompt_suffix(input.skills);
|
||||
let skills_text = skills_prompt_suffix(input.skills, input.tool_vocabulary);
|
||||
let memory_tokens = estimate_text_tokens(&memory_text);
|
||||
let skills_tokens = estimate_text_tokens(&skills_text);
|
||||
let mut system_parts_seen = false;
|
||||
|
|
@ -221,8 +222,8 @@ fn memory_prompt_suffix(memory: &[MemoryDocument]) -> String {
|
|||
}
|
||||
}
|
||||
|
||||
fn skills_prompt_suffix(skills: &[Skill]) -> String {
|
||||
let section = format_skills_prompt_section(skills, NativeTool::UseSkill.canonical_name());
|
||||
fn skills_prompt_suffix(skills: &[Skill], vocabulary: ToolVocabulary) -> String {
|
||||
let section = format_skills_prompt_section(skills, vocabulary);
|
||||
if section.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
|
|
@ -391,7 +392,7 @@ mod tests {
|
|||
let system_prompt = format!(
|
||||
"core prompt{}{}",
|
||||
memory_prompt_suffix(&memory),
|
||||
skills_prompt_suffix(&skills)
|
||||
skills_prompt_suffix(&skills, ToolVocabulary::Fabro)
|
||||
);
|
||||
let tools = vec![
|
||||
tool("read_file", ToolSource::Native),
|
||||
|
|
@ -415,6 +416,7 @@ mod tests {
|
|||
system_prompt: &system_prompt,
|
||||
memory: &memory,
|
||||
skills: &skills,
|
||||
tool_vocabulary: ToolVocabulary::Fabro,
|
||||
activated_skill_context_observed: true,
|
||||
provider: "test",
|
||||
model: "model-a",
|
||||
|
|
@ -447,6 +449,18 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skills_suffix_uses_the_profile_tool_vocabulary() {
|
||||
let skills = vec![Skill {
|
||||
name: "commit".to_string(),
|
||||
description: "Commit changes".to_string(),
|
||||
template: "commit template".to_string(),
|
||||
}];
|
||||
|
||||
assert!(skills_prompt_suffix(&skills, ToolVocabulary::Fabro).contains("`use_skill`"));
|
||||
assert!(skills_prompt_suffix(&skills, ToolVocabulary::KimiCode).contains("`Skill`"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn scaled_breakdown_totals_provider_count() {
|
||||
let local = StageContextWindowProjection {
|
||||
|
|
|
|||
|
|
@ -3,6 +3,16 @@ use std::fmt::Write;
|
|||
|
||||
use fabro_llm::types::{ToolCall, ToolResult};
|
||||
|
||||
use crate::native_tool::NativeTool;
|
||||
use crate::tool_permissions::canonical_tool_name;
|
||||
|
||||
fn file_path(arguments: &serde_json::Value) -> Option<&str> {
|
||||
arguments
|
||||
.get("file_path")
|
||||
.or_else(|| arguments.get("path"))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default)]
|
||||
struct FileOps {
|
||||
read: bool,
|
||||
|
|
@ -59,23 +69,23 @@ impl FileTracker {
|
|||
if result.is_error {
|
||||
continue;
|
||||
}
|
||||
match tc.name.as_str() {
|
||||
"read_file" => {
|
||||
if let Some(path) = tc.arguments.get("file_path").and_then(|v| v.as_str()) {
|
||||
match canonical_tool_name(&tc.name) {
|
||||
name if name == NativeTool::ReadFile.canonical_name() => {
|
||||
if let Some(path) = file_path(&tc.arguments) {
|
||||
self.record_read(path);
|
||||
}
|
||||
}
|
||||
"write_file" => {
|
||||
if let Some(path) = tc.arguments.get("file_path").and_then(|v| v.as_str()) {
|
||||
name if name == NativeTool::WriteFile.canonical_name() => {
|
||||
if let Some(path) = file_path(&tc.arguments) {
|
||||
self.record_write(path);
|
||||
}
|
||||
}
|
||||
"edit_file" => {
|
||||
if let Some(path) = tc.arguments.get("file_path").and_then(|v| v.as_str()) {
|
||||
name if name == NativeTool::EditFile.canonical_name() => {
|
||||
if let Some(path) = file_path(&tc.arguments) {
|
||||
self.record_edit(path);
|
||||
}
|
||||
}
|
||||
"apply_patch" => {
|
||||
name if name == NativeTool::ApplyPatch.canonical_name() => {
|
||||
let content = match result.content.as_str() {
|
||||
Some(s) => s.to_string(),
|
||||
None => result.content.to_string(),
|
||||
|
|
@ -166,6 +176,31 @@ mod tests {
|
|||
assert_eq!(tracker.render(), "- /tmp/baz.rs (edited)\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn record_from_kimi_tool_calls_uses_path_argument() {
|
||||
let mut tracker = FileTracker::default();
|
||||
let tool_calls = vec![
|
||||
ToolCall::new("tc1", "Read", serde_json::json!({"path": "/tmp/a.rs"})),
|
||||
ToolCall::new(
|
||||
"tc2",
|
||||
"Write",
|
||||
serde_json::json!({"path": "/tmp/b.rs", "content": "x"}),
|
||||
),
|
||||
ToolCall::new("tc3", "Edit", serde_json::json!({"path": "/tmp/c.rs"})),
|
||||
];
|
||||
let results = ["tc1", "tc2", "tc3"]
|
||||
.into_iter()
|
||||
.map(|id| ToolResult::success(id, serde_json::json!("ok")))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
tracker.record_from_tool_calls(&tool_calls, &results);
|
||||
|
||||
assert_eq!(
|
||||
tracker.render(),
|
||||
"- /tmp/a.rs (read)\n- /tmp/b.rs (written)\n- /tmp/c.rs (edited)\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn record_from_tool_calls_skips_errors() {
|
||||
let mut tracker = FileTracker::default();
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ pub use loop_detection::detect_loop;
|
|||
pub use memory::{MemoryDocument, discover_memory};
|
||||
pub use native_tool::{NativeTool, ToolVocabulary};
|
||||
pub use profiles::{
|
||||
AgentProfileBuilder, AnthropicProfile, EnvContext, GeminiProfile, OpenAiProfile,
|
||||
AgentProfileBuilder, AnthropicProfile, EnvContext, GeminiProfile, KimiProfile, OpenAiProfile,
|
||||
};
|
||||
pub use question_tools::{
|
||||
ANTHROPIC_ASK_USER_QUESTION_TOOL, AgentQuestion, AgentQuestionAnswer,
|
||||
|
|
|
|||
|
|
@ -34,8 +34,8 @@ pub async fn discover_memory(
|
|||
AgentProfileKind::Anthropic => vec!["AGENTS.md", "CLAUDE.md"],
|
||||
AgentProfileKind::OpenAi => vec!["AGENTS.md", ".codex/instructions.md"],
|
||||
AgentProfileKind::Gemini => vec!["AGENTS.md", "GEMINI.md"],
|
||||
// Kimi Code reads only AGENTS.md (and a lowercase variant); it has no
|
||||
// vendor-specific instruction filename of its own.
|
||||
// Kimi Code reads only AGENTS.md; it has no vendor-specific
|
||||
// instruction filename of its own.
|
||||
AgentProfileKind::Kimi => vec!["AGENTS.md"],
|
||||
};
|
||||
|
||||
|
|
@ -223,7 +223,7 @@ mod tests {
|
|||
assert_eq!(openai_docs[1].content, "copilot");
|
||||
|
||||
let env: Arc<dyn Sandbox> = Arc::new(MockSandbox {
|
||||
files,
|
||||
files: files.clone(),
|
||||
..Default::default()
|
||||
});
|
||||
let gemini_docs = discover_memory(
|
||||
|
|
@ -238,6 +238,22 @@ mod tests {
|
|||
assert_eq!(gemini_docs.len(), 2);
|
||||
assert_eq!(gemini_docs[0].content, "agents");
|
||||
assert_eq!(gemini_docs[1].content, "gemini");
|
||||
|
||||
let env: Arc<dyn Sandbox> = Arc::new(MockSandbox {
|
||||
files,
|
||||
..Default::default()
|
||||
});
|
||||
let kimi_docs = discover_memory(
|
||||
env.as_ref(),
|
||||
"/repo",
|
||||
"/repo",
|
||||
AgentProfileKind::Kimi,
|
||||
&CancellationToken::new(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(kimi_docs.len(), 1);
|
||||
assert_eq!(kimi_docs[0].content, "agents");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
|
|||
|
|
@ -34,27 +34,27 @@ pub enum ToolVocabulary {
|
|||
Debug, Clone, Copy, PartialEq, Eq, Hash, Display, EnumString, IntoStaticStr, VariantArray,
|
||||
)]
|
||||
pub enum NativeTool {
|
||||
#[strum(to_string = "read_file")]
|
||||
#[strum(to_string = "read_file", serialize = "Read")]
|
||||
ReadFile,
|
||||
#[strum(to_string = "read_many_files")]
|
||||
ReadManyFiles,
|
||||
#[strum(to_string = "write_file")]
|
||||
#[strum(to_string = "write_file", serialize = "Write")]
|
||||
WriteFile,
|
||||
#[strum(to_string = "edit_file")]
|
||||
#[strum(to_string = "edit_file", serialize = "Edit")]
|
||||
EditFile,
|
||||
#[strum(to_string = "apply_patch")]
|
||||
ApplyPatch,
|
||||
#[strum(to_string = "list_dir")]
|
||||
ListDir,
|
||||
#[strum(to_string = "grep")]
|
||||
#[strum(to_string = "grep", serialize = "Grep")]
|
||||
Grep,
|
||||
#[strum(to_string = "glob")]
|
||||
#[strum(to_string = "glob", serialize = "Glob")]
|
||||
Glob,
|
||||
#[strum(to_string = "shell")]
|
||||
#[strum(to_string = "shell", serialize = "Bash")]
|
||||
Shell,
|
||||
#[strum(to_string = "web_search")]
|
||||
#[strum(to_string = "web_search", serialize = "WebSearch")]
|
||||
WebSearch,
|
||||
#[strum(to_string = "web_fetch")]
|
||||
#[strum(to_string = "web_fetch", serialize = "FetchURL")]
|
||||
WebFetch,
|
||||
#[strum(to_string = "spawn_agent")]
|
||||
SpawnAgent,
|
||||
|
|
@ -64,7 +64,7 @@ pub enum NativeTool {
|
|||
Wait,
|
||||
#[strum(to_string = "close_agent")]
|
||||
CloseAgent,
|
||||
#[strum(to_string = "use_skill")]
|
||||
#[strum(to_string = "use_skill", serialize = "Skill")]
|
||||
UseSkill,
|
||||
#[strum(to_string = "update_plan")]
|
||||
UpdatePlan,
|
||||
|
|
@ -93,6 +93,19 @@ impl NativeTool {
|
|||
self.into()
|
||||
}
|
||||
|
||||
/// Resolve a canonical fabro name to its built-in identity.
|
||||
///
|
||||
/// Unlike [`Self::from_any_name`], this deliberately ignores provider
|
||||
/// aliases. Registries use it while registering tools so an unrelated
|
||||
/// extension named `Read` is not silently treated as fabro's file reader.
|
||||
#[must_use]
|
||||
pub fn from_canonical_name(name: &str) -> Option<Self> {
|
||||
Self::VARIANTS
|
||||
.iter()
|
||||
.copied()
|
||||
.find(|tool| tool.canonical_name() == name)
|
||||
}
|
||||
|
||||
/// The name this tool is exposed under in `vocabulary`.
|
||||
///
|
||||
/// A tool with no counterpart in the vocabulary keeps its canonical name.
|
||||
|
|
@ -130,11 +143,7 @@ impl NativeTool {
|
|||
/// not drawn from this set.
|
||||
#[must_use]
|
||||
pub fn from_any_name(name: &str) -> Option<Self> {
|
||||
Self::VARIANTS.iter().copied().find(|tool| {
|
||||
ToolVocabulary::VARIANTS
|
||||
.iter()
|
||||
.any(|vocabulary| tool.name(*vocabulary) == name)
|
||||
})
|
||||
name.parse().ok()
|
||||
}
|
||||
|
||||
/// Coarse access category, or `None` when the tool is not part of the
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ use crate::skills::Skill;
|
|||
use crate::todo_runtime::TodoRuntime;
|
||||
use crate::todo_tools::make_todo_list_tool;
|
||||
use crate::tool_registry::ToolRegistry;
|
||||
use crate::tools::{WebFetchSummarizer, make_edit_file_tool, register_core_tools};
|
||||
use crate::tools::{WebFetchSummarizer, register_discovery_and_web_tools};
|
||||
|
||||
const CORE_PROMPT: &str = include_str!("prompts/kimi.md.j2");
|
||||
|
||||
|
|
@ -67,21 +67,18 @@ impl KimiProfile {
|
|||
// (subagent tools, skills) are renamed too.
|
||||
let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::KimiCode);
|
||||
|
||||
register_core_tools(&mut registry, options, summarizer);
|
||||
registry.register(make_edit_file_tool());
|
||||
|
||||
// Read, Write, and Bash differ from fabro's built-ins in what their
|
||||
// parameters mean, not just what they are called, so they are separate
|
||||
// tools rather than renames. Registered after the core set so they
|
||||
// replace it.
|
||||
// Glob and the web tools have the same contract in both vocabularies.
|
||||
// The remaining Kimi tools use adapters for their different schemas,
|
||||
// while reusing shared execution helpers where their behavior agrees.
|
||||
register_discovery_and_web_tools(&mut registry, options, summarizer);
|
||||
registry.register(kimi_tools::make_kimi_read_tool());
|
||||
registry.register(kimi_tools::make_kimi_write_tool());
|
||||
registry.register(kimi_tools::make_kimi_edit_tool(EDIT_FILE_DESCRIPTION));
|
||||
registry.register(kimi_tools::make_kimi_grep_tool());
|
||||
registry.register(kimi_tools::make_kimi_bash_tool(
|
||||
options.default_command_timeout_ms,
|
||||
options.max_command_timeout_ms,
|
||||
));
|
||||
registry.redescribe(NativeTool::EditFile, EDIT_FILE_DESCRIPTION);
|
||||
registry.redescribe(NativeTool::Glob, GLOB_DESCRIPTION);
|
||||
|
||||
// Kimi Code drives todos with one replace-whole-list call. The
|
||||
|
|
@ -172,7 +169,7 @@ mod tests {
|
|||
use fabro_types::AgentToolCategory;
|
||||
|
||||
use super::*;
|
||||
use crate::skills::make_use_skill_tool;
|
||||
use crate::skills::make_use_skill_tool_for_vocabulary;
|
||||
use crate::subagent::{SessionFactory, SubAgentSupervisor};
|
||||
use crate::test_support::MockSandbox;
|
||||
use crate::tool_permissions::{known_tool_category, tool_category};
|
||||
|
|
@ -224,9 +221,8 @@ mod tests {
|
|||
fn renamed_tools_keep_their_permission_category() {
|
||||
let profile = KimiProfile::new("kimi-k3");
|
||||
for name in profile.tool_registry().names() {
|
||||
let Some(tool) = NativeTool::from_any_name(&name) else {
|
||||
continue;
|
||||
};
|
||||
let tool = NativeTool::from_any_name(&name)
|
||||
.unwrap_or_else(|| panic!("unexpected non-native Kimi profile tool: {name}"));
|
||||
assert_eq!(
|
||||
known_tool_category(&name),
|
||||
tool.category(),
|
||||
|
|
@ -270,15 +266,27 @@ mod tests {
|
|||
profile.register_subagent_tools(SubAgentSupervisor::new(3), factory, 0);
|
||||
profile
|
||||
.tool_registry_mut()
|
||||
.register(make_use_skill_tool(Arc::new(vec![Skill {
|
||||
name: "demo".into(),
|
||||
description: "d".into(),
|
||||
template: "t".into(),
|
||||
}])));
|
||||
.register(make_use_skill_tool_for_vocabulary(
|
||||
Arc::new(vec![Skill {
|
||||
name: "demo".into(),
|
||||
description: "d".into(),
|
||||
template: "t".into(),
|
||||
}]),
|
||||
ToolVocabulary::KimiCode,
|
||||
));
|
||||
|
||||
let names = profile.tool_registry().names();
|
||||
assert!(names.contains(&"Skill".to_string()), "got {names:?}");
|
||||
assert!(!names.contains(&"use_skill".to_string()), "got {names:?}");
|
||||
let skill_parameters = &profile
|
||||
.tool_registry()
|
||||
.get("Skill")
|
||||
.unwrap()
|
||||
.definition
|
||||
.parameters;
|
||||
assert!(skill_parameters["properties"].get("skill").is_some());
|
||||
assert!(skill_parameters["properties"].get("args").is_some());
|
||||
assert!(skill_parameters["properties"].get("skill_name").is_none());
|
||||
// Deliberately not renamed to Kimi Code's `Agent`: fabro's subagent
|
||||
// tools are a supervisor model, not a call-and-return one.
|
||||
assert!(names.contains(&"spawn_agent".to_string()), "got {names:?}");
|
||||
|
|
@ -335,8 +343,24 @@ mod tests {
|
|||
let grep = describe("Grep");
|
||||
assert!(grep.contains("POSIX"), "{grep}");
|
||||
assert!(describe("Glob").contains("most recently modified"));
|
||||
// The shared description is untouched for other profiles.
|
||||
assert!(!describe("Read").contains("refuses writes"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kimi_edit_schema_uses_path_like_kimi_code() {
|
||||
let profile = KimiProfile::new("kimi-k3");
|
||||
let parameters = &profile
|
||||
.tool_registry()
|
||||
.get("Edit")
|
||||
.unwrap()
|
||||
.definition
|
||||
.parameters;
|
||||
|
||||
assert!(parameters["properties"].get("path").is_some());
|
||||
assert!(parameters["properties"].get("file_path").is_none());
|
||||
assert_eq!(
|
||||
parameters["required"],
|
||||
serde_json::json!(["path", "old_string", "new_string"])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -17,20 +17,26 @@
|
|||
//! [`Sandbox`](crate::sandbox::Sandbox) methods the built-ins use, so sandbox
|
||||
//! behavior, path policy, and the read-before-write guard are unchanged.
|
||||
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::fmt::Write as _;
|
||||
use std::str::FromStr;
|
||||
use std::sync::Arc;
|
||||
|
||||
use fabro_llm::types::ToolDefinition;
|
||||
use serde_json::Value;
|
||||
use strum::EnumString;
|
||||
|
||||
use crate::native_tool::NativeTool;
|
||||
use crate::sandbox::GrepOptions;
|
||||
use crate::sandbox::{GrepOptions, format_lines_numbered};
|
||||
use crate::tool_registry::{RegisteredTool, ToolSource};
|
||||
use crate::tools::{optional_usize_arg, required_str};
|
||||
use crate::tools::{
|
||||
DEFAULT_READ_LINES, execute_grep, execute_shell_command, grep_result_path, make_edit_file_tool,
|
||||
optional_usize_arg, required_str,
|
||||
};
|
||||
|
||||
/// Largest `n_lines` a single `Read` call returns, matching fabro's built-in
|
||||
/// read default so the two tools cannot disagree about how much is "a page".
|
||||
const DEFAULT_READ_LINES: usize = 2000;
|
||||
const DEFAULT_GREP_RESULTS: usize = 250;
|
||||
const MAX_GREP_RESULTS: usize = 2000;
|
||||
const MAX_GREP_MATCHES_SCANNED: usize = 20_000;
|
||||
|
||||
fn definition(tool: NativeTool, description: &str, parameters: Value) -> ToolDefinition {
|
||||
ToolDefinition {
|
||||
|
|
@ -114,13 +120,15 @@ explicitly asked. Never run commands requiring superuser privileges unless expli
|
|||
None => default_timeout_ms,
|
||||
};
|
||||
|
||||
let result = ctx
|
||||
.env
|
||||
.exec_command(command, timeout_ms, cwd, None, Some(ctx.cancel.clone()))
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
let result = execute_shell_command(&ctx, command, timeout_ms, cwd).await?;
|
||||
|
||||
let mut out = result.stdout;
|
||||
let mut out = String::new();
|
||||
if result.is_timed_out() {
|
||||
out.push_str("Command timed out.\n");
|
||||
} else if result.is_cancelled() {
|
||||
out.push_str("Command cancelled.\n");
|
||||
}
|
||||
out.push_str(&result.stdout);
|
||||
if !result.stderr.is_empty() {
|
||||
if !out.is_empty() {
|
||||
out.push('\n');
|
||||
|
|
@ -154,8 +162,8 @@ read in this session.
|
|||
- If you have a concrete path, call Read directly. Do not Glob or `ls` first to check that it \
|
||||
exists — a missing path returns an error you can handle.
|
||||
- When you need several files, emit multiple Read calls in one response rather than one per turn.
|
||||
- Returns `<line-number>\\t<content>` per line. Drop the number and tab when taking text for an \
|
||||
Edit `old_string`.
|
||||
- Returns `<line-number> | <content>` per line. Drop the number and separator when taking text for \
|
||||
an Edit `old_string`.
|
||||
- `line_offset` is the 1-based first line to read. A NEGATIVE value reads from the end, so -100 \
|
||||
returns the last 100 lines.
|
||||
- `n_lines` defaults to 2000 lines.
|
||||
|
|
@ -166,11 +174,14 @@ returns the last 100 lines.
|
|||
"path": {"type": "string", "description": "Path to the file to read."},
|
||||
"line_offset": {
|
||||
"type": "integer",
|
||||
"minimum": -2000,
|
||||
"description": "1-based first line to read. Negative reads from the end \
|
||||
of the file (-100 reads the last 100 lines)."
|
||||
of the file (-100 reads the last 100 lines); zero is invalid."
|
||||
},
|
||||
"n_lines": {
|
||||
"type": "integer",
|
||||
"minimum": 1,
|
||||
"maximum": 2000,
|
||||
"description": "Number of lines to read (default 2000)."
|
||||
}
|
||||
},
|
||||
|
|
@ -180,8 +191,16 @@ returns the last 100 lines.
|
|||
executor: Arc::new(|args, ctx| {
|
||||
Box::pin(async move {
|
||||
let path = required_str(&args, "path")?;
|
||||
let n_lines = optional_usize_arg(&args, "n_lines")?;
|
||||
let n_lines = optional_usize_arg(&args, "n_lines")?.unwrap_or(DEFAULT_READ_LINES);
|
||||
if n_lines == 0 || n_lines > DEFAULT_READ_LINES {
|
||||
return Err(format!(
|
||||
"n_lines must be between 1 and {DEFAULT_READ_LINES}"
|
||||
));
|
||||
}
|
||||
let line_offset = args.get("line_offset").and_then(Value::as_i64);
|
||||
if line_offset == Some(0) {
|
||||
return Err("line_offset must not be zero".to_string());
|
||||
}
|
||||
|
||||
let content = match line_offset {
|
||||
// Negative offset: count the file's lines, then start that
|
||||
|
|
@ -189,28 +208,30 @@ returns the last 100 lines.
|
|||
Some(offset) if offset < 0 => {
|
||||
let from_end = usize::try_from(offset.unsigned_abs())
|
||||
.map_err(|_| "line_offset is too large".to_string())?;
|
||||
let total = ctx
|
||||
if from_end > DEFAULT_READ_LINES {
|
||||
return Err(format!(
|
||||
"negative line_offset must be at least -{DEFAULT_READ_LINES}"
|
||||
));
|
||||
}
|
||||
let raw = ctx
|
||||
.env
|
||||
.read_file(path, None, None)
|
||||
.read_file_text(path)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?
|
||||
.lines()
|
||||
.count();
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
let total = raw.lines().count();
|
||||
let start = total.saturating_sub(from_end).saturating_add(1);
|
||||
ctx.env
|
||||
.read_file(path, Some(start), n_lines.or(Some(from_end)))
|
||||
.await
|
||||
Ok(format_lines_numbered(
|
||||
&raw,
|
||||
Some(start),
|
||||
Some(n_lines.min(from_end)),
|
||||
))
|
||||
}
|
||||
Some(offset) => {
|
||||
let start = usize::try_from(offset)
|
||||
.map_err(|_| "line_offset must fit in usize".to_string())?;
|
||||
ctx.env.read_file(path, Some(start), n_lines).await
|
||||
}
|
||||
None => {
|
||||
ctx.env
|
||||
.read_file(path, None, n_lines.or(Some(DEFAULT_READ_LINES)))
|
||||
.await
|
||||
ctx.env.read_file(path, Some(start), Some(n_lines)).await
|
||||
}
|
||||
None => ctx.env.read_file(path, None, Some(n_lines)).await,
|
||||
}
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
|
||||
|
|
@ -222,6 +243,14 @@ returns the last 100 lines.
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Default, EnumString)]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
enum KimiWriteMode {
|
||||
#[default]
|
||||
Overwrite,
|
||||
Append,
|
||||
}
|
||||
|
||||
/// `Write`, with Kimi Code's `mode` so it can append.
|
||||
#[must_use]
|
||||
pub fn make_kimi_write_tool() -> RegisteredTool {
|
||||
|
|
@ -261,27 +290,32 @@ overwrite replaces everything you did not restate.
|
|||
let mode = args
|
||||
.get("mode")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("overwrite");
|
||||
.unwrap_or("overwrite")
|
||||
.parse::<KimiWriteMode>()
|
||||
.map_err(|_| "Invalid mode (expected overwrite|append)".to_string())?;
|
||||
|
||||
let payload = match mode {
|
||||
"overwrite" => content.to_string(),
|
||||
match mode {
|
||||
KimiWriteMode::Overwrite => {
|
||||
ctx.env
|
||||
.write_file(path, content)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
}
|
||||
// The sandbox trait has no append; read-modify-write keeps
|
||||
// every provider working and stays inside path policy.
|
||||
"append" => {
|
||||
let existing = ctx.env.read_file_text(path).await.unwrap_or_default();
|
||||
format!("{existing}{content}")
|
||||
KimiWriteMode::Append => {
|
||||
let mut existing = ctx
|
||||
.env
|
||||
.read_file_text(path)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
existing.push_str(content);
|
||||
ctx.env
|
||||
.write_file(path, &existing)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
}
|
||||
other => {
|
||||
return Err(format!(
|
||||
"Invalid mode `{other}` (expected overwrite|append)"
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
ctx.env
|
||||
.write_file(path, &payload)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
}
|
||||
Ok(format!("Wrote {path}"))
|
||||
})
|
||||
}),
|
||||
|
|
@ -289,6 +323,48 @@ overwrite replaces everything you did not restate.
|
|||
}
|
||||
}
|
||||
|
||||
/// Kimi Code's `Edit` schema names the target `path`; fabro's shared edit
|
||||
/// executor calls it `file_path`. Translate only that adapter field and reuse
|
||||
/// the exact-match/read-before-write implementation.
|
||||
#[must_use]
|
||||
pub fn make_kimi_edit_tool(description: &str) -> RegisteredTool {
|
||||
let shared = make_edit_file_tool();
|
||||
let shared_executor = shared.executor;
|
||||
RegisteredTool {
|
||||
definition: definition(
|
||||
NativeTool::EditFile,
|
||||
description,
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"path": {"type": "string", "description": "Path to the text file to edit."},
|
||||
"old_string": {"type": "string", "description": "Exact content to replace."},
|
||||
"new_string": {"type": "string", "description": "Replacement text."},
|
||||
"replace_all": {
|
||||
"type": "boolean",
|
||||
"description": "Replace every occurrence (default false)."
|
||||
}
|
||||
},
|
||||
"required": ["path", "old_string", "new_string"]
|
||||
}),
|
||||
),
|
||||
executor: Arc::new(move |mut args, ctx| {
|
||||
let shared_executor = shared_executor.clone();
|
||||
Box::pin(async move {
|
||||
let object = args
|
||||
.as_object_mut()
|
||||
.ok_or_else(|| "Edit arguments must be an object".to_string())?;
|
||||
let path = object
|
||||
.remove("path")
|
||||
.ok_or_else(|| "Missing required parameter: path".to_string())?;
|
||||
object.insert("file_path".to_string(), path);
|
||||
shared_executor(args, ctx).await
|
||||
})
|
||||
}),
|
||||
source: ToolSource::Native,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
|
|
@ -297,7 +373,7 @@ mod tests {
|
|||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use super::*;
|
||||
use crate::sandbox::Sandbox;
|
||||
use crate::sandbox::{ExecResult, Sandbox};
|
||||
use crate::test_support::{MockSandbox, MutableMockSandbox};
|
||||
use crate::tool_registry::ToolContext;
|
||||
|
||||
|
|
@ -358,6 +434,22 @@ mod tests {
|
|||
assert!(!out.contains("line8"), "{out}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_positive_offset_still_applies_the_default_limit() {
|
||||
let lines: Vec<String> = (1..=DEFAULT_READ_LINES + 5)
|
||||
.map(|n| format!("line{n}"))
|
||||
.collect();
|
||||
let env = sandbox_with("/f.txt", &lines.join("\n"));
|
||||
let tool = make_kimi_read_tool();
|
||||
|
||||
let out = (tool.executor)(json!({"path": "/f.txt", "line_offset": 2}), ctx(env))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(out.contains("2001 | line2001"), "{out}");
|
||||
assert!(!out.contains("2002 | line2002"), "{out}");
|
||||
}
|
||||
|
||||
/// The reason Write is a separate tool: it has a mode, so it can append.
|
||||
#[tokio::test]
|
||||
async fn write_append_mode_preserves_existing_content() {
|
||||
|
|
@ -400,6 +492,47 @@ mod tests {
|
|||
assert!(err.contains("expected overwrite|append"), "{err}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn write_append_propagates_a_missing_file_error() {
|
||||
let env = Arc::new(MutableMockSandbox::new(HashMap::new()));
|
||||
let tool = make_kimi_write_tool();
|
||||
|
||||
let err = (tool.executor)(
|
||||
json!({"path": "/missing.txt", "content": "new", "mode": "append"}),
|
||||
ctx(env),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(err.contains("missing.txt"), "{err}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn edit_translates_kimi_path_to_the_shared_executor() {
|
||||
let env = sandbox_with("/f.txt", "before");
|
||||
env.mark_agent_read("/f.txt");
|
||||
let tool = make_kimi_edit_tool("Edit");
|
||||
|
||||
(tool.executor)(
|
||||
json!({"path": "/f.txt", "old_string": "before", "new_string": "after"}),
|
||||
ctx(env.clone()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(env.read_file_text("/f.txt").await.unwrap(), "after");
|
||||
assert!(
|
||||
tool.definition.parameters["properties"]
|
||||
.get("path")
|
||||
.is_some()
|
||||
);
|
||||
assert!(
|
||||
tool.definition.parameters["properties"]
|
||||
.get("file_path")
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
/// `files_with_matches` and `count` both need the file path, which the
|
||||
/// underlying search only prefixes when scanning a directory.
|
||||
#[test]
|
||||
|
|
@ -432,7 +565,7 @@ mod tests {
|
|||
|
||||
#[tokio::test]
|
||||
async fn grep_content_mode_returns_matching_lines() {
|
||||
let out = grep_with(json!({"pattern": "x"}), vec![
|
||||
let out = grep_with(json!({"pattern": "x", "output_mode": "content"}), vec![
|
||||
"a.rs:1:x".into(),
|
||||
"b.rs:2:x".into(),
|
||||
])
|
||||
|
|
@ -441,6 +574,18 @@ mod tests {
|
|||
assert_eq!(out, "a.rs:1:x\nb.rs:2:x");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn grep_defaults_to_files_with_matches() {
|
||||
let out = grep_with(json!({"pattern": "x"}), vec![
|
||||
"a.rs:1:x".into(),
|
||||
"a.rs:2:x".into(),
|
||||
"b.rs:2:x".into(),
|
||||
])
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(out, "a.rs\nb.rs");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn grep_files_with_matches_deduplicates_paths_in_order() {
|
||||
let out = grep_with(
|
||||
|
|
@ -454,11 +599,10 @@ mod tests {
|
|||
|
||||
#[tokio::test]
|
||||
async fn grep_count_mode_counts_per_file() {
|
||||
let out = grep_with(json!({"pattern": "x", "output_mode": "count"}), vec![
|
||||
"a.rs:1:x".into(),
|
||||
"a.rs:9:x".into(),
|
||||
"b.rs:2:x".into(),
|
||||
])
|
||||
let out = grep_with(
|
||||
json!({"pattern": "x", "output_mode": "count_matches"}),
|
||||
vec!["a.rs:1:x".into(), "a.rs:9:x".into(), "b.rs:2:x".into()],
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(out, "a.rs:2\nb.rs:1");
|
||||
|
|
@ -467,9 +611,17 @@ mod tests {
|
|||
#[tokio::test]
|
||||
async fn grep_offset_and_head_limit_page_results() {
|
||||
let lines: Vec<String> = (1..=6).map(|n| format!("f{n}.rs:1:x")).collect();
|
||||
let out = grep_with(json!({"pattern": "x", "offset": 2, "head_limit": 2}), lines)
|
||||
.await
|
||||
.unwrap();
|
||||
let out = grep_with(
|
||||
json!({
|
||||
"pattern": "x",
|
||||
"output_mode": "content",
|
||||
"offset": 2,
|
||||
"head_limit": 2
|
||||
}),
|
||||
lines,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(out, "f3.rs:1:x\nf4.rs:1:x");
|
||||
}
|
||||
|
||||
|
|
@ -479,7 +631,7 @@ mod tests {
|
|||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
err.contains("expected content|files_with_matches|count"),
|
||||
err.contains("expected content|files_with_matches|count_matches"),
|
||||
"{err}"
|
||||
);
|
||||
}
|
||||
|
|
@ -490,6 +642,17 @@ mod tests {
|
|||
assert_eq!(out, "No matches found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn grep_schema_uses_kimi_code_modes_and_flags() {
|
||||
let parameters = make_kimi_grep_tool().definition.parameters;
|
||||
assert_eq!(
|
||||
parameters["properties"]["output_mode"]["enum"],
|
||||
json!(["content", "files_with_matches", "count_matches"])
|
||||
);
|
||||
assert!(parameters["properties"].get("-i").is_some());
|
||||
assert!(parameters["properties"].get("case_insensitive").is_none());
|
||||
}
|
||||
|
||||
/// The reason Bash is a separate tool: `timeout` is seconds, not
|
||||
/// milliseconds. A rename would have made every timeout 1000x wrong.
|
||||
#[test]
|
||||
|
|
@ -511,49 +674,57 @@ mod tests {
|
|||
// Fabro has no background shell, so none is promised.
|
||||
assert!(!tool.definition.description.contains("run_in_background"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn bash_reuses_session_env_cwd_and_timeout_rendering() {
|
||||
use fabro_types::CommandTermination;
|
||||
|
||||
let tool = make_kimi_bash_tool(60_000, 600_000);
|
||||
let env = Arc::new(MockSandbox {
|
||||
exec_result: ExecResult {
|
||||
stdout: String::new(),
|
||||
stderr: String::new(),
|
||||
exit_code: None,
|
||||
termination: CommandTermination::TimedOut,
|
||||
duration_ms: 7_000,
|
||||
},
|
||||
..MockSandbox::default()
|
||||
});
|
||||
let mut tool_ctx = ctx(env.clone());
|
||||
let tool_env = HashMap::from([("TOKEN".to_string(), "value".to_string())]);
|
||||
tool_ctx.tool_env_provider = Some(Arc::new(crate::StaticEnvProvider(tool_env.clone())));
|
||||
|
||||
let output = (tool.executor)(
|
||||
json!({"command": "echo $TOKEN", "cwd": "/repo", "timeout": 7}),
|
||||
tool_ctx,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(output.starts_with("Command timed out.\n"), "{output}");
|
||||
assert_eq!(*env.captured_timeout.lock().unwrap(), Some(7_000));
|
||||
assert_eq!(env.captured_working_dirs.lock().unwrap().as_slice(), &[
|
||||
Some("/repo".to_string())
|
||||
]);
|
||||
assert_eq!(*env.captured_env_vars.lock().unwrap(), Some(tool_env));
|
||||
assert!(
|
||||
env.captured_command
|
||||
.lock()
|
||||
.unwrap()
|
||||
.as_deref()
|
||||
.is_some_and(|command| command.starts_with("exec 2>&1\n"))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Output shapes Kimi Code's `Grep` supports.
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
#[derive(Clone, Copy, Default, PartialEq, Eq, EnumString)]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
enum GrepOutputMode {
|
||||
Content,
|
||||
#[default]
|
||||
FilesWithMatches,
|
||||
Count,
|
||||
}
|
||||
|
||||
impl GrepOutputMode {
|
||||
fn parse(value: Option<&str>) -> Result<Self, String> {
|
||||
match value.unwrap_or("content") {
|
||||
"content" => Ok(Self::Content),
|
||||
"files_with_matches" => Ok(Self::FilesWithMatches),
|
||||
"count" => Ok(Self::Count),
|
||||
other => Err(format!(
|
||||
"Invalid output_mode `{other}` (expected content|files_with_matches|count)"
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract the file path from a grep result line.
|
||||
///
|
||||
/// The underlying search emits `<path>:<line>:<content>` when scanning a
|
||||
/// directory, but omits the path when scanning a single file, so fall back to
|
||||
/// the path that was searched.
|
||||
fn grep_result_path<'a>(line: &'a str, searched: &'a str) -> &'a str {
|
||||
// Walk candidate separators so absolute Windows-style paths and paths
|
||||
// containing colons still split at the line-number field.
|
||||
let mut rest = line;
|
||||
let mut consumed = 0usize;
|
||||
while let Some(idx) = rest.find(':') {
|
||||
let after = &rest[idx + 1..];
|
||||
let digits: String = after.chars().take_while(char::is_ascii_digit).collect();
|
||||
if !digits.is_empty() && after[digits.len()..].starts_with(':') {
|
||||
return &line[..consumed + idx];
|
||||
}
|
||||
consumed += idx + 1;
|
||||
rest = after;
|
||||
}
|
||||
searched
|
||||
CountMatches,
|
||||
}
|
||||
|
||||
/// `Grep` with Kimi Code's `output_mode`, `head_limit`, and `offset`.
|
||||
|
|
@ -576,11 +747,11 @@ will not flood the conversation.
|
|||
|
||||
- Backed by ripgrep when available and POSIX `grep` otherwise, so keep patterns portable across \
|
||||
both rather than relying on ripgrep-only syntax.
|
||||
- `output_mode` selects what comes back: `content` (matching lines, the default), \
|
||||
`files_with_matches` (just the paths), or `count` (matches per file).
|
||||
- `output_mode` selects what comes back: `files_with_matches` (just the paths, the default), \
|
||||
`content` (matching lines), or `count_matches` (matches per file).
|
||||
- `head_limit` caps how many results are returned and `offset` skips that many first, so you can \
|
||||
page through a large result set.
|
||||
- `glob` limits which files are searched; `case_insensitive` folds case.",
|
||||
- `glob` limits which files are searched; `-i` folds case.",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
|
@ -589,12 +760,22 @@ page through a large result set.
|
|||
"glob": {"type": "string", "description": "Only search files matching this glob."},
|
||||
"output_mode": {
|
||||
"type": "string",
|
||||
"enum": ["content", "files_with_matches", "count"],
|
||||
"description": "Shape of the results (default content)."
|
||||
"enum": ["content", "files_with_matches", "count_matches"],
|
||||
"description": "Shape of the results (default files_with_matches)."
|
||||
},
|
||||
"head_limit": {"type": "integer", "description": "Return at most this many results."},
|
||||
"offset": {"type": "integer", "description": "Skip this many results before returning."},
|
||||
"case_insensitive": {"type": "boolean", "description": "Fold case when matching."}
|
||||
"head_limit": {
|
||||
"type": "integer",
|
||||
"minimum": 1,
|
||||
"maximum": 2000,
|
||||
"description": "Return at most this many results (default 250)."
|
||||
},
|
||||
"offset": {
|
||||
"type": "integer",
|
||||
"minimum": 0,
|
||||
"maximum": 20000,
|
||||
"description": "Skip this many results before returning."
|
||||
},
|
||||
"-i": {"type": "boolean", "description": "Perform a case-insensitive search."}
|
||||
},
|
||||
"required": ["pattern"]
|
||||
}),
|
||||
|
|
@ -604,66 +785,88 @@ page through a large result set.
|
|||
let pattern = required_str(&args, "pattern")?;
|
||||
// The trait requires a search root; "." is the working directory.
|
||||
let path = args.get("path").and_then(Value::as_str).unwrap_or(".");
|
||||
let mode = GrepOutputMode::parse(args.get("output_mode").and_then(Value::as_str))?;
|
||||
let head_limit = optional_usize_arg(&args, "head_limit")?;
|
||||
let mode = GrepOutputMode::from_str(
|
||||
args.get("output_mode")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("files_with_matches"),
|
||||
)
|
||||
.map_err(|_| {
|
||||
"Invalid output_mode (expected content|files_with_matches|count_matches)"
|
||||
.to_string()
|
||||
})?;
|
||||
let head_limit =
|
||||
optional_usize_arg(&args, "head_limit")?.unwrap_or(DEFAULT_GREP_RESULTS);
|
||||
if head_limit == 0 || head_limit > MAX_GREP_RESULTS {
|
||||
return Err(format!(
|
||||
"head_limit must be between 1 and {MAX_GREP_RESULTS}"
|
||||
));
|
||||
}
|
||||
let offset = optional_usize_arg(&args, "offset")?.unwrap_or(0);
|
||||
if offset > MAX_GREP_MATCHES_SCANNED {
|
||||
return Err(format!("offset must be at most {MAX_GREP_MATCHES_SCANNED}"));
|
||||
}
|
||||
if offset.saturating_add(head_limit) > MAX_GREP_MATCHES_SCANNED {
|
||||
return Err(format!(
|
||||
"offset + head_limit must be at most {MAX_GREP_MATCHES_SCANNED}"
|
||||
));
|
||||
}
|
||||
|
||||
let options = GrepOptions {
|
||||
glob_filter: args.get("glob").and_then(Value::as_str).map(str::to_string),
|
||||
case_insensitive: args
|
||||
.get("case_insensitive")
|
||||
.and_then(Value::as_bool)
|
||||
.unwrap_or(false),
|
||||
// Only push the cap down for `content`, where results and
|
||||
// lines are the same thing. Capping lines early would
|
||||
// undercount files for the other modes.
|
||||
case_insensitive: args.get("-i").and_then(Value::as_bool).unwrap_or(false),
|
||||
max_results: match mode {
|
||||
GrepOutputMode::Content => head_limit.map(|n| n.saturating_add(offset)),
|
||||
_ => None,
|
||||
GrepOutputMode::Content => Some(
|
||||
head_limit
|
||||
.saturating_add(offset)
|
||||
.min(MAX_GREP_MATCHES_SCANNED),
|
||||
),
|
||||
GrepOutputMode::FilesWithMatches | GrepOutputMode::CountMatches => {
|
||||
Some(MAX_GREP_MATCHES_SCANNED)
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
let lines = ctx
|
||||
.env
|
||||
.grep(pattern, path, &options)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
let lines = execute_grep(&ctx, pattern, path, &options).await?;
|
||||
|
||||
let searched = path;
|
||||
let mut results: Vec<String> = match mode {
|
||||
let results: Vec<String> = match mode {
|
||||
GrepOutputMode::Content => lines,
|
||||
GrepOutputMode::FilesWithMatches => {
|
||||
let mut seen: Vec<String> = Vec::new();
|
||||
for line in &lines {
|
||||
let file = grep_result_path(line, searched).to_string();
|
||||
if !seen.contains(&file) {
|
||||
seen.push(file);
|
||||
let mut seen = HashSet::new();
|
||||
let mut files = Vec::new();
|
||||
for line in lines {
|
||||
let file = grep_result_path(&line, searched).to_string();
|
||||
if seen.insert(file.clone()) {
|
||||
files.push(file);
|
||||
}
|
||||
}
|
||||
seen
|
||||
files
|
||||
}
|
||||
GrepOutputMode::Count => {
|
||||
let mut counts: Vec<(String, usize)> = Vec::new();
|
||||
for line in &lines {
|
||||
let file = grep_result_path(line, searched).to_string();
|
||||
match counts.iter_mut().find(|(name, _)| *name == file) {
|
||||
Some((_, count)) => *count += 1,
|
||||
None => counts.push((file, 1)),
|
||||
GrepOutputMode::CountMatches => {
|
||||
let mut counts: HashMap<String, usize> = HashMap::new();
|
||||
let mut order = Vec::new();
|
||||
for line in lines {
|
||||
let file = grep_result_path(&line, searched).to_string();
|
||||
if let Some(count) = counts.get_mut(&file) {
|
||||
*count += 1;
|
||||
} else {
|
||||
counts.insert(file.clone(), 1);
|
||||
order.push(file);
|
||||
}
|
||||
}
|
||||
counts
|
||||
order
|
||||
.into_iter()
|
||||
.map(|(file, count)| format!("{file}:{count}"))
|
||||
.map(|file| {
|
||||
let count = counts[&file];
|
||||
format!("{file}:{count}")
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
};
|
||||
|
||||
if offset > 0 {
|
||||
results = results.into_iter().skip(offset).collect();
|
||||
}
|
||||
if let Some(limit) = head_limit {
|
||||
results.truncate(limit);
|
||||
}
|
||||
.into_iter()
|
||||
.skip(offset)
|
||||
.take(head_limit)
|
||||
.collect();
|
||||
|
||||
if results.is_empty() {
|
||||
return Ok("No matches found".to_string());
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ pub use openai::OpenAiProfile;
|
|||
|
||||
use crate::agent_profile::AgentProfile;
|
||||
use crate::config::{NativeToolOptions, ToolSecrets};
|
||||
use crate::native_tool::{NativeTool, ToolVocabulary};
|
||||
use crate::native_tool::ToolVocabulary;
|
||||
use crate::sandbox::Sandbox;
|
||||
use crate::skills::{Skill, format_skills_prompt_section};
|
||||
use crate::tool_registry::ToolRegistry;
|
||||
|
|
@ -196,7 +196,7 @@ pub fn assemble_system_prompt(
|
|||
skills: &[Skill],
|
||||
) -> String {
|
||||
let env_block = build_env_context_block_with(env, env_context);
|
||||
let skill_tool = NativeTool::UseSkill.name(template.vocabulary);
|
||||
let vocabulary = template.vocabulary;
|
||||
let prompt = template.render(env_block);
|
||||
|
||||
let docs_section = if memory.is_empty() {
|
||||
|
|
@ -205,7 +205,7 @@ pub fn assemble_system_prompt(
|
|||
format!("\n\n{}", memory.join("\n\n"))
|
||||
};
|
||||
let skills_section = {
|
||||
let s = format_skills_prompt_section(skills, skill_tool);
|
||||
let s = format_skills_prompt_section(skills, vocabulary);
|
||||
if s.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ Apply the same care beyond git: weigh the reversibility and blast radius of any
|
|||
|
||||
If the codebase has tests or the ability to build or run, use them to verify your work. Start as specific as possible to the code you changed to catch issues efficiently, then widen to broader tests as you build confidence.
|
||||
|
||||
Long-running commands need a raised timeout rather than a retry. `Bash` takes a `timeout_ms` argument; use it for builds, test suites, and installs instead of letting the default elapse and trying again.
|
||||
Long-running commands need a raised timeout rather than a retry. `Bash` takes a `timeout` argument in seconds; use it for builds, test suites, and installs instead of letting the default elapse and trying again.
|
||||
|
||||
# Ultimate Reminders
|
||||
|
||||
|
|
|
|||
|
|
@ -587,6 +587,11 @@ mod tests {
|
|||
assert!(anthropic.get(ANTHROPIC_ASK_USER_QUESTION_TOOL).is_some());
|
||||
assert!(anthropic.get(OPENAI_REQUEST_USER_INPUT_TOOL).is_none());
|
||||
|
||||
let mut kimi = ToolRegistry::new();
|
||||
register_question_tools(AgentProfileKind::Kimi, &mut kimi);
|
||||
assert!(kimi.get(ANTHROPIC_ASK_USER_QUESTION_TOOL).is_some());
|
||||
assert!(kimi.get(OPENAI_REQUEST_USER_INPUT_TOOL).is_none());
|
||||
|
||||
let mut gemini = ToolRegistry::new();
|
||||
register_question_tools(AgentProfileKind::Gemini, &mut gemini);
|
||||
assert!(gemini.names().is_empty());
|
||||
|
|
|
|||
|
|
@ -38,14 +38,17 @@ use crate::file_tracker::FileTracker;
|
|||
use crate::history::History;
|
||||
use crate::loop_detection::detect_loop;
|
||||
use crate::memory::{BUDGET_BYTES, MemoryDocument, discover_memory};
|
||||
use crate::native_tool::NativeTool;
|
||||
use crate::profiles::EnvContext;
|
||||
use crate::question_tools::AgentToolRuntime;
|
||||
use crate::sandbox::Sandbox;
|
||||
use crate::skills::{
|
||||
ExpandedInput, Skill, default_skill_dirs, discover_skills, expand_skill, make_use_skill_tool,
|
||||
ExpandedInput, Skill, default_skill_dirs, discover_skills, expand_skill,
|
||||
make_use_skill_tool_for_vocabulary,
|
||||
};
|
||||
use crate::subagent::{SubAgentCallbackEvent, SubAgentEventCallback, SubAgentSupervisor};
|
||||
use crate::tool_execution::execute_tool_calls;
|
||||
use crate::tool_permissions::canonical_tool_name;
|
||||
use crate::tool_registry::ToolDefinitionWithSource;
|
||||
use crate::types::{
|
||||
AgentEvent, McpToolSummary, MemoryFileSummary, Message, SessionEvent, SessionState,
|
||||
|
|
@ -649,9 +652,10 @@ impl Session {
|
|||
if !self.skills.is_empty() {
|
||||
let skills_arc = Arc::new(self.skills.clone());
|
||||
if let Some(profile) = Arc::get_mut(&mut self.provider_profile) {
|
||||
let vocabulary = profile.tool_registry().vocabulary();
|
||||
profile
|
||||
.tool_registry_mut()
|
||||
.register(make_use_skill_tool(skills_arc));
|
||||
.register(make_use_skill_tool_for_vocabulary(skills_arc, vocabulary));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1866,10 +1870,10 @@ impl Session {
|
|||
.await;
|
||||
timing.tool = timing.tool.saturating_add(tool_start.elapsed());
|
||||
composite_watcher.abort();
|
||||
if tool_calls
|
||||
.iter()
|
||||
.any(|tool_call| tool_call.name == "use_skill")
|
||||
{
|
||||
if tool_calls.iter().zip(&results).any(|(tool_call, result)| {
|
||||
!result.is_error
|
||||
&& canonical_tool_name(&tool_call.name) == NativeTool::UseSkill.canonical_name()
|
||||
}) {
|
||||
self.activated_skill_context_observed = true;
|
||||
}
|
||||
|
||||
|
|
@ -2052,6 +2056,7 @@ impl Session {
|
|||
system_prompt: &self.system_prompt,
|
||||
memory: &self.memory,
|
||||
skills: &self.skills,
|
||||
tool_vocabulary: self.provider_profile.tool_registry().vocabulary(),
|
||||
activated_skill_context_observed: self.activated_skill_context_observed,
|
||||
provider: &provider,
|
||||
model: &model,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ use fabro_llm::types::ToolDefinition;
|
|||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::error::{Error, InterruptReason};
|
||||
use crate::native_tool::{NativeTool, ToolVocabulary};
|
||||
use crate::sandbox::Sandbox;
|
||||
use crate::tool_registry::{RegisteredTool, ToolSource};
|
||||
use crate::tools::required_str;
|
||||
|
|
@ -160,13 +161,21 @@ pub fn expand_skill(skills: &[Skill], input: &str) -> Result<ExpandedInput, Stri
|
|||
}
|
||||
|
||||
pub fn make_use_skill_tool(skills: Arc<Vec<Skill>>) -> RegisteredTool {
|
||||
RegisteredTool {
|
||||
definition: ToolDefinition {
|
||||
name: "use_skill".into(),
|
||||
description: "Load a skill's instructions by name. Call this when the user's \
|
||||
request matches an available skill."
|
||||
.into(),
|
||||
parameters: serde_json::json!({
|
||||
make_use_skill_tool_for_vocabulary(skills, ToolVocabulary::Fabro)
|
||||
}
|
||||
|
||||
/// Build the skill loader with the argument schema used by `vocabulary`.
|
||||
///
|
||||
/// Kimi Code calls the fields `skill` and `args`; fabro's native surface uses
|
||||
/// `skill_name`. The executor keeps one implementation for both.
|
||||
pub fn make_use_skill_tool_for_vocabulary(
|
||||
skills: Arc<Vec<Skill>>,
|
||||
vocabulary: ToolVocabulary,
|
||||
) -> RegisteredTool {
|
||||
let (name_parameter, parameters) = match vocabulary {
|
||||
ToolVocabulary::Fabro => (
|
||||
"skill_name",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"skill_name": {
|
||||
|
|
@ -176,11 +185,37 @@ pub fn make_use_skill_tool(skills: Arc<Vec<Skill>>) -> RegisteredTool {
|
|||
},
|
||||
"required": ["skill_name"]
|
||||
}),
|
||||
),
|
||||
ToolVocabulary::KimiCode => (
|
||||
"skill",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"skill": {
|
||||
"type": "string",
|
||||
"description": "Exact name of the skill to invoke"
|
||||
},
|
||||
"args": {
|
||||
"type": "string",
|
||||
"description": "Optional argument string to pass to the skill"
|
||||
}
|
||||
},
|
||||
"required": ["skill"]
|
||||
}),
|
||||
),
|
||||
};
|
||||
RegisteredTool {
|
||||
definition: ToolDefinition {
|
||||
name: NativeTool::UseSkill.canonical_name().into(),
|
||||
description: "Load a skill's instructions by name. Call this when the user's \
|
||||
request matches an available skill."
|
||||
.into(),
|
||||
parameters,
|
||||
},
|
||||
executor: Arc::new(move |args, ctx| {
|
||||
let skills = skills.clone();
|
||||
Box::pin(async move {
|
||||
let name = required_str(&args, "skill_name")?;
|
||||
let name = required_str(&args, name_parameter)?;
|
||||
let skill = skills
|
||||
.iter()
|
||||
.find(|s| s.name == name)
|
||||
|
|
@ -189,7 +224,15 @@ pub fn make_use_skill_tool(skills: Arc<Vec<Skill>>) -> RegisteredTool {
|
|||
skill_name: name.to_string(),
|
||||
source: SkillActivationSource::Tool,
|
||||
});
|
||||
Ok(skill.template.clone())
|
||||
let skill_args = args.get("args").and_then(serde_json::Value::as_str);
|
||||
let content = match skill_args.filter(|value| !value.is_empty()) {
|
||||
Some(value) if skill.template.contains("{{user_input}}") => {
|
||||
skill.template.replace("{{user_input}}", value)
|
||||
}
|
||||
Some(value) => format!("{}\n\nARGUMENTS:\n{value}", skill.template),
|
||||
None => skill.template.clone(),
|
||||
};
|
||||
Ok(content)
|
||||
})
|
||||
}),
|
||||
source: ToolSource::Skill,
|
||||
|
|
@ -197,15 +240,12 @@ pub fn make_use_skill_tool(skills: Arc<Vec<Skill>>) -> RegisteredTool {
|
|||
}
|
||||
|
||||
/// Render the skills section of a system prompt.
|
||||
///
|
||||
/// `skill_tool` is the name the skill tool is exposed under, which depends on
|
||||
/// the profile's vocabulary — telling a model to call a tool it was not given
|
||||
/// is worse than omitting the guidance.
|
||||
pub fn format_skills_prompt_section(skills: &[Skill], skill_tool: &str) -> String {
|
||||
pub fn format_skills_prompt_section(skills: &[Skill], vocabulary: ToolVocabulary) -> String {
|
||||
if skills.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
|
||||
let skill_tool = NativeTool::UseSkill.name(vocabulary);
|
||||
let mut lines = vec![
|
||||
"# Available Skills".to_string(),
|
||||
format!(
|
||||
|
|
@ -458,13 +498,13 @@ name: trimmed
|
|||
|
||||
#[test]
|
||||
fn format_empty() {
|
||||
assert_eq!(format_skills_prompt_section(&[], "use_skill"), "");
|
||||
assert_eq!(format_skills_prompt_section(&[], ToolVocabulary::Fabro), "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_lists_skills() {
|
||||
let skills = test_skills();
|
||||
let section = format_skills_prompt_section(&skills, "use_skill");
|
||||
let section = format_skills_prompt_section(&skills, ToolVocabulary::Fabro);
|
||||
assert!(section.contains("# Available Skills"));
|
||||
assert!(section.contains("call the `use_skill` tool"));
|
||||
assert!(section.contains("- `commit`: Create a commit"));
|
||||
|
|
@ -647,4 +687,44 @@ name: trimmed
|
|||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().contains("Missing required parameter"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn kimi_skill_schema_and_args_match_kimi_code() {
|
||||
let skills = Arc::new(test_skills());
|
||||
let tool = make_use_skill_tool_for_vocabulary(skills, ToolVocabulary::KimiCode);
|
||||
let env: Arc<dyn Sandbox> = Arc::new(MockSandbox::default());
|
||||
let ctx = ToolContext {
|
||||
env,
|
||||
cancel: CancellationToken::new(),
|
||||
tool_env_provider: None,
|
||||
session_id: None,
|
||||
root_session_id: None,
|
||||
tool_call_id: None,
|
||||
agent_event_emitter: None,
|
||||
};
|
||||
|
||||
let result = (tool.executor)(
|
||||
serde_json::json!({"skill": "commit", "args": "only staged files"}),
|
||||
ctx,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(result.contains("only staged files"), "{result}");
|
||||
assert!(
|
||||
tool.definition.parameters["properties"]
|
||||
.get("skill")
|
||||
.is_some()
|
||||
);
|
||||
assert!(
|
||||
tool.definition.parameters["properties"]
|
||||
.get("args")
|
||||
.is_some()
|
||||
);
|
||||
assert!(
|
||||
tool.definition.parameters["properties"]
|
||||
.get("skill_name")
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ use futures::stream;
|
|||
|
||||
use crate::agent_profile::AgentProfile;
|
||||
use crate::config::SessionOptions;
|
||||
use crate::native_tool::NativeTool;
|
||||
use crate::native_tool::ToolVocabulary;
|
||||
use crate::profiles::EnvContext;
|
||||
use crate::sandbox::*;
|
||||
use crate::session::Session;
|
||||
|
|
@ -81,8 +81,7 @@ impl AgentProfile for TestProfile {
|
|||
user_instructions: Option<&str>,
|
||||
skills: &[Skill],
|
||||
) -> String {
|
||||
let skills_section =
|
||||
format_skills_prompt_section(skills, NativeTool::UseSkill.canonical_name());
|
||||
let skills_section = format_skills_prompt_section(skills, ToolVocabulary::Fabro);
|
||||
let skills_part = if skills_section.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
//! Model-facing todo / task tools.
|
||||
//!
|
||||
//! Two surfaces share one engine ([`TodoRuntime`]):
|
||||
//! Three surfaces share one engine ([`TodoRuntime`]):
|
||||
//!
|
||||
//! - [`make_update_plan_tool`] — Codex-compatible OpenAI `update_plan`.
|
||||
//! - [`make_todo_list_tool`] — Kimi Code-compatible whole-list `TodoList`.
|
||||
//! - [`make_task_create_tool`] / [`make_task_update_tool`] /
|
||||
//! [`make_task_get_tool`] / [`make_task_list_tool`] — Claude task tools.
|
||||
|
||||
|
|
@ -15,17 +16,22 @@ use std::sync::{Arc, Mutex};
|
|||
use fabro_llm::types::ToolDefinition;
|
||||
use fabro_types::{TodoListKind, TodoProjection, TodoStatus, TodoUpdatedProps};
|
||||
use serde_json::Value;
|
||||
use strum::{EnumString, IntoStaticStr};
|
||||
|
||||
use crate::todo_runtime::TodoRuntime;
|
||||
use crate::tool_registry::{RegisteredTool, ToolContext, ToolSource};
|
||||
|
||||
/// Compute the OpenAI plan scope (`openai_plan:<session_id>`). Returns an
|
||||
/// error string the model can see if no session ID is bound to the call.
|
||||
fn openai_plan_scope(ctx: &ToolContext) -> Result<String, String> {
|
||||
/// Compute a session-scoped todo-list ID. Returns an error the model can see
|
||||
/// when a tool is invoked without an active session.
|
||||
fn session_todo_scope(
|
||||
ctx: &ToolContext,
|
||||
kind: TodoListKind,
|
||||
tool_name: &str,
|
||||
) -> Result<String, String> {
|
||||
ctx.session_id
|
||||
.as_ref()
|
||||
.map(|sid| TodoListKind::OpenAiPlan.list_id(sid))
|
||||
.ok_or_else(|| "update_plan requires an active session".to_string())
|
||||
.map(|session_id| kind.list_id(session_id))
|
||||
.ok_or_else(|| format!("{tool_name} requires an active session"))
|
||||
}
|
||||
|
||||
/// Compute the Anthropic task scope
|
||||
|
|
@ -72,15 +78,14 @@ description and dependency details.";
|
|||
const TASK_GET_DESCRIPTION: &str = "Get one task by taskId, including subject, status, \
|
||||
description, owner, blockedBy, and blocks.";
|
||||
|
||||
/// Deterministic todo id derived from `<list_id>::<step>`. Codex identifies
|
||||
/// a plan step by the exact step text, so the projection ID is the
|
||||
/// `sha256(list_id, step)` truncated for compactness.
|
||||
fn openai_step_id(list_id: &str, step: &str) -> String {
|
||||
/// Deterministic todo id derived from `<list_id>::<text>`. Whole-list tools
|
||||
/// identify an item by its exact text, so unchanged entries preserve identity.
|
||||
fn todo_text_id(list_id: &str, text: &str) -> String {
|
||||
use sha2::{Digest, Sha256};
|
||||
let mut hasher = Sha256::new();
|
||||
hasher.update(list_id.as_bytes());
|
||||
hasher.update(b"\x00");
|
||||
hasher.update(step.as_bytes());
|
||||
hasher.update(text.as_bytes());
|
||||
let digest = hasher.finalize();
|
||||
let mut out = String::with_capacity(16);
|
||||
for byte in &digest[..8] {
|
||||
|
|
@ -89,6 +94,60 @@ fn openai_step_id(list_id: &str, step: &str) -> String {
|
|||
out
|
||||
}
|
||||
|
||||
struct ReplacementTodo {
|
||||
id: String,
|
||||
subject: String,
|
||||
status: TodoStatus,
|
||||
}
|
||||
|
||||
fn reconcile_replacement_list(
|
||||
runtime: &TodoRuntime,
|
||||
ctx: &ToolContext,
|
||||
kind: TodoListKind,
|
||||
list_id: &str,
|
||||
incoming: &[ReplacementTodo],
|
||||
) {
|
||||
let previous = runtime
|
||||
.snapshot(list_id)
|
||||
.map(|list| list.items)
|
||||
.unwrap_or_default();
|
||||
let previous_by_id: HashMap<&str, &TodoProjection> = previous
|
||||
.iter()
|
||||
.map(|todo| (todo.id.as_str(), todo))
|
||||
.collect();
|
||||
let incoming_ids: HashSet<&str> = incoming.iter().map(|todo| todo.id.as_str()).collect();
|
||||
|
||||
for todo in &previous {
|
||||
if !incoming_ids.contains(todo.id.as_str()) {
|
||||
runtime.delete(ctx, kind, list_id.to_string(), todo.id.clone());
|
||||
}
|
||||
}
|
||||
|
||||
for (index, todo) in incoming.iter().enumerate() {
|
||||
let order = u32::try_from(index).unwrap_or(u32::MAX);
|
||||
match previous_by_id.get(todo.id.as_str()) {
|
||||
Some(previous)
|
||||
if previous.status == todo.status
|
||||
&& previous.order == order
|
||||
&& previous.subject == todo.subject => {}
|
||||
Some(_) => {
|
||||
runtime.update(ctx, TodoUpdatedProps {
|
||||
status: Some(todo.status),
|
||||
order: Some(order),
|
||||
subject: Some(todo.subject.clone()),
|
||||
..TodoUpdatedProps::new(list_id, kind, &todo.id)
|
||||
});
|
||||
}
|
||||
None => {
|
||||
let mut projection =
|
||||
TodoProjection::new(todo.id.clone(), order, todo.subject.clone());
|
||||
projection.status = todo.status;
|
||||
runtime.create(ctx, kind, list_id.to_string(), projection);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// OpenAI `update_plan` tool. See plan summary for semantics.
|
||||
#[must_use]
|
||||
pub fn make_update_plan_tool(runtime: Arc<TodoRuntime>) -> RegisteredTool {
|
||||
|
|
@ -127,15 +186,14 @@ pub fn make_update_plan_tool(runtime: Arc<TodoRuntime>) -> RegisteredTool {
|
|||
executor: Arc::new(move |args, ctx| {
|
||||
let runtime = runtime.clone();
|
||||
Box::pin(async move {
|
||||
let list_id = openai_plan_scope(&ctx)?;
|
||||
let list_id = session_todo_scope(&ctx, TodoListKind::OpenAiPlan, "update_plan")?;
|
||||
let plan = args
|
||||
.get("plan")
|
||||
.and_then(Value::as_array)
|
||||
.ok_or_else(|| "Missing required parameter: plan".to_string())?;
|
||||
|
||||
// Parse incoming steps, precompute ids, and enforce step-text uniqueness.
|
||||
let mut incoming: Vec<(String, String, TodoStatus)> =
|
||||
Vec::with_capacity(plan.len());
|
||||
let mut incoming = Vec::with_capacity(plan.len());
|
||||
let mut seen_steps: HashSet<&str> = HashSet::with_capacity(plan.len());
|
||||
for (index, entry) in plan.iter().enumerate() {
|
||||
let step = entry
|
||||
|
|
@ -152,57 +210,20 @@ pub fn make_update_plan_tool(runtime: Arc<TodoRuntime>) -> RegisteredTool {
|
|||
"Duplicate plan step `{step}` — step text must be unique"
|
||||
));
|
||||
}
|
||||
let todo_id = openai_step_id(&list_id, step);
|
||||
incoming.push((todo_id, step.to_string(), status));
|
||||
incoming.push(ReplacementTodo {
|
||||
id: todo_text_id(&list_id, step),
|
||||
subject: step.to_string(),
|
||||
status,
|
||||
});
|
||||
}
|
||||
|
||||
// Snapshot previous state into a HashMap for O(1) lookup.
|
||||
let previous: HashMap<String, TodoProjection> = runtime
|
||||
.snapshot(&list_id)
|
||||
.map(|list| list.items.into_iter().map(|t| (t.id.clone(), t)).collect())
|
||||
.unwrap_or_default();
|
||||
let incoming_ids: HashSet<&str> =
|
||||
incoming.iter().map(|(id, _, _)| id.as_str()).collect();
|
||||
|
||||
// Deletes: anything in previous but not in incoming.
|
||||
for id in previous.keys() {
|
||||
if !incoming_ids.contains(id.as_str()) {
|
||||
runtime.delete(&ctx, TodoListKind::OpenAiPlan, list_id.clone(), id.clone());
|
||||
}
|
||||
}
|
||||
|
||||
// Upserts: each incoming step becomes a create (new) or update.
|
||||
for (index, (todo_id, step, status)) in incoming.iter().enumerate() {
|
||||
let order = u32::try_from(index).unwrap_or(u32::MAX);
|
||||
match previous.get(todo_id) {
|
||||
Some(prev)
|
||||
if prev.status == *status
|
||||
&& prev.order == order
|
||||
&& prev.subject == *step =>
|
||||
{
|
||||
// No change.
|
||||
}
|
||||
Some(_) => {
|
||||
runtime.update(&ctx, TodoUpdatedProps {
|
||||
status: Some(*status),
|
||||
order: Some(order),
|
||||
subject: Some(step.clone()),
|
||||
..TodoUpdatedProps::new(&list_id, TodoListKind::OpenAiPlan, todo_id)
|
||||
});
|
||||
}
|
||||
None => {
|
||||
let mut projection =
|
||||
TodoProjection::new(todo_id.clone(), order, step.clone());
|
||||
projection.status = *status;
|
||||
runtime.create(
|
||||
&ctx,
|
||||
TodoListKind::OpenAiPlan,
|
||||
list_id.clone(),
|
||||
projection,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
reconcile_replacement_list(
|
||||
&runtime,
|
||||
&ctx,
|
||||
TodoListKind::OpenAiPlan,
|
||||
&list_id,
|
||||
&incoming,
|
||||
);
|
||||
|
||||
Ok("Plan updated".to_string())
|
||||
})
|
||||
|
|
@ -211,42 +232,56 @@ pub fn make_update_plan_tool(runtime: Arc<TodoRuntime>) -> RegisteredTool {
|
|||
}
|
||||
}
|
||||
|
||||
/// Compute the Kimi todo scope (`kimi_todos:<session_id>`).
|
||||
fn kimi_todo_scope(ctx: &ToolContext) -> Result<String, String> {
|
||||
ctx.session_id
|
||||
.as_ref()
|
||||
.map(|sid| TodoListKind::KimiTodos.list_id(sid))
|
||||
.ok_or_else(|| "TodoList requires an active session".to_string())
|
||||
#[derive(Clone, Copy, EnumString, IntoStaticStr)]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
enum KimiTodoStatus {
|
||||
Pending,
|
||||
InProgress,
|
||||
#[strum(to_string = "done")]
|
||||
Done,
|
||||
}
|
||||
|
||||
impl From<KimiTodoStatus> for TodoStatus {
|
||||
fn from(status: KimiTodoStatus) -> Self {
|
||||
match status {
|
||||
KimiTodoStatus::Pending => Self::Pending,
|
||||
KimiTodoStatus::InProgress => Self::InProgress,
|
||||
KimiTodoStatus::Done => Self::Completed,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<TodoStatus> for KimiTodoStatus {
|
||||
fn from(status: TodoStatus) -> Self {
|
||||
match status {
|
||||
TodoStatus::Pending => Self::Pending,
|
||||
TodoStatus::InProgress => Self::InProgress,
|
||||
TodoStatus::Completed | TodoStatus::Deleted => Self::Done,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Kimi Code spells the terminal status `done`; internally it is
|
||||
/// [`TodoStatus::Completed`].
|
||||
fn parse_kimi_status(value: &str) -> Result<TodoStatus, String> {
|
||||
match value {
|
||||
"pending" => Ok(TodoStatus::Pending),
|
||||
"in_progress" => Ok(TodoStatus::InProgress),
|
||||
"done" => Ok(TodoStatus::Completed),
|
||||
other => Err(format!(
|
||||
"Invalid status `{other}` (expected pending|in_progress|done)"
|
||||
)),
|
||||
}
|
||||
value
|
||||
.parse::<KimiTodoStatus>()
|
||||
.map(TodoStatus::from)
|
||||
.map_err(|_| format!("Invalid status `{value}` (expected pending|in_progress|done)"))
|
||||
}
|
||||
|
||||
fn kimi_status_name(status: TodoStatus) -> &'static str {
|
||||
match status {
|
||||
TodoStatus::Pending => "pending",
|
||||
TodoStatus::InProgress => "in_progress",
|
||||
TodoStatus::Completed | TodoStatus::Deleted => "done",
|
||||
}
|
||||
KimiTodoStatus::from(status).into()
|
||||
}
|
||||
|
||||
fn render_kimi_todos(items: &[TodoProjection]) -> String {
|
||||
if items.is_empty() {
|
||||
fn render_kimi_todos<'a>(items: impl IntoIterator<Item = (TodoStatus, &'a str)>) -> String {
|
||||
let mut items = items.into_iter().peekable();
|
||||
if items.peek().is_none() {
|
||||
return "The todo list is empty.".to_string();
|
||||
}
|
||||
let mut out = String::new();
|
||||
for todo in items {
|
||||
let _ = writeln!(out, "[{}] {}", kimi_status_name(todo.status), todo.subject);
|
||||
for (status, subject) in items {
|
||||
let _ = writeln!(out, "[{}] {subject}", kimi_status_name(status));
|
||||
}
|
||||
out.truncate(out.trim_end().len());
|
||||
out
|
||||
|
|
@ -302,7 +337,7 @@ pub fn make_todo_list_tool(runtime: Arc<TodoRuntime>) -> RegisteredTool {
|
|||
executor: Arc::new(move |args, ctx| {
|
||||
let runtime = runtime.clone();
|
||||
Box::pin(async move {
|
||||
let list_id = kimi_todo_scope(&ctx)?;
|
||||
let list_id = session_todo_scope(&ctx, TodoListKind::KimiTodos, "TodoList")?;
|
||||
|
||||
// Read mode: `todos` omitted entirely.
|
||||
let Some(todos) = args.get("todos") else {
|
||||
|
|
@ -310,14 +345,17 @@ pub fn make_todo_list_tool(runtime: Arc<TodoRuntime>) -> RegisteredTool {
|
|||
.snapshot(&list_id)
|
||||
.map(|l| l.items)
|
||||
.unwrap_or_default();
|
||||
return Ok(render_kimi_todos(&items));
|
||||
return Ok(render_kimi_todos(
|
||||
items
|
||||
.iter()
|
||||
.map(|todo| (todo.status, todo.subject.as_str())),
|
||||
));
|
||||
};
|
||||
let todos = todos
|
||||
.as_array()
|
||||
.ok_or_else(|| "`todos` must be an array".to_string())?;
|
||||
|
||||
let mut incoming: Vec<(String, String, TodoStatus)> =
|
||||
Vec::with_capacity(todos.len());
|
||||
let mut incoming = Vec::with_capacity(todos.len());
|
||||
let mut seen: HashSet<&str> = HashSet::with_capacity(todos.len());
|
||||
for (index, entry) in todos.iter().enumerate() {
|
||||
let title = entry
|
||||
|
|
@ -332,56 +370,25 @@ pub fn make_todo_list_tool(runtime: Arc<TodoRuntime>) -> RegisteredTool {
|
|||
if !seen.insert(title) {
|
||||
return Err(format!("Duplicate todo `{title}` — titles must be unique"));
|
||||
}
|
||||
incoming.push((openai_step_id(&list_id, title), title.to_string(), status));
|
||||
incoming.push(ReplacementTodo {
|
||||
id: todo_text_id(&list_id, title),
|
||||
subject: title.to_string(),
|
||||
status,
|
||||
});
|
||||
}
|
||||
|
||||
let previous: HashMap<String, TodoProjection> = runtime
|
||||
.snapshot(&list_id)
|
||||
.map(|list| list.items.into_iter().map(|t| (t.id.clone(), t)).collect())
|
||||
.unwrap_or_default();
|
||||
let incoming_ids: HashSet<&str> =
|
||||
incoming.iter().map(|(id, _, _)| id.as_str()).collect();
|
||||
|
||||
for id in previous.keys() {
|
||||
if !incoming_ids.contains(id.as_str()) {
|
||||
runtime.delete(&ctx, TodoListKind::KimiTodos, list_id.clone(), id.clone());
|
||||
}
|
||||
}
|
||||
|
||||
for (index, (todo_id, title, status)) in incoming.iter().enumerate() {
|
||||
let order = u32::try_from(index).unwrap_or(u32::MAX);
|
||||
match previous.get(todo_id) {
|
||||
Some(prev)
|
||||
if prev.status == *status
|
||||
&& prev.order == order
|
||||
&& prev.subject == *title => {}
|
||||
Some(_) => {
|
||||
runtime.update(&ctx, TodoUpdatedProps {
|
||||
status: Some(*status),
|
||||
order: Some(order),
|
||||
subject: Some(title.clone()),
|
||||
..TodoUpdatedProps::new(&list_id, TodoListKind::KimiTodos, todo_id)
|
||||
});
|
||||
}
|
||||
None => {
|
||||
let mut projection =
|
||||
TodoProjection::new(todo_id.clone(), order, title.clone());
|
||||
projection.status = *status;
|
||||
runtime.create(
|
||||
&ctx,
|
||||
TodoListKind::KimiTodos,
|
||||
list_id.clone(),
|
||||
projection,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let items = runtime
|
||||
.snapshot(&list_id)
|
||||
.map(|l| l.items)
|
||||
.unwrap_or_default();
|
||||
Ok(render_kimi_todos(&items))
|
||||
reconcile_replacement_list(
|
||||
&runtime,
|
||||
&ctx,
|
||||
TodoListKind::KimiTodos,
|
||||
&list_id,
|
||||
&incoming,
|
||||
);
|
||||
Ok(render_kimi_todos(
|
||||
incoming
|
||||
.iter()
|
||||
.map(|todo| (todo.status, todo.subject.as_str())),
|
||||
))
|
||||
})
|
||||
}),
|
||||
source: ToolSource::Native,
|
||||
|
|
|
|||
|
|
@ -156,7 +156,14 @@ impl ToolRegistry {
|
|||
}
|
||||
|
||||
pub fn register(&mut self, mut tool: RegisteredTool) {
|
||||
if let Some(native) = NativeTool::from_any_name(&tool.definition.name) {
|
||||
let native = match &tool.source {
|
||||
ToolSource::Native => NativeTool::from_canonical_name(&tool.definition.name),
|
||||
ToolSource::Skill if tool.definition.name == NativeTool::UseSkill.canonical_name() => {
|
||||
Some(NativeTool::UseSkill)
|
||||
}
|
||||
ToolSource::Skill | ToolSource::Mcp { .. } => None,
|
||||
};
|
||||
if let Some(native) = native {
|
||||
tool.definition.name = native.name(self.vocabulary).to_string();
|
||||
}
|
||||
self.tools.insert(tool.definition.name.clone(), tool);
|
||||
|
|
@ -168,9 +175,8 @@ impl ToolRegistry {
|
|||
/// identity rather than by whatever string it is currently exposed under.
|
||||
pub fn redescribe(&mut self, tool: NativeTool, description: impl Into<String>) {
|
||||
let exposed = tool.name(self.vocabulary);
|
||||
if let Some(mut registered) = self.tools.remove(exposed) {
|
||||
if let Some(registered) = self.tools.get_mut(exposed) {
|
||||
registered.definition.description = description.into();
|
||||
self.tools.insert(exposed.to_string(), registered);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -298,6 +304,31 @@ mod tests {
|
|||
assert_eq!(tool.unwrap().definition.name, "read_file");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kimi_registry_renames_canonical_native_tools_only() {
|
||||
let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::KimiCode);
|
||||
registry.register(make_tool("read_file"));
|
||||
registry.register(make_tool("Read"));
|
||||
|
||||
assert!(registry.get("Read").is_some());
|
||||
assert!(registry.get("read_file").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn registry_does_not_reinterpret_mcp_names_as_native_tools() {
|
||||
let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::KimiCode);
|
||||
let mut tool = make_tool("read_file");
|
||||
tool.source = ToolSource::Mcp {
|
||||
server_name: "files".to_string(),
|
||||
original_name: "read_file".to_string(),
|
||||
};
|
||||
|
||||
registry.register(tool);
|
||||
|
||||
assert!(registry.get("read_file").is_some());
|
||||
assert!(registry.get("Read").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_missing_returns_none() {
|
||||
let registry = ToolRegistry::new();
|
||||
|
|
|
|||
|
|
@ -10,11 +10,12 @@ use fabro_static::EnvVars;
|
|||
use futures::{StreamExt, stream};
|
||||
|
||||
use crate::config::NativeToolOptions;
|
||||
use crate::sandbox::GrepOptions;
|
||||
use crate::tool_registry::{RegisteredTool, ToolRegistry, ToolSource};
|
||||
use crate::sandbox::{ExecResult, GrepOptions};
|
||||
use crate::tool_registry::{RegisteredTool, ToolContext, ToolRegistry, ToolSource};
|
||||
|
||||
const MAX_WEB_FETCH_BYTES: usize = 100 * 1024;
|
||||
const MAX_READ_MANY_FILES_CONCURRENCY: usize = 8;
|
||||
pub(crate) const DEFAULT_READ_LINES: usize = 2000;
|
||||
|
||||
/// Configuration for the optional LLM-based summarizer used by `web_fetch`.
|
||||
#[derive(Clone)]
|
||||
|
|
@ -65,6 +66,15 @@ pub fn register_core_tools(
|
|||
registry.register(make_write_file_tool());
|
||||
registry.register(make_shell_tool_with_options(options));
|
||||
registry.register(make_grep_tool());
|
||||
register_discovery_and_web_tools(registry, options, summarizer);
|
||||
}
|
||||
|
||||
/// Register the core tools whose Kimi Code contracts match fabro's own.
|
||||
pub(crate) fn register_discovery_and_web_tools(
|
||||
registry: &mut ToolRegistry,
|
||||
options: &NativeToolOptions,
|
||||
summarizer: Option<WebFetchSummarizer>,
|
||||
) {
|
||||
registry.register(make_glob_tool());
|
||||
if let Some(api_key) = &options.secrets.brave_search_api_key {
|
||||
registry.register(make_web_search_tool_with_api_key(api_key.clone()));
|
||||
|
|
@ -110,7 +120,8 @@ pub fn make_read_file_tool() -> RegisteredTool {
|
|||
Box::pin(async move {
|
||||
let file_path = required_str(&args, "file_path")?;
|
||||
let offset_usize = optional_usize_arg(&args, "offset")?;
|
||||
let limit_usize = optional_usize_arg(&args, "limit")?;
|
||||
let limit_usize =
|
||||
optional_usize_arg(&args, "limit")?.or(Some(DEFAULT_READ_LINES));
|
||||
|
||||
let content = ctx
|
||||
.env
|
||||
|
|
@ -242,29 +253,13 @@ pub fn make_shell_tool_with_options(options: &NativeToolOptions) -> RegisteredTo
|
|||
executor: Arc::new(move |args, ctx| {
|
||||
Box::pin(async move {
|
||||
let command = required_str(&args, "command")?;
|
||||
let command = format!("exec 2>&1\n{command}");
|
||||
let timeout_ms = args
|
||||
.get("timeout_ms")
|
||||
.and_then(serde_json::Value::as_u64)
|
||||
.unwrap_or(default_timeout)
|
||||
.min(max_timeout);
|
||||
|
||||
let tool_env = ctx.resolve_tool_env().await.map_err(|e| format!("{e:#}"))?;
|
||||
tracing::debug!(
|
||||
env_var_count = tool_env.as_ref().map_or(0, std::collections::HashMap::len),
|
||||
"Injecting sandbox env vars into tool execution"
|
||||
);
|
||||
let result = ctx
|
||||
.env
|
||||
.exec_command(
|
||||
&command,
|
||||
timeout_ms,
|
||||
None,
|
||||
tool_env.as_ref(),
|
||||
Some(ctx.cancel),
|
||||
)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
let result = execute_shell_command(&ctx, command, timeout_ms, None).await?;
|
||||
|
||||
let mut output = String::new();
|
||||
if result.is_timed_out() {
|
||||
|
|
@ -287,6 +282,33 @@ pub fn make_shell_tool_with_options(options: &NativeToolOptions) -> RegisteredTo
|
|||
}
|
||||
}
|
||||
|
||||
/// Execute a shell command with the session's environment and cancellation
|
||||
/// plumbing. Provider profiles can vary their wire schema and result
|
||||
/// rendering without accidentally bypassing those shared semantics.
|
||||
pub(crate) async fn execute_shell_command(
|
||||
ctx: &ToolContext,
|
||||
command: &str,
|
||||
timeout_ms: u64,
|
||||
cwd: Option<&str>,
|
||||
) -> Result<ExecResult, String> {
|
||||
let command = format!("exec 2>&1\n{command}");
|
||||
let tool_env = ctx.resolve_tool_env().await.map_err(|e| format!("{e:#}"))?;
|
||||
tracing::debug!(
|
||||
env_var_count = tool_env.as_ref().map_or(0, std::collections::HashMap::len),
|
||||
"Injecting sandbox env vars into tool execution"
|
||||
);
|
||||
ctx.env
|
||||
.exec_command(
|
||||
&command,
|
||||
timeout_ms,
|
||||
cwd,
|
||||
tool_env.as_ref(),
|
||||
Some(ctx.cancel.clone()),
|
||||
)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn make_grep_tool() -> RegisteredTool {
|
||||
RegisteredTool {
|
||||
|
|
@ -332,19 +354,7 @@ pub fn make_grep_tool() -> RegisteredTool {
|
|||
max_results,
|
||||
};
|
||||
|
||||
let results = ctx
|
||||
.env
|
||||
.grep(pattern, path, &options)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
let mut seen_files = std::collections::HashSet::new();
|
||||
for line in &results {
|
||||
if let Some(file_path) = line.split(':').next() {
|
||||
if !file_path.is_empty() && seen_files.insert(file_path) {
|
||||
ctx.env.mark_agent_read(file_path);
|
||||
}
|
||||
}
|
||||
}
|
||||
let results = execute_grep(&ctx, pattern, path, &options).await?;
|
||||
Ok(results.join("\n"))
|
||||
})
|
||||
}),
|
||||
|
|
@ -352,6 +362,49 @@ pub fn make_grep_tool() -> RegisteredTool {
|
|||
}
|
||||
}
|
||||
|
||||
/// Run a content search and mark every returned file as observed by the
|
||||
/// agent's read-before-write guard.
|
||||
pub(crate) async fn execute_grep(
|
||||
ctx: &ToolContext,
|
||||
pattern: &str,
|
||||
path: &str,
|
||||
options: &GrepOptions,
|
||||
) -> Result<Vec<String>, String> {
|
||||
let results = ctx
|
||||
.env
|
||||
.grep(pattern, path, options)
|
||||
.await
|
||||
.map_err(|e| e.display_with_causes())?;
|
||||
let mut seen_files = std::collections::HashSet::new();
|
||||
for line in &results {
|
||||
let file_path = grep_result_path(line, path);
|
||||
if !file_path.is_empty() && seen_files.insert(file_path) {
|
||||
ctx.env.mark_agent_read(file_path);
|
||||
}
|
||||
}
|
||||
Ok(results)
|
||||
}
|
||||
|
||||
/// Extract the file path from `<path>:<line>:<content>` grep output.
|
||||
///
|
||||
/// A search of one concrete file may omit `<path>`, in which case the searched
|
||||
/// path itself is returned. Candidate separators are walked so paths that
|
||||
/// contain colons (including Windows drive prefixes) still parse correctly.
|
||||
pub(crate) fn grep_result_path<'a>(line: &'a str, searched: &'a str) -> &'a str {
|
||||
let mut rest = line;
|
||||
let mut consumed = 0usize;
|
||||
while let Some(index) = rest.find(':') {
|
||||
let after = &rest[index + 1..];
|
||||
let digit_count = after.chars().take_while(char::is_ascii_digit).count();
|
||||
if digit_count > 0 && after[digit_count..].starts_with(':') {
|
||||
return &line[..consumed + index];
|
||||
}
|
||||
consumed += index + 1;
|
||||
rest = after;
|
||||
}
|
||||
searched
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn make_glob_tool() -> RegisteredTool {
|
||||
RegisteredTool {
|
||||
|
|
@ -774,6 +827,34 @@ mod tests {
|
|||
assert_eq!(result.unwrap(), "1 | hello\n2 | world\n");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_file_applies_the_documented_default_limit() {
|
||||
let tool = make_read_file_tool();
|
||||
let content = (1..=DEFAULT_READ_LINES + 1)
|
||||
.map(|line| format!("line{line}"))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
let env: Arc<dyn Sandbox> = Arc::new(MockSandbox {
|
||||
files: HashMap::from([("/test.txt".to_string(), content)]),
|
||||
..Default::default()
|
||||
});
|
||||
|
||||
let result = (tool.executor)(serde_json::json!({"file_path": "/test.txt"}), ToolContext {
|
||||
env,
|
||||
cancel: CancellationToken::new(),
|
||||
tool_env_provider: None,
|
||||
session_id: None,
|
||||
root_session_id: None,
|
||||
tool_call_id: None,
|
||||
agent_event_emitter: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(result.contains("2000 | line2000"), "{result}");
|
||||
assert!(!result.contains("2001 | line2001"), "{result}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_file_with_offset_and_limit() {
|
||||
let tool = make_read_file_tool();
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
use crate::config::SessionOptions;
|
||||
use crate::tool_permissions::canonical_tool_name;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum TruncationMode {
|
||||
|
|
@ -86,14 +87,16 @@ pub fn truncate_lines(output: &str, max_lines: usize) -> String {
|
|||
|
||||
#[must_use]
|
||||
pub fn truncate_tool_output(output: &str, tool_name: &str, config: &SessionOptions) -> String {
|
||||
let mode = default_truncation_mode(tool_name);
|
||||
let canonical_name = canonical_tool_name(tool_name);
|
||||
let mode = default_truncation_mode(canonical_name);
|
||||
|
||||
// Char truncation first
|
||||
let char_limit = config
|
||||
.tool_output_limits
|
||||
.get(tool_name)
|
||||
.copied()
|
||||
.or_else(|| default_char_limit(tool_name));
|
||||
.or_else(|| config.tool_output_limits.get(canonical_name).copied())
|
||||
.or_else(|| default_char_limit(canonical_name));
|
||||
|
||||
let after_chars = match char_limit {
|
||||
Some(limit) => truncate_output(output, limit, mode),
|
||||
|
|
@ -105,7 +108,8 @@ pub fn truncate_tool_output(output: &str, tool_name: &str, config: &SessionOptio
|
|||
.tool_line_limits
|
||||
.get(tool_name)
|
||||
.copied()
|
||||
.or_else(|| default_line_limit(tool_name));
|
||||
.or_else(|| config.tool_line_limits.get(canonical_name).copied())
|
||||
.or_else(|| default_line_limit(canonical_name));
|
||||
|
||||
match line_limit {
|
||||
Some(limit) => truncate_lines(&after_chars, limit),
|
||||
|
|
@ -172,6 +176,24 @@ mod tests {
|
|||
assert!(result.len() < output.len());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kimi_aliases_use_canonical_limits() {
|
||||
let config = SessionOptions::default();
|
||||
let shell_output = "x".repeat(40_000);
|
||||
let write_output = "x".repeat(2_000);
|
||||
|
||||
assert!(truncate_tool_output(&shell_output, "Bash", &config).len() < shell_output.len());
|
||||
assert!(truncate_tool_output(&write_output, "Write", &config).len() < write_output.len());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn canonical_config_override_applies_to_kimi_alias() {
|
||||
let mut config = SessionOptions::default();
|
||||
config.tool_output_limits.insert("shell".into(), 100);
|
||||
let result = truncate_tool_output(&"x".repeat(1_000), "Bash", &config);
|
||||
assert!(result.contains("Tool output was truncated"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_override_char_limit() {
|
||||
let output = "x".repeat(5000);
|
||||
|
|
|
|||
|
|
@ -5260,35 +5260,34 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn child_openai_plan_does_not_project_when_root_has_no_plan() {
|
||||
let mut state = initialized_projection();
|
||||
let stage_id = stage_id();
|
||||
state
|
||||
.apply_event(&test_stage_event(
|
||||
1,
|
||||
EventBody::StageStarted(started_props()),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
state
|
||||
.apply_event(&child_stage_event(
|
||||
2,
|
||||
created(
|
||||
"openai_plan:child_session",
|
||||
TodoListKind::OpenAiPlan,
|
||||
"c-a",
|
||||
0,
|
||||
"child work",
|
||||
),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
fn child_session_whole_lists_do_not_project_when_root_has_no_list() {
|
||||
for (kind, child_list) in [
|
||||
(TodoListKind::OpenAiPlan, "openai_plan:child_session"),
|
||||
(TodoListKind::KimiTodos, "kimi_todos:child_session"),
|
||||
] {
|
||||
let mut state = initialized_projection();
|
||||
let stage_id = stage_id();
|
||||
state
|
||||
.apply_event(&test_stage_event(
|
||||
1,
|
||||
EventBody::StageStarted(started_props()),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
state
|
||||
.apply_event(&child_stage_event(
|
||||
2,
|
||||
created(child_list, kind, "c-a", 0, "child work"),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
|
||||
let stage = state.stage(&stage_id).expect("stage projection present");
|
||||
assert!(
|
||||
stage.root_agent_todos.is_none(),
|
||||
"a child session's plan must not become the stage's root plan"
|
||||
);
|
||||
let stage = state.stage(&stage_id).expect("stage projection present");
|
||||
assert!(
|
||||
stage.root_agent_todos.is_none(),
|
||||
"a child session's {kind} list must not become the stage's root list"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use fabro_agent::tool_registry::{RegisteredTool, ToolContext, ToolRegistry, Tool
|
|||
use fabro_agent::{
|
||||
AgentEvent, AgentProfile, AgentProfileBuilder, CompletionCoordinator, Message as AgentMessage,
|
||||
Sandbox, Session, SessionOptions, SessionShutdownReason, StaticEnvProvider, ToolEnvProvider,
|
||||
ToolSecrets, register_question_tools,
|
||||
ToolSecrets, canonical_tool_name, register_question_tools,
|
||||
};
|
||||
use fabro_auth::{CredentialSource, EnvCredentialSource};
|
||||
use fabro_graphviz::graph::{AttrValue, Node};
|
||||
|
|
@ -444,8 +444,12 @@ fn track_file_event(event: &AgentEvent, state: &mut FileTracking) {
|
|||
tool_name,
|
||||
tool_call_id,
|
||||
arguments,
|
||||
} if tool_name == "write_file" || tool_name == "edit_file" => {
|
||||
if let Some(path) = arguments.get("file_path").and_then(|v| v.as_str()) {
|
||||
} if matches!(canonical_tool_name(tool_name), "write_file" | "edit_file") => {
|
||||
if let Some(path) = arguments
|
||||
.get("file_path")
|
||||
.or_else(|| arguments.get("path"))
|
||||
.and_then(|v| v.as_str())
|
||||
{
|
||||
state.pending.insert(tool_call_id.clone(), path.to_string());
|
||||
}
|
||||
}
|
||||
|
|
@ -2622,6 +2626,34 @@ reasoning = false
|
|||
assert_eq!(state.last.as_deref(), Some("/src/lib.rs"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn track_file_event_tracks_kimi_write_alias() {
|
||||
let mut state = new_file_tracking();
|
||||
track_file_event(
|
||||
&AgentEvent::ToolCallStarted {
|
||||
tool_name: "Write".to_string(),
|
||||
tool_call_id: "tc-kimi".to_string(),
|
||||
arguments: serde_json::json!({
|
||||
"path": "/src/kimi.rs",
|
||||
"content": "new"
|
||||
}),
|
||||
},
|
||||
&mut state,
|
||||
);
|
||||
track_file_event(
|
||||
&AgentEvent::ToolCallCompleted {
|
||||
tool_call_id: "tc-kimi".to_string(),
|
||||
tool_name: "Write".to_string(),
|
||||
is_error: false,
|
||||
output: serde_json::Value::String("ok".to_string()),
|
||||
},
|
||||
&mut state,
|
||||
);
|
||||
|
||||
assert!(state.touched.contains("/src/kimi.rs"));
|
||||
assert_eq!(state.last.as_deref(), Some("/src/kimi.rs"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn track_file_event_error_removes_pending() {
|
||||
let mut state = new_file_tracking();
|
||||
|
|
|
|||
|
|
@ -269,18 +269,27 @@ fn permission_level_matches_openapi_json_shape() {
|
|||
|
||||
#[test]
|
||||
fn nested_agent_state_types_match_openapi_json_shape() {
|
||||
let todo_list = TodoListProjection::new(TodoListKind::OpenAiPlan, "openai_plan:ses_root");
|
||||
let todo_json = serde_json::to_value(&todo_list).unwrap();
|
||||
assert_eq!(
|
||||
todo_json,
|
||||
json!({
|
||||
"kind": "openai_plan",
|
||||
"list_id": "openai_plan:ses_root",
|
||||
"items": []
|
||||
})
|
||||
);
|
||||
let api_todo_list: ApiTodoListProjection = serde_json::from_value(todo_json).unwrap();
|
||||
assert_eq!(api_todo_list, todo_list);
|
||||
for (kind, list_id, wire_kind) in [
|
||||
(
|
||||
TodoListKind::OpenAiPlan,
|
||||
"openai_plan:ses_root",
|
||||
"openai_plan",
|
||||
),
|
||||
(TodoListKind::KimiTodos, "kimi_todos:ses_root", "kimi_todos"),
|
||||
] {
|
||||
let todo_list = TodoListProjection::new(kind, list_id);
|
||||
let todo_json = serde_json::to_value(&todo_list).unwrap();
|
||||
assert_eq!(
|
||||
todo_json,
|
||||
json!({
|
||||
"kind": wire_kind,
|
||||
"list_id": list_id,
|
||||
"items": []
|
||||
})
|
||||
);
|
||||
let api_todo_list: ApiTodoListProjection = serde_json::from_value(todo_json).unwrap();
|
||||
assert_eq!(api_todo_list, todo_list);
|
||||
}
|
||||
|
||||
let subagent = SubAgentProjection {
|
||||
agent_id: "sub-1".to_string(),
|
||||
|
|
|
|||
|
|
@ -105,17 +105,13 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn agent_profile_kind_round_trips_as_settings_strings() {
|
||||
for (kind, expected) in [
|
||||
(AgentProfileKind::Anthropic, "anthropic"),
|
||||
(AgentProfileKind::OpenAi, "openai"),
|
||||
(AgentProfileKind::Gemini, "gemini"),
|
||||
] {
|
||||
for kind in AgentProfileKind::VARIANTS {
|
||||
let expected = kind.to_string();
|
||||
let json = serde_json::to_string(&kind).unwrap();
|
||||
assert_eq!(json, format!("\"{expected}\""));
|
||||
let parsed: AgentProfileKind = serde_json::from_str(&json).unwrap();
|
||||
assert_eq!(parsed, kind);
|
||||
assert_eq!(expected.parse::<AgentProfileKind>().unwrap(), kind);
|
||||
assert_eq!(kind.to_string(), expected);
|
||||
assert_eq!(parsed, *kind);
|
||||
assert_eq!(expected.parse::<AgentProfileKind>().unwrap(), *kind);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,10 +1,12 @@
|
|||
//! Shared todo / task domain types used by `update_plan` (OpenAI) and the
|
||||
//! Claude task tools (`TaskCreate`, `TaskUpdate`, `TaskList`).
|
||||
//! Shared todo / task domain types used by `update_plan` (OpenAI), `TodoList`
|
||||
//! (Kimi Code), and the Claude task tools (`TaskCreate`, `TaskUpdate`,
|
||||
//! `TaskList`).
|
||||
//!
|
||||
//! Both tool families share the same event-sourced projection. The only
|
||||
//! difference is the scoping convention captured by [`TodoListKind`]:
|
||||
//!
|
||||
//! - `openai_plan:<session_id>` — one list per emitting session.
|
||||
//! - `kimi_todos:<session_id>` — one list per emitting session.
|
||||
//! - `anthropic_tasks:<root_session_id>` — one list shared by a root session
|
||||
//! and all of its subagent sessions.
|
||||
//!
|
||||
|
|
|
|||
|
|
@ -20,7 +20,8 @@
|
|||
|
||||
export const TodoListKind = {
|
||||
OPENAI_PLAN: 'openai_plan',
|
||||
ANTHROPIC_TASKS: 'anthropic_tasks'
|
||||
ANTHROPIC_TASKS: 'anthropic_tasks',
|
||||
KIMI_TODOS: 'kimi_todos'
|
||||
} as const;
|
||||
|
||||
export type TodoListKind = typeof TodoListKind[keyof typeof TodoListKind];
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue