From c268217e6e7a160cb3cec618f92b2baff0a2c7af Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Sun, 5 Jul 2026 21:15:02 -0300 Subject: [PATCH] =?UTF-8?q?feat:=20edit-shape=20fidelity=20=E2=80=94=20rou?= =?UTF-8?q?ted=20provider=20family=20+=20freeform=20apply=5Fpatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - harness: a send (or parentless spawn) that omits provider resolves it from the router's model catalog before options freeze, so prompt-family selection and routing agree — headless codex sends now get the apply_patch code identity without passing provider explicitly. Router unreachable → warn and fall back to the previous default-family behavior. - provider-openai-codex: coder::apply-patch travels as a freeform custom tool named exactly apply_patch — the trained name and shape. The wire emits { type: custom }, and the SSE state machine accumulates custom_tool_call raw input (delta + authoritative done) into { patch: }, so patches cross the wire with no JSON escaping. Harness and shell still see an ordinary coder::apply-patch call. --- harness/src/functions/send.rs | 71 +++++++++++++++--- harness/src/subagent.rs | 10 ++- provider-openai-codex/src/sse.rs | 98 ++++++++++++++++++++++++- provider-openai-codex/src/wire/names.rs | 27 +++++++ provider-openai-codex/src/wire/tools.rs | 28 +++++++ 5 files changed, 220 insertions(+), 14 deletions(-) diff --git a/harness/src/functions/send.rs b/harness/src/functions/send.rs index e05ab51df..d857aace3 100644 --- a/harness/src/functions/send.rs +++ b/harness/src/functions/send.rs @@ -121,7 +121,13 @@ pub async fn start(deps: &Deps, req: SendRequest) -> Result Some(p), + None => resolve_provider(deps, &req.model).await, + }; + let options = build_options(&cfg, &req, provider); // Normalise the incoming message and validate its role. let message = normalize_message(req.message)?; @@ -235,7 +241,27 @@ pub(crate) fn normalize_message(input: MessageInput) -> Result TurnOptions { +/// Resolve the provider for a send that omitted it, via the router's model +/// catalog — prompt-family selection and routing then agree. `None` when the +/// router is unreachable or the model is unknown (the prompt family then +/// falls back to the seeded default, mirroring the un-routed mesh prompt). +pub(crate) async fn resolve_provider(deps: &Deps, model: &str) -> Option { + let provider = deps + .router() + .await + .models_get(None, model) + .await + .map(|m| m.provider); + if provider.is_none() { + tracing::warn!( + model, + "provider not resolvable from the router catalog; prompt family falls back to the default" + ); + } + provider +} + +fn build_options(cfg: &WorkerConfig, req: &SendRequest, provider: Option) -> TurnOptions { let opts = req.options.clone().unwrap_or_default(); // Code mode without an explicit policy gets the curated coding surface // (natively exposed); an explicit caller policy always wins. @@ -245,12 +271,12 @@ fn build_options(cfg: &WorkerConfig, req: &SendRequest) -> TurnOptions { }; TurnOptions { model: req.model.clone(), - provider: req.provider.clone(), + provider: provider.clone(), system_prompt: prompt::resolve_system_prompt( opts.system_prompt, opts.system_prompt_strategy, opts.mode, - req.provider.as_deref(), + provider.as_deref(), ), mode: opts.mode, max_turns: opts.max_turns.unwrap_or(cfg.default_max_turns), @@ -370,7 +396,7 @@ mod tests { ..Default::default() }), }; - let opts = build_options(&cfg, &req); + let opts = build_options(&cfg, &req, req.provider.clone()); let prompt = opts.system_prompt.expect("built-in prompt"); assert!(prompt.contains("operating in agent mode")); assert!(prompt.contains("IMPORTANT: NEVER invent function ids")); @@ -391,7 +417,7 @@ mod tests { ..Default::default() }), }; - let opts = build_options(&cfg, &req); + let opts = build_options(&cfg, &req, req.provider.clone()); let functions = opts.functions.expect("code mode defaults a policy"); assert_eq!( functions.expose, @@ -407,6 +433,33 @@ mod tests { assert!(!prompt.contains("NEVER invent function ids")); } + #[test] + fn build_options_code_mode_uses_resolved_provider_for_family() { + // The caller omitted `provider`; start() resolves it from the router + // catalog and passes it here — the GPT family must get the + // apply_patch code identity. + let cfg = WorkerConfig::default(); + let req = SendRequest { + session_id: None, + message: MessageInput::Text("hi".into()), + model: "codex/gpt-5.4".into(), + provider: None, + idempotency_key: None, + session: None, + options: Some(SendOptions { + mode: Some(Mode::Code), + ..Default::default() + }), + }; + let opts = build_options(&cfg, &req, Some("openai-codex".into())); + assert_eq!(opts.provider.as_deref(), Some("openai-codex")); + let prompt = opts.system_prompt.expect("built-in prompt"); + assert!( + prompt.contains("coder::apply-patch"), + "GPT family gets the apply_patch discipline" + ); + } + #[test] fn build_options_code_mode_explicit_policy_wins() { let cfg = WorkerConfig::default(); @@ -427,7 +480,7 @@ mod tests { ..Default::default() }), }; - let opts = build_options(&cfg, &req); + let opts = build_options(&cfg, &req, req.provider.clone()); let functions = opts.functions.unwrap(); assert_eq!(functions.allow, vec!["coder::read-file".to_string()]); assert_eq!( @@ -453,7 +506,7 @@ mod tests { ..Default::default() }), }; - let opts = build_options(&cfg, &req); + let opts = build_options(&cfg, &req, req.provider.clone()); assert_eq!(opts.system_prompt.as_deref(), Some("custom")); } @@ -473,7 +526,7 @@ mod tests { ..Default::default() }), }; - let opts = build_options(&cfg, &req); + let opts = build_options(&cfg, &req, req.provider.clone()); let prompt = opts.system_prompt.expect("enriched prompt"); assert!(prompt.contains("IMPORTANT: NEVER invent function ids")); assert!(prompt.ends_with("Speak only in haiku.")); diff --git a/harness/src/subagent.rs b/harness/src/subagent.rs index 96130d416..d1cec668c 100644 --- a/harness/src/subagent.rs +++ b/harness/src/subagent.rs @@ -257,10 +257,16 @@ async fn seed_child_with_root( .clone() .or_else(|| parent_record.map(|p| p.options.model.clone())) .ok_or_else(|| HarnessError::InvalidRequest("spawn requires a model".into()))?; - let provider = req + let provider = match req .provider .clone() - .or_else(|| parent_record.and_then(|p| p.options.provider.clone())); + .or_else(|| parent_record.and_then(|p| p.options.provider.clone())) + { + Some(p) => Some(p), + // No caller/parent provider: resolve from the router catalog so the + // child's prompt family matches where the model actually routes. + None => crate::functions::send::resolve_provider(deps, &model).await, + }; // Code mode without an explicit policy requests the curated coding // surface; with a parent it still narrows through subset_policy below, diff --git a/provider-openai-codex/src/sse.rs b/provider-openai-codex/src/sse.rs index 0b7c2e50b..17c4e2b27 100644 --- a/provider-openai-codex/src/sse.rs +++ b/provider-openai-codex/src/sse.rs @@ -22,6 +22,9 @@ struct PartialToolCall { id: String, function_id: String, args_json: String, + /// Freeform custom-tool call: `args_json` holds RAW text (the V4A + /// patch), not JSON — materialized as `{ "patch": }`. + raw_input: bool, } pub struct PartialState { @@ -95,7 +98,10 @@ fn build_content(state: &PartialState) -> Vec { if tc.function_id.is_empty() { continue; } - let arguments = if tc.args_json.is_empty() { + let arguments = if tc.raw_input { + // Freeform custom tool (apply_patch): the buffer IS the patch. + serde_json::json!({ "patch": tc.args_json }) + } else if tc.args_json.is_empty() { serde_json::json!({}) } else { serde_json::from_str(&tc.args_json).unwrap_or(Value::Null) @@ -258,7 +264,8 @@ pub fn handle_chunk( let Some(item) = chunk.get("item").or_else(|| chunk.get("output_item")) else { return events; }; - if item.get("type").and_then(Value::as_str) == Some("function_call") { + let item_type = item.get("type").and_then(Value::as_str); + if item_type == Some("function_call") || item_type == Some("custom_tool_call") { let id = item .get("call_id") .or_else(|| item.get("id")) @@ -270,10 +277,22 @@ pub fn handle_chunk( .and_then(Value::as_str) .map(decode_tool_name) .unwrap_or_default(); + let raw_input = item_type == Some("custom_tool_call"); + // A custom_tool_call item may arrive with its full input + // attached (no delta stream) — seed the buffer from it. + let args_json = if raw_input { + item.get("input") + .and_then(Value::as_str) + .unwrap_or("") + .to_string() + } else { + String::new() + }; state.tool_calls.push(PartialToolCall { id, function_id: fname, - args_json: String::new(), + args_json, + raw_input, }); close_open_block(state, model, &mut events); state.open_block = Some(OpenBlock::Call); @@ -306,6 +325,27 @@ pub fn handle_chunk( delta, }); } + n if n.contains("custom_tool_call_input.delta") => { + let delta = str_delta(chunk); + if delta.is_empty() { + return events; + } + if let Some(last) = state.tool_calls.last_mut() { + last.args_json.push_str(&delta); + } + events.push(AssistantMessageEvent::FunctioncallDelta { + partial: build_partial(state, model), + delta, + }); + } + n if n.contains("custom_tool_call_input.done") => { + // Authoritative full input: replace whatever the deltas built. + if let Some(input) = chunk.get("input").and_then(Value::as_str) { + if let Some(last) = state.tool_calls.last_mut() { + last.args_json = input.to_string(); + } + } + } n if n.contains("function_call_arguments.delta") => { let delta = chunk .get("delta") @@ -434,6 +474,58 @@ mod tests { } } + #[test] + fn custom_tool_call_streams_raw_patch_text() { + let (_, events) = run(&[ + json!({ "type": "response.output_item.added", "item": { "type": "custom_tool_call", "call_id": "c7", "name": "apply_patch" } }), + json!({ "type": "response.custom_tool_call_input.delta", "delta": "*** Begin Patch\n*** Update File: a.py\n" }), + json!({ "type": "response.custom_tool_call_input.delta", "delta": "@@\n-x = 1\n+x = 2\n*** End Patch" }), + json!({ "type": "response.completed" }), + ]); + match events.last().unwrap() { + AssistantMessageEvent::Done { message } => match &message.content[0] { + ContentBlock::FunctionCall { + id, + function_id, + arguments, + } => { + assert_eq!(id, "c7"); + assert_eq!( + function_id, "coder::apply-patch", + "alias decodes to the bus id" + ); + let patch = arguments["patch"].as_str().unwrap(); + assert!(patch.starts_with("*** Begin Patch")); + assert!(patch.ends_with("*** End Patch")); + assert!( + patch.contains("+x = 2"), + "raw text, no JSON escaping applied" + ); + } + other => panic!("want function_call, got {other:?}"), + }, + other => panic!("want done, got {other:?}"), + } + } + + #[test] + fn custom_tool_call_done_input_is_authoritative() { + let (_, events) = run(&[ + json!({ "type": "response.output_item.added", "item": { "type": "custom_tool_call", "call_id": "c8", "name": "apply_patch", "input": "partial" } }), + json!({ "type": "response.custom_tool_call_input.done", "input": "*** Begin Patch\n*** End Patch" }), + json!({ "type": "response.completed" }), + ]); + match events.last().unwrap() { + AssistantMessageEvent::Done { message } => match &message.content[0] { + ContentBlock::FunctionCall { arguments, .. } => { + assert_eq!(arguments["patch"], "*** Begin Patch\n*** End Patch"); + } + other => panic!("want function_call, got {other:?}"), + }, + other => panic!("want done, got {other:?}"), + } + } + #[test] fn unknown_events_are_ignored() { let (_, events) = run(&[json!({ "type": "response.some_future_event", "x": 1 })]); diff --git a/provider-openai-codex/src/wire/names.rs b/provider-openai-codex/src/wire/names.rs index 7bc372f89..1ab9959d8 100644 --- a/provider-openai-codex/src/wire/names.rs +++ b/provider-openai-codex/src/wire/names.rs @@ -1,11 +1,27 @@ //! iii function ids ↔ OpenAI function names. OpenAI enforces //! `^[a-zA-Z0-9_-]{1,64}$`; bus ids use `::` separators. +//! +//! One deliberate alias: `coder::apply-patch` travels as `apply_patch` — +//! the EXACT tool name codex models are trained on (it is also emitted as +//! a freeform `custom` tool; see `wire::tools` and the sse custom-input +//! handling). + +/// The wire name codex models know for the V4A patch tool. +pub const APPLY_PATCH_WIRE: &str = "apply_patch"; +/// The bus function the alias maps to. +pub const APPLY_PATCH_FN: &str = "coder::apply-patch"; pub fn encode_tool_name(name: &str) -> String { + if name == APPLY_PATCH_FN { + return APPLY_PATCH_WIRE.to_string(); + } name.replace("::", "__") } pub fn decode_tool_name(name: &str) -> String { + if name == APPLY_PATCH_WIRE { + return APPLY_PATCH_FN.to_string(); + } name.replace("__", "::") } @@ -25,4 +41,15 @@ mod tests { assert_eq!(encode_tool_name("submit_result"), "submit_result"); assert_eq!(decode_tool_name("submit_result"), "submit_result"); } + + #[test] + fn apply_patch_aliases_both_directions() { + assert_eq!(encode_tool_name("coder::apply-patch"), "apply_patch"); + assert_eq!(decode_tool_name("apply_patch"), "coder::apply-patch"); + // The generic codec must not double-map the alias. + assert_eq!( + decode_tool_name(&encode_tool_name("coder::apply-patch")), + "coder::apply-patch" + ); + } } diff --git a/provider-openai-codex/src/wire/tools.rs b/provider-openai-codex/src/wire/tools.rs index fcfe1c3e6..850b3b209 100644 --- a/provider-openai-codex/src/wire/tools.rs +++ b/provider-openai-codex/src/wire/tools.rs @@ -9,6 +9,16 @@ pub fn functions_to_wire(tools: &[AgentFunction]) -> Vec { tools .iter() .map(|t| { + // The V4A patch tool goes out as a FREEFORM custom tool named + // exactly `apply_patch`: its input is the raw patch text codex + // models are trained to emit — no JSON escaping of the patch. + if t.name == crate::wire::names::APPLY_PATCH_FN { + return json!({ + "type": "custom", + "name": crate::wire::names::APPLY_PATCH_WIRE, + "description": t.description, + }); + } json!({ "type": "function", "name": encode_tool_name(&t.name), @@ -47,4 +57,22 @@ mod tests { fn empty_input_yields_empty_array() { assert!(functions_to_wire(&[]).is_empty()); } + + #[test] + fn apply_patch_goes_out_as_freeform_custom_tool() { + let tools = vec![AgentFunction { + name: "coder::apply-patch".into(), + description: "Apply a V4A patch".into(), + parameters: json!({ "type": "object" }), + label: None, + execution_mode: None, + }]; + let wire = functions_to_wire(&tools); + assert_eq!(wire[0]["type"], "custom"); + assert_eq!(wire[0]["name"], "apply_patch"); + assert!( + wire[0].get("parameters").is_none(), + "custom tools carry no JSON schema" + ); + } }