Fix OpenAI-compatible agent tools

lib/crates/fabro-agent/src/profiles/openai.rs

@@ -1,6 +1,6 @@
 use std::sync::Arc;
 
-use fabro_model::{AgentProfileKind, Catalog, ProviderId};
+use fabro_model::{AgentProfileKind, Catalog, CodecKind, ProviderId};
 
 use super::EnvContext;
 use crate::agent_profile::AgentProfile;
@@ -12,10 +12,27 @@ use crate::skills::Skill;
 use crate::todo_runtime::TodoRuntime;
 use crate::todo_tools::make_update_plan_tool;
 use crate::tool_registry::ToolRegistry;
-use crate::tools::{WebFetchSummarizer, register_core_tools};
+use crate::tools::{self, WebFetchSummarizer, register_core_tools};
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum FileEditToolKind {
+    ApplyPatch,
+    EditFile,
+}
+
+impl FileEditToolKind {
+    fn for_codec(codec: CodecKind) -> Self {
+        if codec == CodecKind::OpenAiResponses {
+            Self::ApplyPatch
+        } else {
+            Self::EditFile
+        }
+    }
+}
 
 pub struct OpenAiProfile {
-    base: BaseProfile,
+    base:           BaseProfile,
+    file_edit_tool: FileEditToolKind,
 }
 
 impl OpenAiProfile {
@@ -39,13 +56,14 @@ impl OpenAiProfile {
         registry.register(make_update_plan_tool(todo_runtime));
 
         Self {
-            base: BaseProfile {
+            base:           BaseProfile {
                 profile_kind: AgentProfileKind::OpenAi,
                 provider_id: ProviderId::openai(),
                 model: model.into(),
                 catalog: None,
                 registry,
             },
+            file_edit_tool: FileEditToolKind::ApplyPatch,
         }
     }
 
@@ -53,15 +71,43 @@ impl OpenAiProfile {
     #[must_use]
     pub fn with_provider_id(mut self, provider_id: ProviderId) -> Self {
         self.base.provider_id = provider_id;
+        self.configure_file_edit_tool();
         self
     }
 
     #[must_use]
     pub fn with_catalog(mut self, catalog: Arc<Catalog>) -> Self {
         self.base.catalog = Some(catalog);
+        self.configure_file_edit_tool();
         self
     }
 
+    fn configure_file_edit_tool(&mut self) {
+        let Some(codec) = self.base.catalog.as_ref().and_then(|catalog| {
+            catalog.effective_codec(&self.base.provider_id, Some(&self.base.model))
+        }) else {
+            return;
+        };
+        let desired = FileEditToolKind::for_codec(codec);
+        if desired == self.file_edit_tool {
+            return;
+        }
+
+        match desired {
+            FileEditToolKind::ApplyPatch => {
+                self.base.registry.unregister("edit_file");
+                self.base
+                    .registry
+                    .register(apply_patch::make_apply_patch_tool());
+            }
+            FileEditToolKind::EditFile => {
+                self.base.registry.unregister("apply_patch");
+                self.base.registry.register(tools::make_edit_file_tool());
+            }
+        }
+        self.file_edit_tool = desired;
+    }
+
     fn provider_display_name(&self) -> String {
         self.base
             .catalog
@@ -108,13 +154,47 @@ impl AgentProfile for OpenAiProfile {
         skills: &[Skill],
     ) -> String {
         let provider_name = self.provider_display_name();
+        let (file_edit_tool_name, file_edit_failure_guidance, file_edit_tool_guidance) =
+            match self.file_edit_tool {
+                FileEditToolKind::ApplyPatch => (
+                    "apply_patch",
+                    "- When apply_patch fails, use the error text to construct a corrected patch. \
+Re-read the target file if you need fresh context.",
+                    "## apply_patch
+Use the `apply_patch` tool for all file modifications. This is a freeform tool: pass the raw \
+patch text directly, never wrap it in JSON. The format uses `*** Begin Patch` / \
+`*** End Patch` delimiters with `*** Add File:`, `*** Delete File:`, `*** Update File:` \
+operations. Use `-` for removals, `+` for additions, and space-prefix for unchanged context \
+lines. Show 3 lines of context around each change. NEVER use `applypatch` or `apply-patch`, \
+only `apply_patch`.
+
+Example:
+```
+*** Begin Patch
+*** Update File: src/main.py
+@@ def hello():
+-    print(\"old\")
++    print(\"new\")
+*** End Patch
+```",
+                ),
+                FileEditToolKind::EditFile => (
+                    "edit_file",
+                    "- When edit_file fails, use the error text to construct a corrected exact \
+replacement. Re-read the target file if you need fresh context.",
+                    "## edit_file
+Use `edit_file` to modify an existing file by replacing an exact string. Read the file first. \
+The `old_string` must match exactly and be unique unless `replace_all` is true; include enough \
+surrounding context to make the match unique and preserve the existing indentation.",
+                ),
+            };
         let core_prompt = format!("\
 You are a coding agent powered by {provider_name}, running in a terminal-based agentic coding assistant. \
 You are expected to be precise, safe, and helpful.
 
 You can receive user prompts and context such as files in the workspace, communicate with the \
 user by streaming thinking and responses, and emit function calls to run terminal commands and \
-apply patches.
+edit files.
 
 # Personality
 
@@ -146,8 +226,7 @@ If completing the task requires writing or modifying files:
 and focused on the task.
 - Use `git log` and `git blame` to search the history of the codebase if additional context is needed.
 - NEVER add copyright or license headers unless specifically requested.
-- When apply_patch fails, use the error text to construct a corrected patch. Re-read the target \
-file if you need fresh context.
+{file_edit_failure_guidance}
 - Do not `git commit` your changes or create new git branches unless explicitly requested.
 
 # Validating Your Work
@@ -163,26 +242,10 @@ Use the provided tools to interact with the codebase and environment.
 ## read_file
 Read files to understand code before modifying. Use offset/limit for large files.
 
-## apply_patch
-Use the `apply_patch` tool for all file modifications. This is a freeform tool: pass the raw \
-patch text directly, never wrap it in JSON. The format uses `*** Begin Patch` / \
-`*** End Patch` delimiters with `*** Add File:`, `*** Delete File:`, `*** Update File:` \
-operations. Use `-` for removals, `+` for additions, and space-prefix for unchanged context \
-lines. Show 3 lines of context around each change. NEVER use `applypatch` or `apply-patch`, \
-only `apply_patch`.
-
-Example:
-```
-*** Begin Patch
-*** Update File: src/main.py
-@@ def hello():
--    print(\"old\")
-+    print(\"new\")
-*** End Patch
-```
+{file_edit_tool_guidance}
 
 ## write_file
-Use for creating new files. For modifications, prefer apply_patch.
+Use for creating new files. For modifications, prefer {file_edit_tool_name}.
 
 ## shell
 Execute shell commands. Default timeout is 10 seconds. Use timeout_ms parameter for \
@@ -350,6 +413,44 @@ mod tests {
         assert!(!prompt.contains("powered by OpenAI"));
     }
 
+    #[test]
+    fn openai_compatible_profile_uses_json_schema_edit_tool() {
+        let profile = OpenAiProfile::new("kimi-k2.5")
+            .with_provider_id(ProviderId::new("kimi"))
+            .with_catalog(test_catalog());
+
+        let names = profile.tool_registry().names();
+        assert!(names.contains(&"edit_file".to_string()));
+        assert!(!names.contains(&"apply_patch".to_string()));
+
+        let edit_file = profile.tool_registry().get("edit_file").unwrap();
+        assert!(!edit_file.definition.is_custom());
+        assert_eq!(edit_file.definition.parameters["type"], "object");
+        for definition in profile.tool_registry().definitions() {
+            assert_eq!(
+                definition.parameters["type"], "object",
+                "tool '{}' must use an object parameter schema",
+                definition.name
+            );
+        }
+
+        let env = MockSandbox::linux();
+        let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]);
+        assert!(prompt.contains("## edit_file"));
+        assert!(!prompt.contains("## apply_patch"));
+        assert!(!prompt.contains("freeform tool"));
+    }
+
+    #[test]
+    fn file_edit_tool_selection_is_builder_order_independent() {
+        let profile = OpenAiProfile::new("kimi-k2.5")
+            .with_catalog(test_catalog())
+            .with_provider_id(ProviderId::new("kimi"));
+
+        assert!(profile.tool_registry().get("edit_file").is_some());
+        assert!(profile.tool_registry().get("apply_patch").is_none());
+    }
+
     #[test]
     fn zai_provider_prompt_uses_catalog_display_name() {
         let profile = OpenAiProfile::new("glm-4.7")

lib/crates/fabro-agent/tests/it/parity_matrix.rs

@@ -56,11 +56,16 @@ fn build_summarizer(provider: &Provider, client: &Client) -> WebFetchSummarizer
 
 fn build_profile(provider: &Provider, model: &str, client: &Client) -> Box<dyn AgentProfile> {
     let summarizer = Some(build_summarizer(provider, client));
+    let catalog = Arc::new(Catalog::from_builtin().expect("default catalog should build"));
     match provider.as_str() {
         ProviderId::ANTHROPIC => Box::new(AnthropicProfile::with_summarizer(model, summarizer)),
-        ProviderId::OPENAI => Box::new(OpenAiProfile::with_summarizer(model, summarizer)),
+        ProviderId::OPENAI => Box::new(
+            OpenAiProfile::with_summarizer(model, summarizer).with_catalog(Arc::clone(&catalog)),
+        ),
         "kimi" | "zai" | "minimax" | "inception" => Box::new(
-            OpenAiProfile::with_summarizer(model, summarizer).with_provider_id(provider.clone()),
+            OpenAiProfile::with_summarizer(model, summarizer)
+                .with_provider_id(provider.clone())
+                .with_catalog(Arc::clone(&catalog)),
         ),
         ProviderId::GEMINI => Box::new(GeminiProfile::with_summarizer(model, summarizer)),
         other => panic!("unexpected provider {other}"),
@@ -85,6 +90,7 @@ async fn make_session(
     let factory_cwd = cwd.to_path_buf();
     let factory_provider = provider.clone();
     let factory: SessionFactory = Arc::new(move || {
+        let catalog = Arc::new(Catalog::from_builtin().expect("default catalog should build"));
         let sub_profile: Arc<dyn AgentProfile> = {
             let summarizer = Some(build_summarizer(&factory_provider, &factory_client));
             match factory_provider.as_str() {
@@ -92,12 +98,14 @@ async fn make_session(
                     &factory_model,
                     summarizer,
                 )),
-                ProviderId::OPENAI => {
-                    Arc::new(OpenAiProfile::with_summarizer(&factory_model, summarizer))
-                }
+                ProviderId::OPENAI => Arc::new(
+                    OpenAiProfile::with_summarizer(&factory_model, summarizer)
+                        .with_catalog(Arc::clone(&catalog)),
+                ),
                 "kimi" | "zai" | "minimax" | "inception" => Arc::new(
                     OpenAiProfile::with_summarizer(&factory_model, summarizer)
-                        .with_provider_id(factory_provider.clone()),
+                        .with_provider_id(factory_provider.clone())
+                        .with_catalog(Arc::clone(&catalog)),
                 ),
                 ProviderId::GEMINI => {
                     Arc::new(GeminiProfile::with_summarizer(&factory_model, summarizer))
@@ -176,8 +184,12 @@ fn make_openai_compatible_twin_session(
     twin: &OpenAiTwinOptions,
 ) -> Session {
     let client = make_openai_compatible_twin_client(&provider, twin);
-    let profile: Arc<dyn AgentProfile> =
-        Arc::new(OpenAiProfile::new(model).with_provider_id(provider));
+    let catalog = Arc::new(Catalog::from_builtin().expect("default catalog should build"));
+    let profile: Arc<dyn AgentProfile> = Arc::new(
+        OpenAiProfile::new(model)
+            .with_provider_id(provider)
+            .with_catalog(catalog),
+    );
     let env = Arc::new(LocalSandbox::new(cwd.to_path_buf()));
     Session::new(client, profile, env, config, None)
 }
@@ -271,27 +283,25 @@ macro_rules! provider_tests {
 }
 
 #[fabro_macros::e2e_test(twin)]
-async fn openai_compatible_twin_preserves_raw_apply_patch_arguments() {
+async fn openai_compatible_twin_uses_json_edit_file_tool() {
     let tmp = tempfile::tempdir().expect("failed to create tempdir");
     let file_path = tmp.path().join("data.txt");
     std::fs::write(&file_path, "old\n").expect("failed to write data.txt");
 
     let (base_url, api_key) = fabro_test::e2e_openai!();
     let twin = OpenAiTwinOptions { base_url, api_key };
-    let patch = "\
-*** Begin Patch
-*** Update File: data.txt
-@@
--old
-+new
-*** End Patch
-";
-
     TwinScenarios::new(twin.api_key.clone())
         .scenario(
             TwinScenario::chat_completions("gpt-5.4-mini")
                 .input_contains("Replace old with new")
-                .tool_call(TwinToolCall::apply_patch_raw_arguments(patch)),
+                .tool_call(TwinToolCall::new(
+                    "edit_file",
+                    serde_json::json!({
+                        "file_path": "data.txt",
+                        "old_string": "old",
+                        "new_string": "new"
+                    }),
+                )),
         )
         .load(twin_openai().await)
         .await;
@@ -311,7 +321,7 @@ async fn openai_compatible_twin_preserves_raw_apply_patch_arguments() {
     let mut rx = session.subscribe();
 
     session
-        .process_input("Replace old with new in data.txt using apply_patch")
+        .process_input("Replace old with new in data.txt using edit_file")
         .await
         .expect("process_input failed");
 

lib/crates/fabro-llm/src/codec/openai_compatible/mod.rs

@@ -21,7 +21,7 @@ pub(crate) struct OpenAiCompatible;
 
 impl Codec for OpenAiCompatible {
     fn encode(&self, ctx: &CodecCtx<'_>, stream: bool) -> Result<EncodedRequest, Error> {
-        Ok(request::encode(ctx, stream))
+        request::encode(ctx, stream)
     }
 
     fn decode_response(

lib/crates/fabro-llm/src/codec/openai_compatible/request.rs

@@ -3,20 +3,22 @@
 use super::translate;
 use super::wire::ApiRequest;
 use crate::codec::{CodecCtx, EncodedRequest, merge_named_provider_options};
+use crate::error::Error;
 
 /// Build the Chat Completions request for `ctx.request`. `stream` toggles the
 /// `stream` body field. The body is assembled as a `serde_json::Value` so
 /// `provider_options.<provider_name>` fields can be merged in before sending.
 ///
-/// Infallible for this dialect — the `Codec::encode` `Result` is wrapped by the
-/// trait impl.
-pub(super) fn encode(ctx: &CodecCtx<'_>, stream: bool) -> EncodedRequest {
+/// Returns an error when the request contains a custom tool definition, which
+/// the Chat Completions tool envelope cannot represent.
+pub(super) fn encode(ctx: &CodecCtx<'_>, stream: bool) -> Result<EncodedRequest, Error> {
     let request = ctx.request;
     let chat_messages = translate::translate_messages(&request.messages);
     let tools = request
         .tools
         .as_ref()
-        .map(|t| translate::translate_tools(t));
+        .map(|t| translate::translate_tools(t))
+        .transpose()?;
     let tool_choice = request
         .tool_choice
         .as_ref()
@@ -46,11 +48,11 @@ pub(super) fn encode(ctx: &CodecCtx<'_>, stream: bool) -> EncodedRequest {
         ctx.provider_name,
     );
 
-    EncodedRequest {
+    Ok(EncodedRequest {
         body,
         endpoint: "/chat/completions".to_string(),
         headers: Vec::new(),
-    }
+    })
 }
 
 /// Merge `provider_options.<provider_name>` fields into the serialized API
@@ -71,7 +73,7 @@ mod tests {
     use super::super::wire::ApiRequest;
     use super::*;
     use crate::codec::CodecParams;
-    use crate::types::{Message, Request};
+    use crate::types::{Message, Request, ToolDefinition};
 
     fn minimal_request() -> Request {
         Request {
@@ -104,7 +106,7 @@ mod tests {
             model: None,
             params: &params,
         };
-        encode(&ctx, stream).body
+        encode(&ctx, stream).unwrap().body
     }
 
     #[test]
@@ -152,10 +154,38 @@ mod tests {
             model:         None,
             params:        &params,
         };
-        let body = encode(&ctx, false).body;
+        let body = encode(&ctx, false).unwrap().body;
         assert_eq!(body["model"], "acme/model-large");
     }
 
+    #[test]
+    fn encode_rejects_custom_tool_definitions() {
+        let mut request = minimal_request();
+        request.tools = Some(vec![ToolDefinition::custom(
+            "apply_patch",
+            "Apply a patch",
+            serde_json::json!({"type": "grammar"}),
+        )]);
+        let params = CodecParams::default();
+        let deployment_id = request.model.clone();
+        let ctx = CodecCtx {
+            request:       &request,
+            provider_name: "kimi",
+            deployment_id: &deployment_id,
+            model:         None,
+            params:        &params,
+        };
+
+        let Err(error) = encode(&ctx, false) else {
+            panic!("custom tool definition should be rejected");
+        };
+        assert!(matches!(
+            error,
+            Error::Configuration { message, source: None }
+                if message.contains("custom tool definition 'apply_patch'")
+        ));
+    }
+
     #[test]
     fn provider_options_none_produces_standard_body() {
         let request = minimal_request();

lib/crates/fabro-llm/src/codec/openai_compatible/translate.rs

@@ -1,6 +1,7 @@
 //! Pure mapping between canonical types and the Chat Completions wire shapes.
 
 use super::wire::{ChatFunction, ChatMessage, ChatToolCall};
+use crate::error::Error;
 use crate::types::{
     ContentPart, CostSource, FinishReason, Message, Request, ResponseFormat, ResponseFormatType,
     Role, ToolChoice, ToolDefinition,
@@ -146,18 +147,28 @@ pub(super) fn translate_messages(messages: &[Message]) -> Vec<ChatMessage> {
         .collect()
 }
 
-pub(super) fn translate_tools(tools: &[ToolDefinition]) -> Vec<serde_json::Value> {
+pub(super) fn translate_tools(tools: &[ToolDefinition]) -> Result<Vec<serde_json::Value>, Error> {
     tools
         .iter()
         .map(|t| {
-            serde_json::json!({
+            if t.is_custom() {
+                return Err(Error::Configuration {
+                    message: format!(
+                        "openai_compatible codec does not support custom tool definition '{}'",
+                        t.name
+                    ),
+                    source:  None,
+                });
+            }
+
+            Ok(serde_json::json!({
                 "type": "function",
                 "function": {
                     "name": t.name,
                     "description": t.description,
                     "parameters": t.parameters,
                 }
-            })
+            }))
         })
         .collect()
 }