diff --git a/README.md b/README.md index bdcdadf..c822c28 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,8 @@ Run with defaults (preferred): `--mcp file.json` loads a claude-code like mcp.json file. For example, the following configuration loads two STDIO based MCP servers. -The functions from `mcp-edit` will be prefixed with `files.` as in `files.create_file`, similarly with `shell.` for `mcp-shell`. +The functions from `mcp-edit` will be prefixed with `files_` as in `files_create_file`, similarly with `shell_` for `mcp-shell`. +MCP server names must not contain underscores because tool identifiers are generated as `_`. The server commands are launched with the same working directory that `llment` was. ```json diff --git a/crates/llm/AGENTS.md b/crates/llm/AGENTS.md index bf9479d..22f8038 100644 --- a/crates/llm/AGENTS.md +++ b/crates/llm/AGENTS.md @@ -82,12 +82,13 @@ Trait-based LLM client implementations for multiple providers. - tool calls with invalid arguments skip executor invocation and return "Could not parse arguments as JSON" - `mcp` module - `load_mcp_servers` starts configured MCP servers and collects tool schemas - - tool names are prefixed with the server name + - tool names are prefixed with the server name, joined with an underscore - `McpService` implements `ClientHandler` - `on_tool_list_changed` refreshes tool metadata from the service - tool metadata stored in an `ArcSwap` for lock-free snapshots - `McpContext` stores running service handles keyed by prefix - supports runtime insertion and removal of services via internal locking + - rejects service prefixes containing underscores to keep `_` parsing unambiguous - exposes merged `tool_infos` from all services - provides a non-blocking `tool_names` snapshot of available tools - implements `ToolExecutor` for MCP calls diff --git a/crates/llm/src/mcp.rs b/crates/llm/src/mcp.rs index 8c1fa28..e6220ff 100644 --- a/crates/llm/src/mcp.rs +++ b/crates/llm/src/mcp.rs @@ -10,6 +10,7 @@ use serde::Deserialize; use serde_json::Value; use std::{ collections::HashMap, + fmt, sync::{Arc, Mutex}, }; use tokio::process::Command; @@ -21,6 +22,39 @@ pub struct McpService { pub tools: ArcSwap>, } +#[derive(Debug)] +pub struct InvalidPrefixError { + prefix: String, +} + +impl InvalidPrefixError { + pub fn new(prefix: impl Into) -> Self { + Self { + prefix: prefix.into(), + } + } + + pub fn prefix(&self) -> &str { + &self.prefix + } +} + +impl fmt::Display for InvalidPrefixError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "MCP prefix '{}' must not contain '_'", self.prefix) + } +} + +impl std::error::Error for InvalidPrefixError {} + +fn validate_prefix(prefix: &str) -> Result<(), InvalidPrefixError> { + if prefix.contains('_') { + Err(InvalidPrefixError::new(prefix)) + } else { + Ok(()) + } +} + impl ClientHandler for McpService { fn on_tool_list_changed( &self, @@ -53,9 +87,14 @@ pub struct McpContext { } impl McpContext { - pub fn insert(&self, service: RunningService) { + pub fn insert( + &self, + service: RunningService, + ) -> Result<(), InvalidPrefixError> { let prefix = service.service().prefix.clone(); + validate_prefix(&prefix)?; self.services.lock().unwrap().insert(prefix, service); + Ok(()) } pub fn remove(&self, prefix: &str) { @@ -70,7 +109,7 @@ impl McpContext { let tools = svc.service().tools.load(); for tool in tools.iter() { infos.push(ToolInfo { - name: format!("{}.{}", prefix, tool.name), + name: format!("{}_{}", prefix, tool.name), description: tool.description.clone(), parameters: tool.parameters.clone(), }); @@ -86,7 +125,7 @@ impl McpContext { let prefix = svc.service().prefix.clone(); let tools = svc.service().tools.load(); for tool in tools.iter() { - names.push(format!("{}.{}", prefix, tool.name)); + names.push(format!("{}_{}", prefix, tool.name)); } } names @@ -101,7 +140,7 @@ impl ToolExecutor for McpContext { args: Value, ) -> Result> { let (prefix, tool_name) = name - .split_once('.') + .split_once('_') .ok_or_else(|| format!("{name} is not a valid tool name"))?; let peer = { let services = self.services.lock().unwrap(); @@ -161,6 +200,7 @@ pub async fn load_mcp_servers( let config: McpConfig = serde_json::from_str(&data)?; let ctx = McpContext::default(); for (server_name, server) in config.mcp_servers.iter() { + validate_prefix(server_name)?; let mut cmd = Command::new(&server.command); cmd.args(&server.args); for (k, v) in &server.env { @@ -184,7 +224,24 @@ pub async fn load_mcp_servers( }); } service.service().tools.store(Arc::new(infos)); - ctx.insert(service); + ctx.insert(service)?; } Ok(ctx) } + +#[cfg(test)] +mod tests { + use super::validate_prefix; + + #[test] + fn accepts_prefix_without_underscore() { + assert!(validate_prefix("files").is_ok()); + assert!(validate_prefix("agent").is_ok()); + } + + #[test] + fn rejects_prefix_with_underscore() { + let err = validate_prefix("with_underscore").unwrap_err(); + assert_eq!(err.prefix(), "with_underscore"); + } +} diff --git a/crates/llment/AGENTS.md b/crates/llment/AGENTS.md index 1485be5..0640352 100644 --- a/crates/llment/AGENTS.md +++ b/crates/llment/AGENTS.md @@ -98,8 +98,8 @@ Basic terminal chat interface scaffold using a bespoke component framework built - agent mode `step` can signal `stop` to exit the mode - agent modes may adjust or clear the role between steps - agent modes may register an MCP service under the `agent` prefix that is added on start and removed when switching modes or when the mode stops - - available modes include `code-agent` coordinating director, design-lead, execution-lead, eng-team, and reviewer roles via an `agent.notify` MCP tool - - `agent.notify` accepts `role` values of `director`, `design-lead`, `execution-lead`, `eng-team`, or `reviewer` + - available modes include `code-agent` coordinating director, design-lead, execution-lead, eng-team, and reviewer roles via an `agent_notify` MCP tool + - `agent_notify` accepts `role` values of `director`, `design-lead`, `execution-lead`, `eng-team`, or `reviewer` - `/agent-mode off` exits the active agent mode - command commit behavior - on successful commit, the router clears the active command instance @@ -164,8 +164,8 @@ Basic terminal chat interface scaffold using a bespoke component framework built - Built-in tools registered via `setup_builtin_tools` - `setup_builtin_tools` returns a running `McpService` inserted into the shared `McpContext` - Tools: - - `chat.get_message_count`: returns the number of chat messages - - `chat.discard_function_response`: + - `chat_get_message_count`: returns the number of chat messages + - `chat_discard_function_response`: - parameters: `{ id: string }` - finds a `Tool` message by matching `id` and clears its result text - returns `"ok"` on success or a not-found message if no matching entry exists @@ -179,4 +179,4 @@ Basic terminal chat interface scaffold using a bespoke component framework built - switching model aborts in-flight requests without clearing history or resetting token counters - partial items are clipped when scrolled - collapsed content does not contribute to layout height -- MCP tool names are prefixed with the server name; built-in tools use the `chat` prefix, agent mode tools use the `agent` prefix +- MCP tool names are prefixed with the server name, joined with an underscore; prefixes must not contain underscores; built-in tools use the `chat` prefix, agent mode tools use the `agent` prefix diff --git a/crates/llment/prompts/roles/code-agent/design-lead.md b/crates/llment/prompts/roles/code-agent/design-lead.md index 1739684..98322f2 100644 --- a/crates/llment/prompts/roles/code-agent/design-lead.md +++ b/crates/llment/prompts/roles/code-agent/design-lead.md @@ -18,5 +18,5 @@ Do not modify any other files in the workspace, leave that to the team. When you are finished: 1. use git to commit your changes, make sure the working directory is clean before continuing -2. call agent.notify for role `execution-lead` +2. call agent_notify for role `execution-lead` 3. stop diff --git a/crates/llment/prompts/roles/code-agent/director.md b/crates/llment/prompts/roles/code-agent/director.md index 8edcf95..978597b 100644 --- a/crates/llment/prompts/roles/code-agent/director.md +++ b/crates/llment/prompts/roles/code-agent/director.md @@ -1,7 +1,7 @@ Analyze the content of the workspace. -If there exists a top-level `task.md` file, then call agent.notify for role `eng-team`, ask them to resume their work, and stop. +If there exists a top-level `task.md` file, then call agent_notify for role `eng-team`, ask them to resume their work, and stop. -If there is already a top-level `objective.md` file, then call agent.notify for role `design-lead` and stop. +If there is already a top-level `objective.md` file, then call agent_notify for role `design-lead` and stop. -Otherwise, discuss with the user what the objective should be; once you have enough information, after confirming with the user, write the objective to the top-level `objective.md`, use git to commit the change, and then call agent.notify for role `design-lead` and stop. +Otherwise, discuss with the user what the objective should be; once you have enough information, after confirming with the user, write the objective to the top-level `objective.md`, use git to commit the change, and then call agent_notify for role `design-lead` and stop. diff --git a/crates/llment/prompts/roles/code-agent/eng-team.md b/crates/llment/prompts/roles/code-agent/eng-team.md index 3795b2d..fc48727 100644 --- a/crates/llment/prompts/roles/code-agent/eng-team.md +++ b/crates/llment/prompts/roles/code-agent/eng-team.md @@ -22,5 +22,5 @@ Begin work on the task. When you are completely finished with the task: 1. ensure the task log is up to date - did you complete all the TODOs? 2. use git to commit your changes, make sure the working directory is clean before continuing (git status) -3. call agent.notify for role `reviewer` summarizing any deviations from the task +3. call agent_notify for role `reviewer` summarizing any deviations from the task 4. stop diff --git a/crates/llment/prompts/roles/code-agent/execution-lead.md b/crates/llment/prompts/roles/code-agent/execution-lead.md index 753e06c..6665deb 100644 --- a/crates/llment/prompts/roles/code-agent/execution-lead.md +++ b/crates/llment/prompts/roles/code-agent/execution-lead.md @@ -22,5 +22,5 @@ Do not modify any other files in the workspace, leave that to the eng team. When you are finished: 1. use git to commit your changes, make sure the working directory is clean (git status) before continuing -2. call agent.notify for role `eng-team` +2. call agent_notify for role `eng-team` 3. stop diff --git a/crates/llment/prompts/roles/code-agent/reviewer.md b/crates/llment/prompts/roles/code-agent/reviewer.md index 4d7b33e..8d76e5e 100644 --- a/crates/llment/prompts/roles/code-agent/reviewer.md +++ b/crates/llment/prompts/roles/code-agent/reviewer.md @@ -19,7 +19,7 @@ Do not rely on or trust the task log or the message from the team. Verify the ch Do not modify the code, if there are problems the eng-team will fix them. -If the working directory is dirty or there are problems call agent.notify for role `eng-team`, and summarize any problems or changes that are necessary, and stop. +If the working directory is dirty or there are problems call agent_notify for role `eng-team`, and summarize any problems or changes that are necessary, and stop. -Otherwise, only if the work is satisfactory, call agent.notify for role `execution-lead`, and request that they assign the next task; if there are any deviations from the task, summarize them, and stop. +Otherwise, only if the work is satisfactory, call agent_notify for role `execution-lead`, and request that they assign the next task; if there are any deviations from the task, summarize them, and stop. diff --git a/crates/llment/prompts/snippets/env/workspace.md b/crates/llment/prompts/snippets/env/workspace.md index bd89bc1..1f9b9a9 100644 --- a/crates/llment/prompts/snippets/env/workspace.md +++ b/crates/llment/prompts/snippets/env/workspace.md @@ -1,6 +1,6 @@ # Workspace Overview -All commands, {% if tool_enabled("shell.run") %}shell interactions, {% endif %}and tools executed in this environment are performed **relative to the workspace root** by default: +All commands, {% if tool_enabled("shell_run") %}shell interactions, {% endif %}and tools executed in this environment are performed **relative to the workspace root** by default: ``` /home/user/workspace diff --git a/crates/llment/prompts/snippets/instructions/context.md b/crates/llment/prompts/snippets/instructions/context.md index 07f633a..fddbabd 100644 --- a/crates/llment/prompts/snippets/instructions/context.md +++ b/crates/llment/prompts/snippets/instructions/context.md @@ -1,10 +1,9 @@ -{% if tool_enabled("chat.discard_function_response") %} +{% if tool_enabled("chat_discard_function_response") %} ## Context In this environment there is limited history and context size available. -It's important that you remove function responses (FR) that are no longer necessary by calling the chat. -discard_function_response. +It's important that you remove function responses (FR) that are no longer necessary by calling the `chat_discard_function_response` tool. Summarize the necessary parts of the FR with chain-of-thought, then proactively discard the FR as soon as possible -- before proceeding with other function calls. If the contents of the FR needs to be part of a message to the user, wait for a subsequent round before discarding. diff --git a/crates/llment/prompts/snippets/shell/apply_patch.md b/crates/llment/prompts/snippets/shell/apply_patch.md index ecdd8a9..15c3f40 100644 --- a/crates/llment/prompts/snippets/shell/apply_patch.md +++ b/crates/llment/prompts/snippets/shell/apply_patch.md @@ -1,4 +1,4 @@ -{% if tool_enabled("shell.run") %} +{% if tool_enabled("shell_run") %} You have access to the `apply_patch` shell command to edit files. Follow these rules exactly. **Contract** diff --git a/crates/llment/prompts/snippets/shell/ls.md b/crates/llment/prompts/snippets/shell/ls.md index 3da2ed5..bc78687 100644 --- a/crates/llment/prompts/snippets/shell/ls.md +++ b/crates/llment/prompts/snippets/shell/ls.md @@ -1,4 +1,4 @@ -{% if tool_enabled("shell.run") %} +{% if tool_enabled("shell_run") %} ## Searching and Listing Files Use `rg` -- ripgrep. Always use `rg` instead of `grep`. diff --git a/crates/llment/prompts/snippets/shell/timeout.md b/crates/llment/prompts/snippets/shell/timeout.md index cdb4231..f60ab59 100644 --- a/crates/llment/prompts/snippets/shell/timeout.md +++ b/crates/llment/prompts/snippets/shell/timeout.md @@ -1,7 +1,7 @@ -{% if tool_enabled("shell.wait") %} +{% if tool_enabled("shell_wait") %} # Timeout Handling for Shell Commands -When a command launched via `shell.run` exceeds the allotted time, the +When a command launched via `shell_run` exceeds the allotted time, the operation may either still be making progress or be stalled. You should decide whether to continue waiting for a normal exit or to terminate the process. @@ -12,9 +12,9 @@ terminate the process. command times out. 2. **Check for ongoing progress** – If the command is still producing output (stdout/stderr) or updating its internal state, it is likely - alive. In this case, call `shell.wait` to allow the process to + alive. In this case, call `shell_wait` to allow the process to continue. 3. **Abort if stalled** – If the command shows no output for a period - or appears stuck, call `shell.terminate`. + or appears stuck, call `shell_terminate`. {% endif %} diff --git a/crates/llment/src/app.rs b/crates/llment/src/app.rs index 96e836b..80e6987 100644 --- a/crates/llment/src/app.rs +++ b/crates/llment/src/app.rs @@ -202,7 +202,9 @@ impl App { pub async fn init(&mut self, mcp_context: McpContext) { self.mcp_context = mcp_context; let builtin_service = setup_builtin_tools(self.chat_history.clone()).await; - self.mcp_context.insert(builtin_service); + self.mcp_context + .insert(builtin_service) + .expect("builtin MCP prefix must not contain '_'"); } fn handle_tool_event(&mut self, ev: ToolEvent) { @@ -494,7 +496,12 @@ impl Component for App { self.abort_requests(); self.mode = mode; if let Some(service) = service { - self.mcp_context.insert(service); + if let Err(err) = self.mcp_context.insert(service) { + self.mode = None; + self.error.set(err.to_string()); + let _ = self.model.needs_redraw.send(true); + continue; + } } let start = if let Some(mode) = self.mode.as_mut() { Some(mode.start()) diff --git a/crates/llment/src/modes/code_agent.rs b/crates/llment/src/modes/code_agent.rs index dabad72..f4985f5 100644 --- a/crates/llment/src/modes/code_agent.rs +++ b/crates/llment/src/modes/code_agent.rs @@ -164,7 +164,7 @@ impl AgentMode for CodeAgentMode { AgentModeStep { role: Some(format!("code-agent/{}", self.current_role.as_str())), prompt: Some( - "Please finish your job and then call agent.notify(role, message) as requested.".to_string(), + "Please finish your job and then call agent_notify(role, message) as requested.".to_string(), ), clear_history: false, stop: false, diff --git a/crates/llment/src/prompts.rs b/crates/llment/src/prompts.rs index 4f2c07d..35b4c06 100644 --- a/crates/llment/src/prompts.rs +++ b/crates/llment/src/prompts.rs @@ -158,7 +158,7 @@ mod tests { #[test] fn tool_enabled_fn() { - let content = load_prompt("tool", None, vec!["shell.run".to_string()], None).unwrap(); + let content = load_prompt("tool", None, vec!["shell_run".to_string()], None).unwrap(); assert!(content.contains("Enabled!")); let content = load_prompt("tool", None, Vec::new(), None).unwrap(); assert!(content.contains("Disabled!")); diff --git a/crates/llment/tests/prompts/system/tool.md b/crates/llment/tests/prompts/system/tool.md index 358f6cf..12b23fe 100644 --- a/crates/llment/tests/prompts/system/tool.md +++ b/crates/llment/tests/prompts/system/tool.md @@ -1,4 +1,4 @@ -{% if tool_enabled("shell.run") %} +{% if tool_enabled("shell_run") %} Enabled! {% else %} Disabled!