Skip to content

Commit 20ca614

Browse files
committed
fix(agent): harden Write path recovery
Previously, POSIX-prefixed Windows drive paths (e.g. /C:/foo/main.py) could resolve as drive-relative paths and write files under the process working directory. Missing-marker fallbacks also led agents to resend payloads unnecessarily. Update Write handling to: - Normalize malformed Windows drive paths while preserving remote POSIX path semantics - Report corrected paths and provide platform-specific examples - Direct agents to move preserved fallback files instead of resubmitting content - Cover path normalization and fallback guidance with focused tests
1 parent 66709c3 commit 20ca614

4 files changed

Lines changed: 180 additions & 40 deletions

File tree

src/crates/assembly/core/src/agentic/tools/implementations/file_write_tool.rs

Lines changed: 95 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use crate::agentic::tools::framework::{
1515
use crate::agentic::tools::ToolPathOperation;
1616
use crate::util::errors::{BitFunError, BitFunResult};
1717
use async_trait::async_trait;
18+
use bitfun_agent_tools::strip_invalid_windows_drive_path_prefix;
1819
use serde_json::{json, Value};
1920
use std::path::Path;
2021
use tokio::fs;
@@ -237,16 +238,21 @@ impl FileWriteTool {
237238
logical_path: &str,
238239
outcome: WriteLocalFileOutcome,
239240
missing_path_fallback: bool,
241+
path_format_warning: Option<&str>,
240242
ignored_parameter_names: &[String],
241243
) -> ToolResult {
242244
let mut assistant_message = if missing_path_fallback {
243245
format!(
244-
"The Write payload did not start with the required '+++ {{file_path}}' marker. The entire payload was saved to {}. Use your shell tool to rename this file to the intended path instead of calling Write to resubmit the same content.",
246+
"The entire payload was saved to {}. Use your shell tool to move this file to the intended path. Do not call Write to resubmit the same content because doing so wastes tokens and time. This happened because the Write payload did not start with the required '+++ {{file_path}}' marker. Future Write calls must follow the required payload format.",
245247
logical_path
246248
)
247249
} else {
248250
outcome.assistant_message
249251
};
252+
if let Some(warning) = path_format_warning {
253+
assistant_message.push(' ');
254+
assistant_message.push_str(warning);
255+
}
250256
if !ignored_parameter_names.is_empty() {
251257
let formatted_names = ignored_parameter_names
252258
.iter()
@@ -267,13 +273,23 @@ impl FileWriteTool {
267273
"status": outcome.status.as_str(),
268274
"missing_path_fallback": missing_path_fallback,
269275
"rename_required": missing_path_fallback,
276+
"path_format_corrected": path_format_warning.is_some(),
277+
"path_format_warning": path_format_warning,
270278
"message": assistant_message,
271279
}),
272280
result_for_assistant: Some(assistant_message),
273281
image_attachments: None,
274282
}
275283
}
276284

285+
fn path_format_correction_warning(resolved: &ToolPathResolution) -> Option<String> {
286+
strip_invalid_windows_drive_path_prefix(&resolved.requested_path)?;
287+
Some(format!(
288+
"The provided Windows path '{}' had an invalid leading '/'. It was normalized to '{}'. Use a drive-letter path without the leading '/' in future Write calls.",
289+
resolved.requested_path, resolved.logical_path
290+
))
291+
}
292+
277293
fn input_schema() -> Value {
278294
json!({
279295
"type": "object",
@@ -306,10 +322,10 @@ Usage:
306322
- Only use emojis if the user explicitly requests it. Avoid writing emojis to files unless asked.
307323
308324
Examples:
309-
<good-example>
310-
`{"payload":"+++ /path/to/main.py\ndef main():\n\tprint(\"Hello world\")\n\nmain()"}`
325+
<good-example platform="macos-linux">
326+
`{"payload":"+++ /example/main.py\ndef main():\n\tprint(\"Hello world\")\n\nmain()"}`
311327
312-
This call creates or overwrites `/path/to/main.py` with the following content:
328+
This call creates or overwrites `/example/main.py` with the following content:
313329
```
314330
def main():
315331
print("Hello world")
@@ -318,14 +334,18 @@ main()
318334
```
319335
</good-example>
320336
337+
<good-example platform="windows">
338+
`{"payload":"+++ C:/foo/main.py\ndef main():\n\tprint(\"Hello world\")\n\nmain()"}`
339+
</good-example>
340+
321341
<bad-example>
322-
`{"file_path":"/path/to/main.py","content":"print(\"Hello world\")"}`
342+
`{"file_path":"/example/main.py","content":"print(\"Hello world\")"}`
323343
324344
This call is invalid because Write requires the single `payload` parameter. Do not pass `file_path` and `content` separately.
325345
</bad-example>
326346
327347
<bad-example>
328-
`{"payload":"+++ /path/to/main.py\nprint(\"Hello world\")","file_path":"/path/to/main.py"}`
348+
`{"payload":"+++ /example/main.py\nprint(\"Hello world\")","file_path":"/example/main.py"}`
329349
330350
This call includes an unnecessary `file_path` parameter. Write only uses `payload`; specify the target path in the first `+++ {file_path}` line and do not pass additional parameters.
331351
</bad-example>
@@ -522,6 +542,7 @@ impl Tool for FileWriteTool {
522542
};
523543

524544
let resolved = context.resolve_tool_path(&file_path)?;
545+
let path_format_warning = Self::path_format_correction_warning(&resolved);
525546
context.enforce_path_operation(ToolPathOperation::Write, &resolved)?;
526547
context
527548
.record_light_checkpoint(
@@ -539,6 +560,7 @@ impl Tool for FileWriteTool {
539560
&resolved.logical_path,
540561
write_same_content_outcome(&resolved.logical_path),
541562
missing_path_fallback,
563+
path_format_warning.as_deref(),
542564
&ignored_parameter_names,
543565
);
544566
return Ok(vec![result]);
@@ -574,6 +596,7 @@ impl Tool for FileWriteTool {
574596
&resolved.logical_path,
575597
write_file_success_outcome(&resolved.logical_path, file_already_exists, &content),
576598
missing_path_fallback,
599+
path_format_warning.as_deref(),
577600
&ignored_parameter_names,
578601
);
579602
return Ok(vec![result]);
@@ -609,6 +632,7 @@ impl Tool for FileWriteTool {
609632
&resolved.logical_path,
610633
outcome,
611634
missing_path_fallback,
635+
path_format_warning.as_deref(),
612636
&ignored_parameter_names,
613637
);
614638

@@ -832,18 +856,6 @@ mod tests {
832856
assert_eq!(data["lines_written"], 0);
833857
}
834858

835-
#[test]
836-
fn description_includes_bad_examples_for_invalid_parameter_shapes() {
837-
let description = FileWriteTool::description();
838-
839-
assert_eq!(description.matches("<bad-example>").count(), 2);
840-
assert!(description
841-
.contains(r#"{"file_path":"/path/to/main.py","content":"print(\"Hello world\")"}"#));
842-
assert!(description.contains(
843-
r#"{"payload":"+++ /path/to/main.py\nprint(\"Hello world\")","file_path":"/path/to/main.py"}"#
844-
));
845-
}
846-
847859
#[tokio::test]
848860
async fn schema_requires_single_payload_parameter() {
849861
let tool = FileWriteTool::new();
@@ -881,6 +893,60 @@ mod tests {
881893
assert!(validation.message.is_none());
882894
}
883895

896+
#[cfg(windows)]
897+
#[tokio::test]
898+
async fn write_result_reports_normalized_windows_drive_path() {
899+
let tool = FileWriteTool::new();
900+
let context = local_context(PathBuf::from(r"E:\workspace"));
901+
let requested_path = "/E:/workspace/project/example.txt";
902+
903+
let validation = tool
904+
.validate_input(
905+
&json!({
906+
"payload": format!("+++ {requested_path}\ncontent")
907+
}),
908+
Some(&context),
909+
)
910+
.await;
911+
assert!(validation.result);
912+
assert!(validation.message.is_none());
913+
914+
let resolved = context
915+
.resolve_tool_path(requested_path)
916+
.expect("mixed Windows path should be normalized");
917+
assert_eq!(
918+
PathBuf::from(&resolved.logical_path),
919+
PathBuf::from(r"E:\workspace\project\example.txt")
920+
);
921+
922+
let warning = FileWriteTool::path_format_correction_warning(&resolved)
923+
.expect("normalization should produce a warning");
924+
let result = FileWriteTool::write_success_result(
925+
&resolved.logical_path,
926+
super::write_file_success_outcome(&resolved.logical_path, false, "content"),
927+
false,
928+
Some(&warning),
929+
&[],
930+
);
931+
let ToolResult::Result {
932+
data,
933+
result_for_assistant,
934+
..
935+
} = result
936+
else {
937+
panic!("expected result");
938+
};
939+
940+
assert_eq!(data["path_format_corrected"], true);
941+
assert_eq!(data["path_format_warning"], warning);
942+
assert!(warning.contains(requested_path));
943+
assert!(warning.contains(r"E:\workspace\project\example.txt"));
944+
assert!(result_for_assistant
945+
.as_deref()
946+
.unwrap_or_default()
947+
.contains(&warning));
948+
}
949+
884950
#[test]
885951
fn parse_payload_recognizes_marked_path_with_lf_or_crlf() {
886952
for value in [
@@ -967,10 +1033,17 @@ mod tests {
9671033
);
9681034
assert_eq!(data["missing_path_fallback"], true);
9691035
assert_eq!(data["rename_required"], true);
970-
assert!(result_for_assistant
971-
.as_deref()
972-
.unwrap_or_default()
973-
.contains("Use your shell tool to rename this file"));
1036+
let assistant_message = result_for_assistant.as_deref().unwrap_or_default();
1037+
assert!(assistant_message
1038+
.contains("Use your shell tool to move this file to the intended path"));
1039+
assert!(assistant_message.contains(
1040+
"Do not call Write to resubmit the same content because doing so wastes tokens and time"
1041+
));
1042+
assert!(assistant_message.contains(
1043+
"the Write payload did not start with the required '+++ {file_path}' marker"
1044+
));
1045+
assert!(assistant_message
1046+
.contains("Future Write calls must follow the required payload format"));
9741047

9751048
let _ = std::fs::remove_dir_all(&root);
9761049
}

src/crates/execution/tool-contracts/src/framework.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1798,10 +1798,32 @@ pub fn normalize_host_path(path: &str) -> String {
17981798
.to_string()
17991799
}
18001800

1801+
/// Returns a Windows drive path without an invalid POSIX-style leading slash.
1802+
/// Non-Windows hosts leave the same spelling available for POSIX path semantics.
1803+
pub fn strip_invalid_windows_drive_path_prefix(path: &str) -> Option<&str> {
1804+
#[cfg(windows)]
1805+
{
1806+
let bytes = path.as_bytes();
1807+
if bytes.len() >= 4
1808+
&& bytes[0] == b'/'
1809+
&& bytes[1].is_ascii_alphabetic()
1810+
&& bytes[2] == b':'
1811+
&& matches!(bytes[3], b'/' | b'\\')
1812+
{
1813+
return Some(&path[1..]);
1814+
}
1815+
}
1816+
1817+
let _ = path;
1818+
None
1819+
}
1820+
18011821
pub fn resolve_host_path_with_workspace(
18021822
path: &str,
18031823
workspace_root: Option<&Path>,
18041824
) -> Result<String, ToolPathContractError> {
1825+
let path = strip_invalid_windows_drive_path_prefix(path).unwrap_or(path);
1826+
18051827
if Path::new(path).is_absolute() {
18061828
Ok(normalize_host_path(path))
18071829
} else {

src/crates/execution/tool-contracts/src/lib.rs

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -78,24 +78,25 @@ pub use framework::{
7878
resolve_host_path, resolve_host_path_with_workspace, resolve_readonly_enabled_tools,
7979
resolve_tool_manifest_policy, resolve_tool_path_with_context,
8080
resolve_tool_path_with_context_roots, resolve_workspace_tool_path,
81-
sort_tool_manifest_definitions, summarize_get_tool_spec_deferred_tools,
82-
tool_manifest_sort_rank, tool_path_is_effectively_absolute,
83-
tool_restrictions_for_delegation_policy, validate_deferred_tool_usage,
84-
validate_get_tool_spec_input, validate_tool_allowed_by_list, ContextualToolManifest,
85-
ContextualToolManifestItem, ContextualVisibleTools, DeferredToolUsageError, DynamicMcpToolInfo,
86-
DynamicToolInfo, GetToolSpecCatalogProvider, GetToolSpecDeferredToolSummary, GetToolSpecDetail,
87-
GetToolSpecExecutionError, GetToolSpecExecutionPlan, GetToolSpecLoadObservation,
88-
GetToolSpecRuntime, LoadedDeferredToolSpec, ParsedBitFunCurrentSessionUri,
89-
ParsedBitFunRuntimeUri, PortableToolContextProvider, PromptVisibleToolManifestItem,
90-
SnapshotToolDecorator, SnapshotToolWrapper, SnapshotToolWrapperRef,
91-
StaticToolMaterializationError, StaticToolProvider, StaticToolProviderFactory,
92-
StaticToolProviderGroup, StaticToolProviderPlan, ToolCatalogRuntime,
93-
ToolCatalogSnapshotProvider, ToolContextFacts, ToolDecoratorRef, ToolExecutionAccessError,
94-
ToolExposure, ToolManifestDefinition, ToolManifestPolicyResolution, ToolManifestPolicyTool,
95-
ToolPathBackend, ToolPathContractError, ToolPathOperation, ToolPathPolicy, ToolPathResolution,
96-
ToolRef, ToolRegistry, ToolRegistryItem, ToolRenderOptions, ToolRestrictionError, ToolResult,
97-
ToolRuntimeAssembly, ToolRuntimeRestrictions, ToolWorkspaceKind, ValidationResult,
98-
BITFUN_CURRENT_SESSION_URI_PREFIX, BITFUN_RUNTIME_URI_PREFIX, GET_TOOL_SPEC_TOOL_NAME,
81+
sort_tool_manifest_definitions, strip_invalid_windows_drive_path_prefix,
82+
summarize_get_tool_spec_deferred_tools, tool_manifest_sort_rank,
83+
tool_path_is_effectively_absolute, tool_restrictions_for_delegation_policy,
84+
validate_deferred_tool_usage, validate_get_tool_spec_input, validate_tool_allowed_by_list,
85+
ContextualToolManifest, ContextualToolManifestItem, ContextualVisibleTools,
86+
DeferredToolUsageError, DynamicMcpToolInfo, DynamicToolInfo, GetToolSpecCatalogProvider,
87+
GetToolSpecDeferredToolSummary, GetToolSpecDetail, GetToolSpecExecutionError,
88+
GetToolSpecExecutionPlan, GetToolSpecLoadObservation, GetToolSpecRuntime,
89+
LoadedDeferredToolSpec, ParsedBitFunCurrentSessionUri, ParsedBitFunRuntimeUri,
90+
PortableToolContextProvider, PromptVisibleToolManifestItem, SnapshotToolDecorator,
91+
SnapshotToolWrapper, SnapshotToolWrapperRef, StaticToolMaterializationError,
92+
StaticToolProvider, StaticToolProviderFactory, StaticToolProviderGroup, StaticToolProviderPlan,
93+
ToolCatalogRuntime, ToolCatalogSnapshotProvider, ToolContextFacts, ToolDecoratorRef,
94+
ToolExecutionAccessError, ToolExposure, ToolManifestDefinition, ToolManifestPolicyResolution,
95+
ToolManifestPolicyTool, ToolPathBackend, ToolPathContractError, ToolPathOperation,
96+
ToolPathPolicy, ToolPathResolution, ToolRef, ToolRegistry, ToolRegistryItem, ToolRenderOptions,
97+
ToolRestrictionError, ToolResult, ToolRuntimeAssembly, ToolRuntimeRestrictions,
98+
ToolWorkspaceKind, ValidationResult, BITFUN_CURRENT_SESSION_URI_PREFIX,
99+
BITFUN_RUNTIME_URI_PREFIX, GET_TOOL_SPEC_TOOL_NAME,
99100
};
100101
pub use input_validator::InputValidator;
101102
#[cfg(feature = "mcp-bridge")]

src/crates/execution/tool-contracts/tests/tool_contracts.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1697,6 +1697,50 @@ fn host_path_contract_keeps_local_workspace_resolution_semantics() {
16971697
);
16981698
}
16991699

1700+
#[cfg(windows)]
1701+
#[test]
1702+
fn host_path_contract_normalizes_posix_prefixed_windows_drive_paths() {
1703+
let workspace = PathBuf::from(r"E:\workspace");
1704+
1705+
for (malformed, expected) in [
1706+
(
1707+
"/E:/workspace/project/example.txt",
1708+
r"E:\workspace\project\example.txt",
1709+
),
1710+
(
1711+
"/e:/workspace/project/example.txt",
1712+
r"e:\workspace\project\example.txt",
1713+
),
1714+
(
1715+
r"/E:\workspace\project\example.txt",
1716+
r"E:\workspace\project\example.txt",
1717+
),
1718+
] {
1719+
let resolved = resolve_host_path_with_workspace(malformed, Some(workspace.as_path()))
1720+
.expect("mixed POSIX and Windows drive syntax should be normalized");
1721+
1722+
assert_eq!(PathBuf::from(resolved), PathBuf::from(expected));
1723+
}
1724+
1725+
let valid = resolve_host_path_with_workspace(
1726+
"E:/workspace/project/example.txt",
1727+
Some(workspace.as_path()),
1728+
)
1729+
.expect("native Windows drive syntax must remain valid");
1730+
assert_eq!(
1731+
PathBuf::from(valid),
1732+
PathBuf::from(r"E:\workspace\project\example.txt")
1733+
);
1734+
1735+
let remote = resolve_workspace_tool_path(
1736+
"/E:/workspace/project/example.txt",
1737+
Some("/workspace"),
1738+
true,
1739+
)
1740+
.expect("remote workspaces keep POSIX path semantics");
1741+
assert_eq!(remote, "/E:/workspace/project/example.txt");
1742+
}
1743+
17001744
#[test]
17011745
fn unified_tool_path_contract_selects_host_or_remote_semantics() {
17021746
let local = resolve_workspace_tool_path("src/lib.rs", Some("/repo/project"), false)

0 commit comments

Comments
 (0)