Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

refactor(worker): share file contracts across Tools and WIP providers

Browse Source 
Flatten shared typed Tool arguments and derive WIP parameters and checkout allowlists from core schemas. Workspace settings uses shared read/write/edit processing within its captured-read and canonical tree-CAS commit path. Identical settings edits now reject before read or commit; provider response contracts and unknown-outcome handling stay intact.

T-707 validation: root cargo check; fs-operation (58); tools (74 lib, 39 integration, 1 doctest); worker (802 lib, 109 integration); workdir --features http-client (125); yoi-workspace-server --lib workspace_config (19); cargo fmt --all -- --check; git diff --check HEAD.
This commit is contained in:
Hare
2026-10-06 14:18:34 +09:00
parent 4511d4f8ac
commit 4f38f7d5b0
12 changed files with 388 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/tools/src/checkout.rs
+15 -9
View File
@@ -36,13 +36,19 @@ pub async fn execute_checkout_tool(
arguments: Value,
ctx: ToolExecutionContext,
) -> Result<CheckoutToolOutput, ToolError> {
// Native file arguments come from the same definitions used by ordinary
// Tools and both WIP projections. Route fields are intentionally not in them.
let file_schema = fs_operation::text::TextOperation::from_name(&tool_name.to_lowercase(), true)
.map(|operation| operation.argument_schema());
let file_fields: Vec<&str> = file_schema
.as_ref()
.and_then(|schema| schema.as_value()["properties"].as_object())
.map(|properties| properties.keys().map(String::as_str).collect())
.unwrap_or_default();
let (capability, allowed): (_, &[&str]) = match tool_name {
"Read" => (WorkdirSessionCapability::Read, &["offset", "limit"]),
"Edit" => (
WorkdirSessionCapability::Edit,
&["old_string", "new_string", "replace_all"],
),
"Write" => (WorkdirSessionCapability::Write, &["content"]),
"Read" => (WorkdirSessionCapability::Read, &file_fields),
"Edit" => (WorkdirSessionCapability::Edit, &file_fields),
"Write" => (WorkdirSessionCapability::Write, &file_fields),
"Create" => (WorkdirSessionCapability::Write, &["path", "content"]),
"Glob" => (WorkdirSessionCapability::Glob, &["pattern", "path"]),
"Grep" => (
@@ -110,7 +116,8 @@ pub async fn execute_checkout_tool(
}
_ => {
let params: crate::write::WriteParams = decode(arguments, tool_name)?;
crate::write::execute_write(target, tracker, params.content, None, ctx).await
crate::write::execute_write(target, tracker, params.write.content, None, ctx)
.await
}
}
}
@@ -185,8 +192,7 @@ pub async fn execute_checkout_tool(
}
fn decode<T: DeserializeOwned>(arguments: Value, name: &str) -> Result<T, ToolError> {
serde_json::from_value(arguments)
.map_err(|error| ToolError::InvalidArgument(format!("invalid {name} input: {error}")))
crate::error::decode_file_value(arguments, name)
}
fn stale_search() -> ToolError {
crates/tools/src/edit.rs
+35 -26
View File
@@ -23,13 +23,8 @@ pub(crate) struct EditParams {
pub target_workdir: Option<String>,
/// Logical path relative to the bound Workdir root.
pub file_path: String,
/// String to replace. Must be unique in the file unless `replace_all` is true.
pub old_string: String,
/// Replacement string. Must differ from `old_string`.
pub new_string: String,
/// Replace all occurrences. Defaults to false.
#[serde(default)]
pub replace_all: bool,
#[serde(flatten)]
pub edit: fs_operation::text::EditArgs,
}
pub(crate) struct EditTool {
@@ -44,8 +39,7 @@ impl Tool for EditTool {
input_json: &str,
ctx: agen::tool::ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
let params: EditParams = serde_json::from_str(input_json)
.map_err(|e| ToolError::InvalidArgument(format!("invalid Edit input: {e}")))?;
let params: EditParams = crate::error::decode_file_input(input_json, "Edit")?;
let selected = crate::routing::resolve_session(
&self.router,
@@ -78,25 +72,19 @@ pub(crate) async fn execute_edit(
ctx: agen::tool::ToolExecutionContext,
) -> Result<crate::checkout::CheckoutToolOutput, ToolError> {
let path = &target.path;
tracing::debug!(path = %path, replace_all = params.replace_all, "Edit");
if params.old_string.is_empty() {
return Err(ToolError::InvalidArgument(
"old_string must not be empty".into(),
));
}
if params.old_string == params.new_string {
return Err(ToolError::InvalidArgument(
"old_string and new_string are identical".into(),
));
}
tracing::debug!(path = %path, replace_all = params.edit.replace_all, "Edit");
params
.edit
.validate(Default::default())
.map_err(crate::error::text_error)?;
let mutation_key = PathBuf::from(path.as_str());
let _mutation_permit = tracker.acquire_mutation(&mutation_key, &ctx).await;
let expected_hash = tracker.expected_workdir_hash(path)?;
let (result, validator) = target
.edit(
params.old_string.clone(),
params.new_string.clone(),
params.replace_all,
params.edit.old_string.clone(),
params.edit.new_string.clone(),
params.edit.replace_all,
expected_hash,
)
.await?;
@@ -105,8 +93,8 @@ pub(crate) async fn execute_edit(
path,
result.content_hash,
replacements,
params.new_string.lines().count(),
params.old_string.lines().count(),
params.edit.new_string.lines().count(),
params.edit.old_string.lines().count(),
);
let summary = format!(
"Edited {} ({} replacement{})",
@@ -114,7 +102,7 @@ pub(crate) async fn execute_edit(
replacements,
if replacements == 1 { "" } else { "s" }
);
let preview = make_preview(&params.new_string, &params.new_string);
let preview = make_preview(&params.edit.new_string, &params.edit.new_string);
Ok(crate::checkout::CheckoutToolOutput {
output: ToolOutput {
summary,
@@ -200,6 +188,27 @@ mod tests {
.unwrap();
}
#[tokio::test]
async fn edit_shared_validation_rejects_identical_text_before_read_or_mutation() {
let (dir, fs, tracker) = setup();
let file = dir.path().join("a.txt");
std::fs::write(&file, "same").unwrap();
let (_, tool) = edit_tool(fs, tracker.clone())();
let error = tool
.execute(
&serde_json::json!({"file_path":"a.txt","old_string":"same","new_string":"same"})
.to_string(),
Default::default(),
)
.await
.unwrap_err();
assert!(
matches!(error, ToolError::InvalidArgument(message) if message == "old_string and new_string are identical")
);
assert_eq!(std::fs::read_to_string(file).unwrap(), "same");
assert_eq!(tracker.change_stat(), crate::ChangeStat::default());
}
#[tokio::test]
async fn edit_unique_replacement() {
let (dir, fs, tracker) = setup();
crates/tools/src/error.rs
+40
View File
@@ -58,11 +58,51 @@ impl From<ToolsError> for ToolError {
}
}
/// Pure text failures are pre-dispatch argument failures, never unknown outcomes.
pub(crate) fn text_error(error: fs_operation::text::TextError) -> ToolError {
ToolError::InvalidArgument(error.to_string())
}
pub(crate) fn decode_file_input<T: serde::de::DeserializeOwned>(
input: &str,
name: &str,
) -> Result<T, ToolError> {
fs_operation::text::decode_json(input).map_err(|error| decode_error(error, name))
}
pub(crate) fn decode_file_value<T: serde::de::DeserializeOwned>(
value: serde_json::Value,
name: &str,
) -> Result<T, ToolError> {
fs_operation::text::decode(value).map_err(|error| decode_error(error, name))
}
fn decode_error(error: fs_operation::text::TextError, name: &str) -> ToolError {
match error {
fs_operation::text::TextError::Decode(error) => {
ToolError::InvalidArgument(format!("invalid {name} input: {error}"))
}
error => text_error(error),
}
}
#[cfg(test)]
mod tests {
use super::*;
use workdir::http::{WorkdirTransportError, WorkdirTransportErrorCode};
#[test]
fn flattened_file_tool_arguments_keep_raw_json_duplicate_rejection() {
let error = decode_file_input::<crate::edit::EditParams>(
r#"{"file_path":"a.txt","old_string":"a","old_string":"b","new_string":"c"}"#,
"Edit",
)
.unwrap_err();
assert!(
matches!(error, ToolError::InvalidArgument(message) if message.contains("duplicate field `old_string`"))
);
}
#[test]
fn checkout_outcome_unknown_remains_execution_failure() {
let error = ToolError::from(ToolsError::WorkdirSession(
crates/tools/src/read.rs
+20 -50
View File
@@ -26,12 +26,8 @@ pub(crate) struct ReadParams {
pub target_workdir: Option<String>,
/// Workdir-relative path, or an absolute path covered by readable scope.
pub file_path: String,
/// 0-based line offset from the start. Defaults to 0.
#[serde(default)]
pub offset: Option<usize>,
/// Maximum number of lines to return. Defaults to 2000.
#[serde(default)]
pub limit: Option<usize>,
#[serde(flatten)]
pub read: fs_operation::text::LineReadArgs,
}
pub(crate) struct ReadTool {
@@ -46,8 +42,7 @@ impl Tool for ReadTool {
input_json: &str,
_ctx: agen::tool::ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
let params: ReadParams = serde_json::from_str(input_json)
.map_err(|e| ToolError::InvalidArgument(format!("invalid Read input: {e}")))?;
let params: ReadParams = crate::error::decode_file_input(input_json, "Read")?;
let selected = crate::routing::resolve_session(
&self.router,
params.target_workdir.as_deref(),
@@ -75,15 +70,21 @@ pub(crate) async fn execute_read(
tracker: Tracker,
params: ReadParams,
) -> Result<crate::checkout::CheckoutToolOutput, ToolError> {
let offset = params.offset.unwrap_or(0);
let limit = params.limit.unwrap_or(DEFAULT_LIMIT).max(1);
let (offset, limit) = params.read.range(DEFAULT_LIMIT);
let path = &target.path;
tracing::debug!(path = %path, offset, limit, "Read");
let (result, validator) = target.read(offset, limit, PROVIDER_BYTE_LIMIT).await?;
tracker.record_workdir_observation(path, result.content_hash, result.total_lines);
let text = String::from_utf8_lossy(&result.bytes).into_owned();
// Source/response bounds were enforced by the streaming provider. Lossy
// UTF-8 conversion is the existing Tool presentation contract; rendering
// below independently bounds its possible byte expansion.
let text = fs_operation::text::read(
String::from_utf8_lossy(&result.bytes).into_owned(),
Default::default(),
)
.map_err(crate::error::text_error)?;
let rendered = render_provider_read(
&text,
&text.content,
result.start_line,
result.total_lines,
result.truncated,
@@ -111,50 +112,19 @@ pub(crate) async fn execute_read(
})
}
struct Rendered {
body: String,
line_count: usize,
total_lines: usize,
truncated: bool,
}
fn render_provider_read(
text: &str,
start_line: usize,
total_lines: usize,
truncated: bool,
) -> Rendered {
const MARKER: &str = "\n[truncated at rendered output byte limit]\n";
// The numbered presentation is bounded too, not only provider source bytes.
// This fits the native wire limit even with JSON's worst-case escaping.
let budget = PROVIDER_BYTE_LIMIT - MARKER.len();
let mut body = String::with_capacity(text.len().min(PROVIDER_BYTE_LIMIT));
let mut line_count = 0;
let mut output_truncated = false;
for (index, line) in text.lines().enumerate() {
let numbered = format!("{:>6}\t{}\n", start_line + index + 1, line);
let remaining = budget.saturating_sub(body.len());
if remaining > 0 {
line_count += 1;
}
if numbered.len() > remaining {
let mut end = remaining;
while !numbered.is_char_boundary(end) {
end -= 1;
}
body.push_str(&numbered[..end]);
body.push_str(MARKER);
output_truncated = true;
break;
}
body.push_str(&numbered);
}
Rendered {
body,
line_count,
) -> fs_operation::text::Rendered {
fs_operation::text::render_numbered(
text,
start_line,
total_lines,
truncated: start_line > 0 || truncated || output_truncated,
}
truncated,
PROVIDER_BYTE_LIMIT,
)
}
/// Factory for the `Read` tool bound to one compatibility session.
crates/tools/src/write.rs
+9 -5
View File
@@ -22,8 +22,8 @@ pub(crate) struct WriteParams {
pub target_workdir: Option<String>,
/// Logical path relative to the bound Workdir root.
pub file_path: String,
/// Full content to write. Overwrites any existing content.
pub content: String,
#[serde(flatten)]
pub write: fs_operation::text::WriteArgs,
}
pub(crate) struct WriteTool {
@@ -38,8 +38,7 @@ impl Tool for WriteTool {
input_json: &str,
ctx: agen::tool::ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
let params: WriteParams = serde_json::from_str(input_json)
.map_err(|e| ToolError::InvalidArgument(format!("invalid Write input: {e}")))?;
let params: WriteParams = crate::error::decode_file_input(input_json, "Write")?;
let selected = crate::routing::resolve_session(
&self.router,
@@ -57,7 +56,7 @@ impl Tool for WriteTool {
validator: None,
},
tracker,
params.content,
params.write.content,
None,
ctx,
)
@@ -74,6 +73,11 @@ pub(crate) async fn execute_write(
create_path: Option<WorkdirPath>,
ctx: agen::tool::ToolExecutionContext,
) -> Result<crate::checkout::CheckoutToolOutput, ToolError> {
// Provider size/access checks still run inside the checked save. The shared
// core owns text semantics, not the provider's bounds or lifecycle.
let content = fs_operation::text::write(content, Default::default())
.map_err(crate::error::text_error)?
.content;
let path = create_path.as_ref().unwrap_or(&target.path);
tracing::debug!(path = %path, bytes = content.len(), "Write");
let mutation_key = PathBuf::from(path.as_str());