mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Delete function-style apply_patch (#21651)
## Why `apply_patch` is now a freeform/custom tool. Keeping the old JSON/function-style registration and parsing path left another way for models and tests to invoke `apply_patch`, which made the tool surface harder to reason about. ## What changed - Removed the `ApplyPatchToolType::Function` variant, JSON `apply_patch` spec, and handler support for function payloads. - Kept `apply_patch_tool_type = freeform` as the supported model metadata path, including Bedrock catalog metadata. - Migrated `apply_patch` tests and SSE fixtures to custom/freeform tool calls. ## Verification - `cargo test -p codex-tools -p codex-protocol -p codex-model-provider` - `cargo test -p codex-core tools::handlers::apply_patch --lib` - `cargo test -p codex-core --test all apply_patch_tool_executes_and_emits_patch_events` - `cargo test -p codex-core --test all apply_patch_reports_parse_diagnostics` - `cargo test -p codex-exec test_apply_patch_tool` - `just fix -p codex-core` - `just fix -p codex-tools -p codex-protocol -p codex-model-provider -p codex-exec`
This commit is contained in:
committed by
GitHub
Unverified
parent
cf941ede15
commit
e783341b70
@@ -21,10 +21,7 @@ use crate::tools::context::ToolPayload;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
use crate::tools::events::ToolEventCtx;
|
||||
use crate::tools::handlers::apply_granted_turn_permissions;
|
||||
use crate::tools::handlers::apply_patch_spec::ApplyPatchToolArgs;
|
||||
use crate::tools::handlers::apply_patch_spec::create_apply_patch_freeform_tool;
|
||||
use crate::tools::handlers::apply_patch_spec::create_apply_patch_json_tool;
|
||||
use crate::tools::handlers::parse_arguments;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::orchestrator::ToolOrchestrator;
|
||||
use crate::tools::registry::PostToolUsePayload;
|
||||
@@ -43,7 +40,6 @@ use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::openai_models::ApplyPatchToolType;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::PatchApplyUpdatedEvent;
|
||||
@@ -56,25 +52,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
const APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL: Duration = Duration::from_millis(500);
|
||||
|
||||
pub struct ApplyPatchHandler {
|
||||
options: ApplyPatchToolType,
|
||||
}
|
||||
|
||||
impl Default for ApplyPatchHandler {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
options: ApplyPatchToolType::Freeform,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ApplyPatchHandler {
|
||||
pub(crate) fn new(apply_patch_tool_type: ApplyPatchToolType) -> Self {
|
||||
Self {
|
||||
options: apply_patch_tool_type,
|
||||
}
|
||||
}
|
||||
}
|
||||
pub struct ApplyPatchHandler;
|
||||
|
||||
#[derive(Default)]
|
||||
struct ApplyPatchArgumentDiffConsumer {
|
||||
@@ -264,15 +242,8 @@ fn write_permissions_for_paths(
|
||||
}
|
||||
|
||||
/// Extracts the raw patch text used as the command-shaped hook input for apply_patch.
|
||||
///
|
||||
/// The apply_patch tool can arrive as the older JSON/function shape or as a
|
||||
/// freeform custom tool call. Both represent the same file edit operation, so
|
||||
/// hooks see the raw patch body in `tool_input.command` either way.
|
||||
fn apply_patch_payload_command(payload: &ToolPayload) -> Option<String> {
|
||||
match payload {
|
||||
ToolPayload::Function { arguments } => parse_arguments::<ApplyPatchToolArgs>(arguments)
|
||||
.ok()
|
||||
.map(|args| args.input),
|
||||
ToolPayload::Custom { input } => Some(input.clone()),
|
||||
_ => None,
|
||||
}
|
||||
@@ -320,10 +291,7 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
}
|
||||
|
||||
fn spec(&self) -> Option<ToolSpec> {
|
||||
Some(match self.options {
|
||||
ApplyPatchToolType::Freeform => create_apply_patch_freeform_tool(),
|
||||
ApplyPatchToolType::Function => create_apply_patch_json_tool(),
|
||||
})
|
||||
Some(create_apply_patch_freeform_tool())
|
||||
}
|
||||
|
||||
fn kind(&self) -> ToolKind {
|
||||
@@ -331,10 +299,7 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
}
|
||||
|
||||
fn matches_kind(&self, payload: &ToolPayload) -> bool {
|
||||
matches!(
|
||||
payload,
|
||||
ToolPayload::Function { .. } | ToolPayload::Custom { .. }
|
||||
)
|
||||
matches!(payload, ToolPayload::Custom { .. })
|
||||
}
|
||||
|
||||
async fn is_mutating(&self, _invocation: &ToolInvocation) -> bool {
|
||||
@@ -380,17 +345,10 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
..
|
||||
} = invocation;
|
||||
|
||||
let patch_input = match payload {
|
||||
ToolPayload::Function { arguments } => {
|
||||
let args: ApplyPatchToolArgs = parse_arguments(&arguments)?;
|
||||
args.input
|
||||
}
|
||||
ToolPayload::Custom { input } => input,
|
||||
_ => {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"apply_patch handler received unsupported payload".to_string(),
|
||||
));
|
||||
}
|
||||
let ToolPayload::Custom { input: patch_input } = payload else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"apply_patch handler received unsupported payload".to_string(),
|
||||
));
|
||||
};
|
||||
|
||||
// Re-parse and verify the patch so we can compute changes and approval.
|
||||
|
||||
@@ -1,89 +1,9 @@
|
||||
use codex_tools::FreeformTool;
|
||||
use codex_tools::FreeformToolFormat;
|
||||
use codex_tools::JsonSchema;
|
||||
use codex_tools::ResponsesApiTool;
|
||||
use codex_tools::ToolSpec;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
const APPLY_PATCH_LARK_GRAMMAR: &str = include_str!("apply_patch.lark");
|
||||
|
||||
const APPLY_PATCH_JSON_TOOL_DESCRIPTION: &str = r#"Use the `apply_patch` tool to edit files.
|
||||
Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope:
|
||||
|
||||
*** Begin Patch
|
||||
[ one or more file sections ]
|
||||
*** End Patch
|
||||
|
||||
Within that envelope, you get a sequence of file operations.
|
||||
You MUST include a header to specify the action you are taking.
|
||||
Each operation starts with one of three headers:
|
||||
|
||||
*** Add File: <path> - create a new file. Every following line is a + line (the initial contents).
|
||||
*** Delete File: <path> - remove an existing file. Nothing follows.
|
||||
*** Update File: <path> - patch an existing file in place (optionally with a rename).
|
||||
|
||||
May be immediately followed by *** Move to: <new path> if you want to rename the file.
|
||||
Then one or more “hunks”, each introduced by @@ (optionally followed by a hunk header).
|
||||
Within a hunk each line starts with:
|
||||
|
||||
For instructions on [context_before] and [context_after]:
|
||||
- By default, show 3 lines of code immediately above and 3 lines immediately below each change. If a change is within 3 lines of a previous change, do NOT duplicate the first change’s [context_after] lines in the second change’s [context_before] lines.
|
||||
- If 3 lines of context is insufficient to uniquely identify the snippet of code within the file, use the @@ operator to indicate the class or function to which the snippet belongs. For instance, we might have:
|
||||
@@ class BaseClass
|
||||
[3 lines of pre-context]
|
||||
- [old_code]
|
||||
+ [new_code]
|
||||
[3 lines of post-context]
|
||||
|
||||
- If a code block is repeated so many times in a class or function such that even a single `@@` statement and 3 lines of context cannot uniquely identify the snippet of code, you can use multiple `@@` statements to jump to the right context. For instance:
|
||||
|
||||
@@ class BaseClass
|
||||
@@ def method():
|
||||
[3 lines of pre-context]
|
||||
- [old_code]
|
||||
+ [new_code]
|
||||
[3 lines of post-context]
|
||||
|
||||
The full grammar definition is below:
|
||||
Patch := Begin { FileOp } End
|
||||
Begin := "*** Begin Patch" NEWLINE
|
||||
End := "*** End Patch" NEWLINE
|
||||
FileOp := AddFile | DeleteFile | UpdateFile
|
||||
AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE }
|
||||
DeleteFile := "*** Delete File: " path NEWLINE
|
||||
UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk }
|
||||
MoveTo := "*** Move to: " newPath NEWLINE
|
||||
Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ]
|
||||
HunkLine := (" " | "-" | "+") text NEWLINE
|
||||
|
||||
A full patch can combine several operations:
|
||||
|
||||
*** Begin Patch
|
||||
*** Add File: hello.txt
|
||||
+Hello world
|
||||
*** Update File: src/app.py
|
||||
*** Move to: src/main.py
|
||||
@@ def greet():
|
||||
-print("Hi")
|
||||
+print("Hello, world!")
|
||||
*** Delete File: obsolete.txt
|
||||
*** End Patch
|
||||
|
||||
It is important to remember:
|
||||
|
||||
- You must include a header with your intended action (Add/Delete/Update)
|
||||
- You must prefix new lines with `+` even when creating a new file
|
||||
- File references can only be relative, NEVER ABSOLUTE.
|
||||
"#;
|
||||
|
||||
/// TODO(dylan): deprecate once we get rid of json tool
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct ApplyPatchToolArgs {
|
||||
pub input: String,
|
||||
}
|
||||
|
||||
/// Returns a custom tool that can be used to edit files. Well-suited for GPT-5 models
|
||||
/// https://platform.openai.com/docs/guides/function-calling#custom-tools
|
||||
pub fn create_apply_patch_freeform_tool() -> ToolSpec {
|
||||
@@ -98,29 +18,6 @@ pub fn create_apply_patch_freeform_tool() -> ToolSpec {
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns a json tool that can be used to edit files. Should only be used with gpt-oss models
|
||||
pub fn create_apply_patch_json_tool() -> ToolSpec {
|
||||
let properties = BTreeMap::from([(
|
||||
"input".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"The entire contents of the apply_patch command".to_string(),
|
||||
)),
|
||||
)]);
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "apply_patch".to_string(),
|
||||
description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(),
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(
|
||||
properties,
|
||||
Some(vec!["input".to_string()]),
|
||||
Some(false.into()),
|
||||
),
|
||||
output_schema: None,
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "apply_patch_spec_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
use super::*;
|
||||
use codex_tools::JsonSchema;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
#[test]
|
||||
fn create_apply_patch_freeform_tool_matches_expected_spec() {
|
||||
@@ -20,27 +18,3 @@ fn create_apply_patch_freeform_tool_matches_expected_spec() {
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_apply_patch_json_tool_matches_expected_spec() {
|
||||
assert_eq!(
|
||||
create_apply_patch_json_tool(),
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "apply_patch".to_string(),
|
||||
description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(),
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(
|
||||
BTreeMap::from([(
|
||||
"input".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"The entire contents of the apply_patch command".to_string(),
|
||||
),),
|
||||
)]),
|
||||
Some(vec!["input".to_string()]),
|
||||
Some(false.into())
|
||||
),
|
||||
output_schema: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -42,24 +42,6 @@ async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pre_tool_use_payload_uses_json_patch_input() {
|
||||
let patch = sample_patch();
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: json!({ "input": patch }).to_string(),
|
||||
};
|
||||
let invocation = invocation_for_payload(payload).await;
|
||||
let handler = ApplyPatchHandler::default();
|
||||
|
||||
assert_eq!(
|
||||
handler.pre_tool_use_payload(&invocation),
|
||||
Some(PreToolUsePayload {
|
||||
tool_name: HookToolName::apply_patch(),
|
||||
tool_input: json!({ "command": patch }),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pre_tool_use_payload_uses_freeform_patch_input() {
|
||||
let patch = sample_patch();
|
||||
@@ -67,7 +49,7 @@ async fn pre_tool_use_payload_uses_freeform_patch_input() {
|
||||
input: patch.to_string(),
|
||||
};
|
||||
let invocation = invocation_for_payload(payload).await;
|
||||
let handler = ApplyPatchHandler::default();
|
||||
let handler = ApplyPatchHandler;
|
||||
|
||||
assert_eq!(
|
||||
handler.pre_tool_use_payload(&invocation),
|
||||
@@ -86,7 +68,7 @@ async fn post_tool_use_payload_uses_patch_input_and_tool_output() {
|
||||
};
|
||||
let invocation = invocation_for_payload(payload).await;
|
||||
let output = ApplyPatchToolOutput::from_text("Success. Updated files.".to_string());
|
||||
let handler = ApplyPatchHandler::default();
|
||||
let handler = ApplyPatchHandler;
|
||||
|
||||
assert_eq!(
|
||||
handler.post_tool_use_payload(&invocation, &output),
|
||||
@@ -99,24 +81,6 @@ async fn post_tool_use_payload_uses_patch_input_and_tool_output() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_consumer_does_not_stream_json_tool_call_arguments() {
|
||||
let mut consumer = ApplyPatchArgumentDiffConsumer::default();
|
||||
assert!(
|
||||
consumer
|
||||
.push_delta("call-1".to_string(), r#"{"input":"*** Begin Patch\n"#)
|
||||
.is_none()
|
||||
);
|
||||
assert!(
|
||||
consumer
|
||||
.push_delta(
|
||||
"call-1".to_string(),
|
||||
r#"*** Add File: hello.txt\n+hello\n*** End Patch\n"}"#
|
||||
)
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_consumer_streams_apply_patch_changes() {
|
||||
let mut consumer = ApplyPatchArgumentDiffConsumer::default();
|
||||
|
||||
@@ -257,12 +257,8 @@ pub fn build_tool_registry_builder(
|
||||
)));
|
||||
}
|
||||
|
||||
if config.environment_mode.has_environment()
|
||||
&& let Some(apply_patch_tool_type) = &config.apply_patch_tool_type
|
||||
{
|
||||
builder.register_handler(Arc::new(ApplyPatchHandler::new(
|
||||
apply_patch_tool_type.clone(),
|
||||
)));
|
||||
if config.environment_mode.has_environment() && config.apply_patch_tool_type.is_some() {
|
||||
builder.register_handler(Arc::new(ApplyPatchHandler));
|
||||
}
|
||||
|
||||
if config
|
||||
|
||||
@@ -890,7 +890,6 @@ pub fn ev_apply_patch_call(
|
||||
) -> Value {
|
||||
match output_type {
|
||||
ApplyPatchModelOutput::Freeform => ev_apply_patch_custom_tool_call(call_id, patch),
|
||||
ApplyPatchModelOutput::Function => ev_apply_patch_function_call(call_id, patch),
|
||||
ApplyPatchModelOutput::Shell => ev_apply_patch_shell_call(call_id, patch),
|
||||
ApplyPatchModelOutput::ShellViaHeredoc => {
|
||||
ev_apply_patch_shell_call_via_heredoc(call_id, patch)
|
||||
@@ -903,7 +902,7 @@ pub fn ev_apply_patch_call(
|
||||
|
||||
/// Convenience: SSE event for an `apply_patch` custom tool call with raw patch
|
||||
/// text. This mirrors the payload produced by the Responses API when the model
|
||||
/// invokes `apply_patch` directly (before we convert it to a function call).
|
||||
/// invokes `apply_patch` directly.
|
||||
pub fn ev_apply_patch_custom_tool_call(call_id: &str, patch: &str) -> Value {
|
||||
serde_json::json!({
|
||||
"type": "response.output_item.done",
|
||||
@@ -916,24 +915,6 @@ pub fn ev_apply_patch_custom_tool_call(call_id: &str, patch: &str) -> Value {
|
||||
})
|
||||
}
|
||||
|
||||
/// Convenience: SSE event for an `apply_patch` function call. The Responses API
|
||||
/// wraps the patch content in a JSON string under the `input` key; we recreate
|
||||
/// the same structure so downstream code exercises the full parsing path.
|
||||
pub fn ev_apply_patch_function_call(call_id: &str, patch: &str) -> Value {
|
||||
let arguments = serde_json::json!({ "input": patch });
|
||||
let arguments = serde_json::to_string(&arguments).expect("serialize apply_patch arguments");
|
||||
|
||||
serde_json::json!({
|
||||
"type": "response.output_item.done",
|
||||
"item": {
|
||||
"type": "function_call",
|
||||
"name": "apply_patch",
|
||||
"arguments": arguments,
|
||||
"call_id": call_id
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
pub fn ev_shell_command_call(call_id: &str, command: &str) -> Value {
|
||||
let args = serde_json::json!({ "command": command });
|
||||
ev_shell_command_call_with_args(call_id, &args)
|
||||
|
||||
@@ -188,7 +188,6 @@ fn docker_command_capture_stdout<const N: usize>(args: [&str; N]) -> Result<Stri
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
|
||||
pub enum ApplyPatchModelOutput {
|
||||
Freeform,
|
||||
Function,
|
||||
Shell,
|
||||
ShellViaHeredoc,
|
||||
ShellCommandViaHeredoc,
|
||||
@@ -951,8 +950,7 @@ impl TestCodexHarness {
|
||||
ApplyPatchModelOutput::Freeform => {
|
||||
Box::pin(self.custom_tool_call_output(call_id)).await
|
||||
}
|
||||
ApplyPatchModelOutput::Function
|
||||
| ApplyPatchModelOutput::Shell
|
||||
ApplyPatchModelOutput::Shell
|
||||
| ApplyPatchModelOutput::ShellViaHeredoc
|
||||
| ApplyPatchModelOutput::ShellCommandViaHeredoc => {
|
||||
Box::pin(self.function_call_stdout(call_id)).await
|
||||
|
||||
@@ -24,7 +24,6 @@ use codex_protocol::user_input::UserInput;
|
||||
#[cfg(target_os = "linux")]
|
||||
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
|
||||
use core_test_support::assert_regex_match;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
@@ -172,14 +171,14 @@ async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() -
|
||||
call_id,
|
||||
patch,
|
||||
"done",
|
||||
ApplyPatchModelOutput::Function,
|
||||
ApplyPatchModelOutput::Freeform,
|
||||
)
|
||||
.await;
|
||||
|
||||
harness.submit("please apply helper alias patch").await?;
|
||||
|
||||
let out = harness
|
||||
.apply_patch_output(call_id, ApplyPatchModelOutput::Function)
|
||||
.apply_patch_output(call_id, ApplyPatchModelOutput::Freeform)
|
||||
.await;
|
||||
assert_regex_match(
|
||||
r"(?s)^Exit code: 0.*Success\. Updated the following files:\nA helper-alias\.txt\n?$",
|
||||
@@ -192,7 +191,6 @@ async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() -
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
async fn apply_patch_cli_multiple_operations_integration(
|
||||
@@ -237,7 +235,6 @@ D delete.txt
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -265,7 +262,6 @@ async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) ->
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -294,7 +290,6 @@ async fn apply_patch_cli_moves_file_to_new_directory(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -323,7 +318,6 @@ async fn apply_patch_cli_updates_file_appends_trailing_newline(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -353,7 +347,6 @@ async fn apply_patch_cli_insert_only_hunk_modifies_file(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -385,7 +378,6 @@ async fn apply_patch_cli_move_overwrites_existing_destination(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -425,7 +417,6 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -453,7 +444,6 @@ async fn apply_patch_cli_add_overwrites_existing_file(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -485,7 +475,6 @@ async fn apply_patch_cli_rejects_invalid_hunk_header(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -521,7 +510,6 @@ async fn apply_patch_cli_reports_missing_context(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -557,7 +545,6 @@ async fn apply_patch_cli_reports_missing_target_file(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -594,7 +581,6 @@ async fn apply_patch_cli_delete_missing_file_reports_error(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -619,7 +605,6 @@ async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -646,7 +631,6 @@ async fn apply_patch_cli_delete_directory_reports_verification_error(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -693,7 +677,6 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -743,7 +726,6 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -1171,7 +1153,7 @@ async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nest
|
||||
call_id,
|
||||
patch,
|
||||
"updated repo-relative path",
|
||||
ApplyPatchModelOutput::Function,
|
||||
ApplyPatchModelOutput::Freeform,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1260,7 +1242,7 @@ async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() ->
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch(
|
||||
async fn apply_patch_shell_accepts_lenient_heredoc_wrapped_patch(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -1281,7 +1263,6 @@ async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -1303,7 +1284,6 @@ async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput)
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -1340,7 +1320,6 @@ async fn apply_patch_cli_missing_second_chunk_context_rejected(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
@@ -1394,12 +1373,12 @@ async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()>
|
||||
|
||||
let s1 = sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call1, patch1),
|
||||
ev_apply_patch_custom_tool_call(call1, patch1),
|
||||
ev_completed("resp-1"),
|
||||
]);
|
||||
let s2 = sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_apply_patch_function_call(call2, patch2),
|
||||
ev_apply_patch_custom_tool_call(call2, patch2),
|
||||
ev_completed("resp-2"),
|
||||
]);
|
||||
let s3 = sse(vec![
|
||||
@@ -1446,12 +1425,12 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result
|
||||
let responses = vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_success, patch_success),
|
||||
ev_apply_patch_custom_tool_call(call_success, patch_success),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_apply_patch_function_call(call_failure, patch_failure),
|
||||
ev_apply_patch_custom_tool_call(call_failure, patch_failure),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
sse(vec![
|
||||
@@ -1488,7 +1467,7 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result
|
||||
"diff should include contents from successful patch: {diff}"
|
||||
);
|
||||
|
||||
let failure_out = harness.function_call_stdout(call_failure).await;
|
||||
let failure_out = harness.custom_tool_call_output(call_failure).await;
|
||||
assert!(
|
||||
failure_out.contains("apply_patch verification failed"),
|
||||
"expected verification failure output: {failure_out}"
|
||||
@@ -1529,12 +1508,12 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()>
|
||||
let responses = vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_success, patch_success),
|
||||
ev_apply_patch_custom_tool_call(call_success, patch_success),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_apply_patch_function_call(call_inexact, patch_inexact),
|
||||
ev_apply_patch_custom_tool_call(call_inexact, patch_inexact),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
sse(vec![
|
||||
@@ -1573,7 +1552,6 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()>
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
|
||||
@@ -20,7 +20,7 @@ use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::managed_network_requirements_loader;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
@@ -108,7 +108,7 @@ enum ActionKind {
|
||||
command: &'static str,
|
||||
justification: Option<&'static str>,
|
||||
},
|
||||
ApplyPatchFunction {
|
||||
ApplyPatchFreeform {
|
||||
target: TargetPath,
|
||||
content: &'static str,
|
||||
},
|
||||
@@ -131,7 +131,7 @@ impl ActionKind {
|
||||
| ActionKind::RunCommand { .. }
|
||||
| ActionKind::RunCommandWithPrefixRule { .. }
|
||||
| ActionKind::RunUnifiedExecCommand { .. }
|
||||
| ActionKind::ApplyPatchFunction { .. }
|
||||
| ActionKind::ApplyPatchFreeform { .. }
|
||||
| ActionKind::ApplyPatchShell { .. } => None,
|
||||
}
|
||||
}
|
||||
@@ -264,11 +264,11 @@ impl ActionKind {
|
||||
)?;
|
||||
Ok((event, Some(command.to_string())))
|
||||
}
|
||||
ActionKind::ApplyPatchFunction { target, content } => {
|
||||
ActionKind::ApplyPatchFreeform { target, content } => {
|
||||
let (path, patch_path) = target.resolve_for_patch(test);
|
||||
let _ = fs::remove_file(&path);
|
||||
let patch = build_add_file_patch(&patch_path, content);
|
||||
Ok((ev_apply_patch_function_call(call_id, &patch), None))
|
||||
Ok((ev_apply_patch_custom_tool_call(call_id, &patch), None))
|
||||
}
|
||||
ActionKind::ApplyPatchShell { target, content } => {
|
||||
let (path, patch_path) = target.resolve_for_patch(test);
|
||||
@@ -1342,46 +1342,46 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "apply_patch_function_auto_inside_workspace",
|
||||
name: "apply_patch_freeform_auto_inside_workspace",
|
||||
approval_policy: OnRequest,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
action: ActionKind::ApplyPatchFunction {
|
||||
target: TargetPath::Workspace("apply_patch_function.txt"),
|
||||
content: "function-apply-patch",
|
||||
action: ActionKind::ApplyPatchFreeform {
|
||||
target: TargetPath::Workspace("apply_patch_freeform.txt"),
|
||||
content: "freeform-apply-patch",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
features: vec![],
|
||||
model_override: Some("gpt-5.4"),
|
||||
outcome: Outcome::Auto,
|
||||
expectation: Expectation::PatchApplied {
|
||||
target: TargetPath::Workspace("apply_patch_function.txt"),
|
||||
content: "function-apply-patch",
|
||||
target: TargetPath::Workspace("apply_patch_freeform.txt"),
|
||||
content: "freeform-apply-patch",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "apply_patch_function_danger_allows_outside_workspace",
|
||||
name: "apply_patch_freeform_danger_allows_outside_workspace",
|
||||
approval_policy: OnRequest,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
action: ActionKind::ApplyPatchFunction {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_danger.txt"),
|
||||
content: "function-patch-danger",
|
||||
action: ActionKind::ApplyPatchFreeform {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_danger.txt"),
|
||||
content: "freeform-patch-danger",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
features: vec![Feature::ApplyPatchFreeform],
|
||||
model_override: Some("gpt-5.4"),
|
||||
outcome: Outcome::Auto,
|
||||
expectation: Expectation::PatchApplied {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_danger.txt"),
|
||||
content: "function-patch-danger",
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_danger.txt"),
|
||||
content: "freeform-patch-danger",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "apply_patch_function_outside_requires_patch_approval",
|
||||
name: "apply_patch_freeform_outside_requires_patch_approval",
|
||||
approval_policy: OnRequest,
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::ApplyPatchFunction {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_outside.txt"),
|
||||
content: "function-patch-outside",
|
||||
action: ActionKind::ApplyPatchFreeform {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside.txt"),
|
||||
content: "freeform-patch-outside",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
features: vec![],
|
||||
@@ -1391,17 +1391,17 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
expected_reason: None,
|
||||
},
|
||||
expectation: Expectation::PatchApplied {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_outside.txt"),
|
||||
content: "function-patch-outside",
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside.txt"),
|
||||
content: "freeform-patch-outside",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "apply_patch_function_outside_denied_blocks_patch",
|
||||
name: "apply_patch_freeform_outside_denied_blocks_patch",
|
||||
approval_policy: OnRequest,
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::ApplyPatchFunction {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_outside_denied.txt"),
|
||||
content: "function-patch-outside-denied",
|
||||
action: ActionKind::ApplyPatchFreeform {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside_denied.txt"),
|
||||
content: "freeform-patch-outside-denied",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
features: vec![],
|
||||
@@ -1411,7 +1411,7 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
expected_reason: None,
|
||||
},
|
||||
expectation: Expectation::FileNotCreated {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_outside_denied.txt"),
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside_denied.txt"),
|
||||
message_contains: &["patch rejected by user"],
|
||||
},
|
||||
},
|
||||
@@ -1436,12 +1436,12 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "apply_patch_function_unless_trusted_requires_patch_approval",
|
||||
name: "apply_patch_freeform_unless_trusted_requires_patch_approval",
|
||||
approval_policy: UnlessTrusted,
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::ApplyPatchFunction {
|
||||
target: TargetPath::Workspace("apply_patch_function_unless_trusted.txt"),
|
||||
content: "function-patch-unless-trusted",
|
||||
action: ActionKind::ApplyPatchFreeform {
|
||||
target: TargetPath::Workspace("apply_patch_freeform_unless_trusted.txt"),
|
||||
content: "freeform-patch-unless-trusted",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
features: vec![],
|
||||
@@ -1451,24 +1451,24 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
expected_reason: None,
|
||||
},
|
||||
expectation: Expectation::PatchApplied {
|
||||
target: TargetPath::Workspace("apply_patch_function_unless_trusted.txt"),
|
||||
content: "function-patch-unless-trusted",
|
||||
target: TargetPath::Workspace("apply_patch_freeform_unless_trusted.txt"),
|
||||
content: "freeform-patch-unless-trusted",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "apply_patch_function_never_rejects_outside_workspace",
|
||||
name: "apply_patch_freeform_never_rejects_outside_workspace",
|
||||
approval_policy: Never,
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::ApplyPatchFunction {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_never.txt"),
|
||||
content: "function-patch-never",
|
||||
action: ActionKind::ApplyPatchFreeform {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_never.txt"),
|
||||
content: "freeform-patch-never",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
features: vec![],
|
||||
model_override: Some("gpt-5.4"),
|
||||
outcome: Outcome::Auto,
|
||||
expectation: Expectation::FileNotCreated {
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_function_never.txt"),
|
||||
target: TargetPath::OutsideWorkspace("apply_patch_freeform_never.txt"),
|
||||
message_contains: &[
|
||||
"patch rejected: writing outside of the project; rejected by user approval settings",
|
||||
],
|
||||
@@ -1812,7 +1812,7 @@ async fn run_scenario_group(group: ScenarioGroup) -> Result<()> {
|
||||
|
||||
fn scenario_group(scenario: &ScenarioSpec) -> ScenarioGroup {
|
||||
match &scenario.action {
|
||||
ActionKind::ApplyPatchFunction { .. } | ActionKind::ApplyPatchShell { .. } => {
|
||||
ActionKind::ApplyPatchFreeform { .. } | ActionKind::ApplyPatchShell { .. } => {
|
||||
ScenarioGroup::ApplyPatch
|
||||
}
|
||||
ActionKind::RunUnifiedExecCommand { .. } => ScenarioGroup::UnifiedExec,
|
||||
@@ -1982,7 +1982,12 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
let output_item = results_mock.single_request().function_call_output(call_id);
|
||||
let output_request = results_mock.single_request();
|
||||
let output_item = if matches!(scenario.action, ActionKind::ApplyPatchFreeform { .. }) {
|
||||
output_request.custom_tool_call_output(call_id)
|
||||
} else {
|
||||
output_request.function_call_output(call_id)
|
||||
};
|
||||
let result = parse_result(&output_item);
|
||||
eprintln!(
|
||||
"approval scenario {} result: exit_code={:?} stdout={:?}",
|
||||
@@ -2035,7 +2040,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file()
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id_1, &patch_add),
|
||||
ev_apply_patch_custom_tool_call(call_id_1, &patch_add),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
@@ -2070,7 +2075,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file()
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-3"),
|
||||
ev_apply_patch_function_call(call_id_2, &patch_update),
|
||||
ev_apply_patch_custom_tool_call(call_id_2, &patch_update),
|
||||
ev_completed("resp-3"),
|
||||
]),
|
||||
)
|
||||
|
||||
@@ -7,7 +7,7 @@ use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::ReviewRequest;
|
||||
use codex_protocol::protocol::ReviewTarget;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
@@ -127,7 +127,7 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() {
|
||||
let patch = "*** Begin Patch\n*** Add File: delegated.txt\n+hello\n*** End Patch\n";
|
||||
let sse1 = sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, patch),
|
||||
ev_apply_patch_custom_tool_call(call_id, patch),
|
||||
ev_completed("resp-1"),
|
||||
]);
|
||||
let review_json = serde_json::json!({
|
||||
|
||||
@@ -23,7 +23,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use core_test_support::hooks::trust_discovered_hooks;
|
||||
use core_test_support::hooks::trust_hooks;
|
||||
use core_test_support::managed_network_requirements_loader;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
@@ -1524,7 +1524,7 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, &patch),
|
||||
ev_apply_patch_custom_tool_call(call_id, &patch),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
@@ -1563,7 +1563,7 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
requests[1].function_call_output(call_id);
|
||||
requests[1].custom_tool_call_output(call_id);
|
||||
assert!(
|
||||
target_path.exists(),
|
||||
"approved apply_patch should create the out-of-workspace file"
|
||||
@@ -2619,7 +2619,7 @@ async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> {
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, &patch),
|
||||
ev_apply_patch_custom_tool_call(call_id, &patch),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
@@ -2652,7 +2652,7 @@ async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> {
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let output_item = requests[1].function_call_output(call_id);
|
||||
let output_item = requests[1].custom_tool_call_output(call_id);
|
||||
let output = output_item
|
||||
.get("output")
|
||||
.and_then(Value::as_str)
|
||||
@@ -2693,7 +2693,7 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> {
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, &patch),
|
||||
ev_apply_patch_custom_tool_call(call_id, &patch),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
@@ -2724,7 +2724,7 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> {
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let output_item = requests[1].function_call_output(call_id);
|
||||
let output_item = requests[1].custom_tool_call_output(call_id);
|
||||
let output = output_item
|
||||
.get("output")
|
||||
.and_then(Value::as_str)
|
||||
@@ -3363,7 +3363,7 @@ async fn post_tool_use_records_additional_context_for_apply_patch() -> Result<()
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, &patch),
|
||||
ev_apply_patch_custom_tool_call(call_id, &patch),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
@@ -3451,7 +3451,7 @@ async fn post_tool_use_records_apply_patch_context_with_edit_alias() -> Result<(
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, &patch),
|
||||
ev_apply_patch_custom_tool_call(call_id, &patch),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
|
||||
@@ -17,7 +17,7 @@ use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
@@ -394,7 +394,7 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created(&format!("{response_prefix}-2")),
|
||||
ev_apply_patch_function_call("apply-patch-call", &patch),
|
||||
ev_apply_patch_custom_tool_call("apply-patch-call", &patch),
|
||||
ev_completed(&format!("{response_prefix}-2")),
|
||||
]),
|
||||
];
|
||||
@@ -475,7 +475,22 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
}
|
||||
|
||||
let patch_output = responses
|
||||
.function_call_output_text("apply-patch-call")
|
||||
.requests()
|
||||
.into_iter()
|
||||
.find_map(|request| {
|
||||
request
|
||||
.input()
|
||||
.into_iter()
|
||||
.find(|item| {
|
||||
item.get("type").and_then(Value::as_str) == Some("custom_tool_call_output")
|
||||
&& item.get("call_id").and_then(Value::as_str) == Some("apply-patch-call")
|
||||
})
|
||||
.and_then(|item| {
|
||||
item.get("output")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_string)
|
||||
})
|
||||
})
|
||||
.map(|output| json!({ "output": output }))
|
||||
.unwrap_or_else(|| panic!("expected apply-patch-call output"));
|
||||
let (exit_code, stdout) = parse_result(&patch_output);
|
||||
|
||||
@@ -472,7 +472,6 @@ $"#;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
async fn apply_patch_custom_tool_output_is_structured(
|
||||
@@ -518,7 +517,6 @@ A {file_name}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
async fn apply_patch_custom_tool_call_creates_file(
|
||||
@@ -566,7 +564,6 @@ A {file_name}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
async fn apply_patch_custom_tool_call_updates_existing_file(
|
||||
@@ -619,7 +616,6 @@ M {file_name}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
async fn apply_patch_custom_tool_call_reports_failure_output(
|
||||
@@ -664,20 +660,17 @@ async fn apply_patch_custom_tool_call_reports_failure_output(
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
async fn apply_patch_function_call_output_is_structured(
|
||||
output_type: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
async fn apply_patch_tool_output_is_structured(output_type: ApplyPatchModelOutput) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness().await?;
|
||||
|
||||
let call_id = "apply-patch-function";
|
||||
let file_name = "function_apply_patch.txt";
|
||||
let file_name = "freeform_apply_patch.txt";
|
||||
let patch =
|
||||
format!("*** Begin Patch\n*** Add File: {file_name}\n+via function call\n*** End Patch\n");
|
||||
format!("*** Begin Patch\n*** Add File: {file_name}\n+via apply_patch\n*** End Patch\n");
|
||||
mount_apply_patch(
|
||||
&harness,
|
||||
call_id,
|
||||
@@ -689,7 +682,7 @@ async fn apply_patch_function_call_output_is_structured(
|
||||
harness
|
||||
.test()
|
||||
.submit_turn_with_permission_profile(
|
||||
"apply the patch via function-call apply_patch",
|
||||
"apply the patch via freeform apply_patch",
|
||||
PermissionProfile::Disabled,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -14,7 +14,7 @@ use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::assert_regex_match;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::ResponsesRequest;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
@@ -47,6 +47,24 @@ fn call_output(req: &ResponsesRequest, call_id: &str) -> (String, Option<bool>)
|
||||
(content, success)
|
||||
}
|
||||
|
||||
fn custom_call_output(req: &ResponsesRequest, call_id: &str) -> (String, Option<bool>) {
|
||||
let raw = req.custom_tool_call_output(call_id);
|
||||
assert_eq!(
|
||||
raw.get("call_id").and_then(Value::as_str),
|
||||
Some(call_id),
|
||||
"mismatched call_id in custom_tool_call_output"
|
||||
);
|
||||
let (content_opt, success) = match req.custom_tool_call_output_content_and_success(call_id) {
|
||||
Some(values) => values,
|
||||
None => panic!("custom_tool_call_output present"),
|
||||
};
|
||||
let content = match content_opt {
|
||||
Some(c) => c,
|
||||
None => panic!("custom_tool_call_output content present"),
|
||||
};
|
||||
(content, success)
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn shell_tool_executes_command_and_streams_output() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -107,10 +125,10 @@ async fn shell_tool_executes_command_and_streams_output() -> anyhow::Result<()>
|
||||
|
||||
let req = second_mock.single_request();
|
||||
let (output_text, _) = call_output(&req, call_id);
|
||||
let exec_output: Value = serde_json::from_str(&output_text)?;
|
||||
assert_eq!(exec_output["metadata"]["exit_code"], 0);
|
||||
let stdout = exec_output["output"].as_str().expect("stdout field");
|
||||
assert_regex_match(r"(?s)^tool harness\n?$", stdout);
|
||||
assert_regex_match(
|
||||
r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:\ntool harness\n?$",
|
||||
&output_text,
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -328,7 +346,7 @@ async fn apply_patch_tool_executes_and_emits_patch_events() -> anyhow::Result<()
|
||||
|
||||
let first_response = sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, &patch_content),
|
||||
ev_apply_patch_custom_tool_call(call_id, &patch_content),
|
||||
ev_completed("resp-1"),
|
||||
]);
|
||||
responses::mount_sse_once(&server, first_response).await;
|
||||
@@ -419,7 +437,7 @@ async fn apply_patch_tool_executes_and_emits_patch_events() -> anyhow::Result<()
|
||||
assert!(patch_end_success);
|
||||
|
||||
let req = second_mock.single_request();
|
||||
let (output_text, _success_flag) = call_output(&req, call_id);
|
||||
let (output_text, _success_flag) = custom_call_output(&req, call_id);
|
||||
|
||||
let expected_pattern = format!(
|
||||
r"(?s)^Exit code: 0
|
||||
@@ -466,7 +484,7 @@ async fn apply_patch_reports_parse_diagnostics() -> anyhow::Result<()> {
|
||||
|
||||
let first_response = sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id, patch_content),
|
||||
ev_apply_patch_custom_tool_call(call_id, patch_content),
|
||||
ev_completed("resp-1"),
|
||||
]);
|
||||
responses::mount_sse_once(&server, first_response).await;
|
||||
@@ -507,7 +525,7 @@ async fn apply_patch_reports_parse_diagnostics() -> anyhow::Result<()> {
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let req = second_mock.single_request();
|
||||
let (output_text, success_flag) = call_output(&req, call_id);
|
||||
let (output_text, success_flag) = custom_call_output(&req, call_id);
|
||||
|
||||
assert!(
|
||||
output_text.contains("apply_patch verification failed"),
|
||||
|
||||
@@ -433,15 +433,10 @@ async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> {
|
||||
.function_call_output_content_and_success(call_id_success)
|
||||
.and_then(|(content, _)| content)
|
||||
.expect("success output string");
|
||||
let output_json: Value = serde_json::from_str(&success_output)?;
|
||||
assert_eq!(
|
||||
output_json["metadata"]["exit_code"].as_i64(),
|
||||
Some(0),
|
||||
"expected exit code 0 after rerunning without escalation",
|
||||
assert_regex_match(
|
||||
r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:\nshell ok\n?$",
|
||||
&success_output,
|
||||
);
|
||||
let stdout = output_json["output"].as_str().unwrap_or_default();
|
||||
let stdout_pattern = r"(?s)^shell ok\n?$";
|
||||
assert_regex_match(stdout_pattern, stdout);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -4,7 +4,6 @@ use anyhow::Context;
|
||||
use assert_cmd::prelude::*;
|
||||
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
@@ -71,7 +70,7 @@ async fn test_apply_patch_tool() -> anyhow::Result<()> {
|
||||
ev_completed("request_0"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_apply_patch_function_call("request_1", update_patch),
|
||||
ev_apply_patch_custom_tool_call("request_1", update_patch),
|
||||
ev_completed("request_1"),
|
||||
]),
|
||||
sse(vec![ev_completed("request_2")]),
|
||||
|
||||
@@ -63,7 +63,7 @@ fn gpt_5_4_cmb_bedrock_model(priority: i32) -> ModelInfo {
|
||||
default_reasoning_summary: ReasoningSummary::None,
|
||||
support_verbosity: true,
|
||||
default_verbosity: Some(Verbosity::Medium),
|
||||
apply_patch_tool_type: Some(ApplyPatchToolType::Function),
|
||||
apply_patch_tool_type: Some(ApplyPatchToolType::Freeform),
|
||||
web_search_tool_type: WebSearchToolType::TextAndImage,
|
||||
truncation_policy: TruncationPolicyConfig::tokens(/*limit*/ 10_000),
|
||||
supports_parallel_tool_calls: true,
|
||||
|
||||
@@ -203,7 +203,6 @@ pub enum ConfigShellToolType {
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum ApplyPatchToolType {
|
||||
Freeform,
|
||||
Function,
|
||||
}
|
||||
|
||||
#[derive(
|
||||
|
||||
@@ -219,11 +219,10 @@ impl ToolsConfig {
|
||||
model_shell_type
|
||||
};
|
||||
|
||||
let apply_patch_tool_type = match model_info.apply_patch_tool_type {
|
||||
Some(ApplyPatchToolType::Freeform) => Some(ApplyPatchToolType::Freeform),
|
||||
Some(ApplyPatchToolType::Function) => Some(ApplyPatchToolType::Function),
|
||||
None => include_apply_patch_tool.then_some(ApplyPatchToolType::Function),
|
||||
};
|
||||
let apply_patch_tool_type = model_info
|
||||
.apply_patch_tool_type
|
||||
.clone()
|
||||
.or_else(|| include_apply_patch_tool.then_some(ApplyPatchToolType::Freeform));
|
||||
|
||||
let agent_jobs_worker_tools = include_agent_jobs
|
||||
&& matches!(
|
||||
|
||||
@@ -156,7 +156,7 @@ fn shell_zsh_fork_prefers_shell_command_over_unified_exec() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fallback_apply_patch_models_use_function_tool_by_default() {
|
||||
fn fallback_apply_patch_models_use_freeform_tool_by_default() {
|
||||
let model_info = model_info();
|
||||
let features = Features::with_defaults();
|
||||
|
||||
@@ -174,7 +174,7 @@ fn fallback_apply_patch_models_use_function_tool_by_default() {
|
||||
|
||||
assert_eq!(
|
||||
tools_config.apply_patch_tool_type,
|
||||
Some(ApplyPatchToolType::Function)
|
||||
Some(ApplyPatchToolType::Freeform)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user