Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 96 additions & 5 deletions crates/goose-mcp/src/developer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ impl DeveloperRouter {
To use the write command, you must specify `file_text` which will become the new content of the file. Be careful with
existing files! This is a full overwrite, so you must include everything - not just sections you are modifying.

To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning)
To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning, -1 for end)
and `new_str` (the text to insert).

To use the edit_file command, you must specify both `old_str` and `new_str`
Expand All @@ -416,7 +416,7 @@ impl DeveloperRouter {
unique section of the original file, including any whitespace. Make sure to include enough context that the match is not
ambiguous. The entire original string will be replaced with `new_str`.

To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning)
To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning, -1 for end)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "correct" way to insert at the end is for the model to pass len(lines). We should still accommodate Gemini but I'd leave this out of the tool description, since the intention is to always use a nonnegative index.

Copy link
Contributor Author

@understood-the-assignment understood-the-assignment Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "correct" way to insert at the end is for the model to pass len(lines).

This requires that the model knows and keeps track of the exact number of lines in the file, which I think doesn't come "for free", e.g. if the model previously only needed to see the start of the file or some rg results, or if the model modified the file since the last time it looked at it.

In any case, I think appending to a file should be easy for the model to do, but I guess it doesn't have to be using the insert command.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point and I'm all for making this easier on the model.

@baxen to your knowledge, might this impact the performance of the editor with claude? I know this tool schema closely resembles https://docs.anthropic.com/en/docs/agents-and-tools/tool-use/text-editor-tool#insert . This is a subtle instruction change but would be good to merge with high confidence in no regression.

and `new_str` (the text to insert).
"#}.to_string(), "str_replace")
};
Expand Down Expand Up @@ -446,7 +446,7 @@ impl DeveloperRouter {
},
"insert_line": {
"type": "integer",
"description": "The line number after which to insert the text (0 for beginning of file). This parameter is required when using the insert command."
"description": "The line number after which to insert the text (0 for beginning of file, -1 for end of file). This parameter is required when using the insert command."
},
"old_str": {"type": "string"},
"new_str": {"type": "string"},
Expand Down Expand Up @@ -1039,7 +1039,7 @@ impl DeveloperRouter {
"Missing 'insert_line' parameter".to_string(),
None,
)
})? as usize;
})?;
let new_str = params
.get("new_str")
.and_then(|v| v.as_str())
Expand Down Expand Up @@ -1442,7 +1442,7 @@ impl DeveloperRouter {
async fn text_editor_insert(
&self,
path: &PathBuf,
insert_line: usize,
insert_line_spec: i64,
new_str: &str,
) -> Result<Vec<Content>, ErrorData> {
// Check if file exists
Expand Down Expand Up @@ -1472,6 +1472,14 @@ impl DeveloperRouter {
let lines: Vec<&str> = content.lines().collect();
let total_lines = lines.len();

// Allow insert_line to be negative
let insert_line = if insert_line_spec < 0 {
// -1 == end of file, -2 == before the last line, etc.
(total_lines as i64 + 1 + insert_line_spec) as usize
} else {
insert_line_spec as usize
};

// Validate insert_line parameter
if insert_line > total_lines {
return Err(ErrorData::new(ErrorCode::INVALID_PARAMS, format!(
Expand Down Expand Up @@ -3270,6 +3278,89 @@ mod tests {
temp_dir.close().unwrap();
}

#[tokio::test]
#[serial]
async fn test_text_editor_insert_at_end_negative() {
let router = get_router().await;

let temp_dir = tempfile::tempdir().unwrap();
let file_path = temp_dir.path().join("test.txt");
let file_path_str = file_path.to_str().unwrap();
std::env::set_current_dir(&temp_dir).unwrap();

// Create a file with some content
let content = "Line 1\nLine 2\nLine 3";
router
.call_tool(
"text_editor",
json!({
"command": "write",
"path": file_path_str,
"file_text": content
}),
dummy_sender(),
)
.await
.unwrap();

// Insert at the end (after line 3)
let insert_result = router
.call_tool(
"text_editor",
json!({
"command": "insert",
"path": file_path_str,
"insert_line": -1,
"new_str": "Line 4"
}),
dummy_sender(),
)
.await
.unwrap();

let text = insert_result
.iter()
.find(|c| {
c.audience()
.is_some_and(|roles| roles.contains(&Role::Assistant))
})
.unwrap()
.as_text()
.unwrap();

assert!(text.text.contains("Text has been inserted at line 4"));

// Verify the file content
let view_result = router
.call_tool(
"text_editor",
json!({
"command": "view",
"path": file_path_str
}),
dummy_sender(),
)
.await
.unwrap();

let view_text = view_result
.iter()
.find(|c| {
c.audience()
.is_some_and(|roles| roles.contains(&Role::User))
})
.unwrap()
.as_text()
.unwrap();

assert!(view_text.text.contains("1: Line 1"));
assert!(view_text.text.contains("2: Line 2"));
assert!(view_text.text.contains("3: Line 3"));
assert!(view_text.text.contains("4: Line 4"));

temp_dir.close().unwrap();
}

#[tokio::test]
#[serial]
async fn test_text_editor_insert_invalid_line() {
Expand Down
Loading