From 8771ef6d8e263a1915cc82334cea15170c8fb7e6 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 11 Sep 2026 15:00:17 -0600 Subject: [PATCH] Move the Read tool's line numbering and the shell quoting wrapper out of fabro-sandbox format_lines_numbered renders a file the way the agent's Read tool shows it: every line prefixed with its number, from an offset for a limit. That is the agent's presentation, not a sandbox concern, and it only lived in fabro-sandbox so RunSandbox::read_file(path, offset, limit) could call it. The function now lives in fabro-agent next to the Read tool, with its tests, and the Read, ReadManyFiles, and Kimi ReadFile tools number the text they get from read_file_text themselves. RunSandbox::read_file goes away; the sandbox returns bytes or text and nothing else. fabro-sandbox also re-exported shell_quote through a one-line wrapper so callers could reach it from the sandbox crate or from fabro-agent. The audited implementation is fabro_util::shell::shell_quote; the six importers now use it directly and the wrapper and both re-exports are gone. Co-Authored-By: Claude Fable 5.1 --- lib/apps/fabro-server/src/run_files.rs | 5 +- lib/components/fabro-acp/tests/session.rs | 28 +++++++--- lib/components/fabro-agent/src/lib.rs | 3 +- .../fabro-agent/src/profiles/kimi_tools.rs | 16 ++++-- lib/components/fabro-agent/src/sandbox.rs | 3 +- lib/components/fabro-agent/src/tools.rs | 44 ++++++++++++++-- .../fabro-sandbox/src/clone_source.rs | 6 ++- .../fabro-sandbox/src/driver_sandbox.rs | 21 ++------ lib/components/fabro-sandbox/src/lib.rs | 3 +- lib/components/fabro-sandbox/src/sandbox.rs | 51 ------------------- .../fabro-workflow/src/handler/llm/acp.rs | 11 ++-- .../src/handler/llm/changed_files.rs | 9 ++-- .../fabro-workflow/src/pipeline/initialize.rs | 2 +- .../fabro-workflow/src/sandbox_git_runtime.rs | 9 ++-- .../tests/it/daytona_integration.rs | 9 ++-- 15 files changed, 109 insertions(+), 111 deletions(-) diff --git a/lib/apps/fabro-server/src/run_files.rs b/lib/apps/fabro-server/src/run_files.rs index ab93b4d6d..d9680931e 100644 --- a/lib/apps/fabro-server/src/run_files.rs +++ b/lib/apps/fabro-server/src/run_files.rs @@ -35,9 +35,10 @@ use fabro_api::types::{ RunFilesMeta, RunFilesMetaDegradedReason, RunFilesMetaScope, RunFilesMetaSource, RunFilesMetaToSha, }; +use fabro_sandbox::Termination; use fabro_sandbox::reconnect::reconnect_for_run; -use fabro_sandbox::{Termination, shell_quote}; use fabro_types::RunId; +use fabro_util::shell; use fabro_workflow::sandbox_git::{ DiffError, DiffNumstat, RawDiffEntry, SubmoduleChange, SymlinkChange, list_changed_files_raw, list_diff_numstat, stream_blob_metadata, stream_blobs, @@ -1204,7 +1205,7 @@ async fn resolve_ref_sha_and_time( sandbox: &RunSandbox, git_ref: &str, ) -> std::result::Result<(String, Option>), ApiError> { - let ref_q = shell_quote(git_ref); + let ref_q = shell::shell_quote(git_ref); let res = sandbox .exec_command( &format!("git -c core.hooksPath=/dev/null show -s --format=%H\\ %cI {ref_q}"), diff --git a/lib/components/fabro-acp/tests/session.rs b/lib/components/fabro-acp/tests/session.rs index f896272b0..5370148c0 100644 --- a/lib/components/fabro-acp/tests/session.rs +++ b/lib/components/fabro-acp/tests/session.rs @@ -10,9 +10,10 @@ use fabro_acp::{ run_acp_turn, }; use fabro_sandbox::test_support::{MockSandbox, MockStdioProcess}; -use fabro_sandbox::{RunSandbox, local_sandbox, shell_quote}; +use fabro_sandbox::{RunSandbox, local_sandbox}; use fabro_types::SteeringMessage; use fabro_util::error::collect_chain; +use fabro_util::shell; use tokio::fs::{read_to_string, write}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader, DuplexStream}; use tokio::process::Command; @@ -104,7 +105,10 @@ async fn session_lifecycle_initializes_sends_prompt_and_aggregates_text() { .await .expect("write fake ACP agent"); - let raw_command = format!("python3 {}", shell_quote(&script_path.to_string_lossy())); + let raw_command = format!( + "python3 {}", + shell::shell_quote(&script_path.to_string_lossy()) + ); let command = AcpProcessSpec::from_command_attr(&raw_command).expect("parse ACP command"); let sandbox: Arc = Arc::new( local_sandbox(tempdir.path().to_path_buf()) @@ -149,7 +153,10 @@ async fn steering_sends_followup_session_prompt_over_acp() { .await .expect("write fake ACP agent"); - let raw_command = format!("python3 {}", shell_quote(&script_path.to_string_lossy())); + let raw_command = format!( + "python3 {}", + shell::shell_quote(&script_path.to_string_lossy()) + ); let command = AcpProcessSpec::from_command_attr(&raw_command).expect("parse ACP command"); let sandbox: Arc = Arc::new( local_sandbox(tempdir.path().to_path_buf()) @@ -216,7 +223,10 @@ async fn interrupt_then_steer_sends_cancel_then_followup_session_prompt_over_acp .await .expect("write fake ACP agent"); - let raw_command = format!("python3 {}", shell_quote(&script_path.to_string_lossy())); + let raw_command = format!( + "python3 {}", + shell::shell_quote(&script_path.to_string_lossy()) + ); let command = AcpProcessSpec::from_command_attr(&raw_command).expect("parse ACP command"); let sandbox: Arc = Arc::new( local_sandbox(tempdir.path().to_path_buf()) @@ -295,7 +305,10 @@ async fn inline_interrupt_terminates_agent_that_ignores_cancel() { .await .expect("write fake ACP agent"); - let raw_command = format!("python3 {}", shell_quote(&script_path.to_string_lossy())); + let raw_command = format!( + "python3 {}", + shell::shell_quote(&script_path.to_string_lossy()) + ); let command = AcpProcessSpec::from_command_attr(&raw_command).expect("parse ACP command"); let sandbox: Arc = Arc::new( local_sandbox(tempdir.path().to_path_buf()) @@ -686,7 +699,10 @@ async fn run_fake_agent_with_activity( write(&script_path, fake_acp_agent_script()) .await .expect("write fake ACP agent"); - let raw_command = format!("python3 {}", shell_quote(&script_path.to_string_lossy())); + let raw_command = format!( + "python3 {}", + shell::shell_quote(&script_path.to_string_lossy()) + ); let command = AcpProcessSpec::from_command_attr(&raw_command).expect("parse ACP command"); let sandbox: Arc = Arc::new( local_sandbox(tempdir.to_path_buf()) diff --git a/lib/components/fabro-agent/src/lib.rs b/lib/components/fabro-agent/src/lib.rs index 82392dbbf..2d6213c3f 100644 --- a/lib/components/fabro-agent/src/lib.rs +++ b/lib/components/fabro-agent/src/lib.rs @@ -58,8 +58,7 @@ pub use sandbox::{ CaptureStats, DirEntry, DriverSpec, ExecControls, ExecResult, ExecResultExt, ExecSpec, ExecStreamingResult, FileKind, GrepMatch, GrepOptions, OutputSink, OutputStream, RunSandbox, SandboxFile, SandboxSource, StderrTail, StdioProcess, StdioProcessHandle, Termination, - TokenProvenance, TokenSnapshot, WalkOptions, command_termination, format_lines_numbered, - program_exit_code, shell_quote, + TokenProvenance, TokenSnapshot, WalkOptions, command_termination, program_exit_code, }; pub use session::{ CompletionCoordinator, Session, SessionControlHandle, SessionInputTiming, diff --git a/lib/components/fabro-agent/src/profiles/kimi_tools.rs b/lib/components/fabro-agent/src/profiles/kimi_tools.rs index 75edb2583..7af367335 100644 --- a/lib/components/fabro-agent/src/profiles/kimi_tools.rs +++ b/lib/components/fabro-agent/src/profiles/kimi_tools.rs @@ -27,11 +27,12 @@ use serde_json::Value; use strum::EnumString; use crate::native_tool::NativeTool; -use crate::sandbox::{ExecResultExt, GrepOptions, Termination, format_lines_numbered}; +use crate::sandbox::{ExecResultExt, GrepOptions, Termination}; use crate::tool_registry::{RegisteredTool, ToolSource}; use crate::tools::{ DEFAULT_READ_LINES, emit_shell_process_completed, execute_grep, execute_shell_command, - grep_result_path, make_edit_file_tool, optional_usize_arg, required_str, retain_shell_output, + format_lines_numbered, grep_result_path, make_edit_file_tool, optional_usize_arg, required_str, + retain_shell_output, }; const DEFAULT_GREP_RESULTS: usize = 250; @@ -230,9 +231,16 @@ depends on an exact file, API, or output shape, inspect the final result before Some(offset) => { let start = usize::try_from(offset) .map_err(|_| "line_offset must fit in usize".to_string())?; - ctx.env.read_file(path, Some(start), Some(n_lines)).await + ctx.env + .read_file_text(path) + .await + .map(|text| format_lines_numbered(&text, Some(start), Some(n_lines))) } - None => ctx.env.read_file(path, None, Some(n_lines)).await, + None => ctx + .env + .read_file_text(path) + .await + .map(|text| format_lines_numbered(&text, None, Some(n_lines))), } .map_err(|e| e.display_with_causes())?; diff --git a/lib/components/fabro-agent/src/sandbox.rs b/lib/components/fabro-agent/src/sandbox.rs index 9cb365649..0e4eb73ea 100644 --- a/lib/components/fabro-agent/src/sandbox.rs +++ b/lib/components/fabro-agent/src/sandbox.rs @@ -3,6 +3,5 @@ pub use fabro_sandbox::{ CaptureStats, DirEntry, DriverSpec, ExecControls, ExecResult, ExecResultExt, ExecSpec, ExecStreamingResult, FileKind, GrepMatch, GrepOptions, OutputSink, OutputStream, RunSandbox, SandboxFile, SandboxSource, StderrTail, StdioProcess, StdioProcessHandle, Termination, - TokenProvenance, TokenSnapshot, WalkOptions, command_termination, format_lines_numbered, - program_exit_code, shell_quote, + TokenProvenance, TokenSnapshot, WalkOptions, command_termination, program_exit_code, }; diff --git a/lib/components/fabro-agent/src/tools.rs b/lib/components/fabro-agent/src/tools.rs index d5dd47466..b3800d5b7 100644 --- a/lib/components/fabro-agent/src/tools.rs +++ b/lib/components/fabro-agent/src/tools.rs @@ -24,6 +24,27 @@ const MAX_WEB_FETCH_BYTES: usize = 100 * 1024; const MAX_READ_MANY_FILES_CONCURRENCY: usize = 8; pub(crate) const DEFAULT_READ_LINES: usize = 2000; +/// The Read tool's rendering of a file: each line prefixed with its 1-based +/// number, right-aligned, from `offset` (1-based) for `limit` lines. +#[must_use] +pub(crate) fn format_lines_numbered( + content: &str, + offset: Option, + limit: Option, +) -> String { + let all_lines: Vec<&str> = content.lines().collect(); + let skip = offset.unwrap_or(1).saturating_sub(1); + let take = limit.unwrap_or(all_lines.len()); + let selected: Vec<&str> = all_lines.into_iter().skip(skip).take(take).collect(); + let width = (skip + selected.len()).to_string().len().max(1); + let mut result = String::new(); + for (i, line) in selected.iter().enumerate() { + let line_num = skip + i + 1; + let _ = writeln!(result, "{line_num:>width$} | {line}"); + } + result +} + /// Configuration for the optional LLM-based summarizer used by `web_fetch`. #[derive(Clone)] pub struct WebFetchSummarizer { @@ -137,12 +158,12 @@ pub fn make_read_file_tool() -> RegisteredTool { let offset_usize = optional_usize_arg(&args, "offset")?; let limit_usize = optional_usize_arg(&args, "limit")?.or(Some(DEFAULT_READ_LINES)); - let content = ctx + let text = ctx .env - .read_file(file_path, offset_usize, limit_usize) + .read_file_text(file_path) .await .map_err(|e| e.display_with_causes())?; - Ok(content) + Ok(format_lines_numbered(&text, offset_usize, limit_usize)) }) }), source: ToolSource::Native, @@ -578,7 +599,10 @@ pub(crate) fn make_read_many_files_tool() -> RegisteredTool { .map(|path| { let env = Arc::clone(&ctx.env); async move { - let result = env.read_file(&path, None, None).await; + let result = env + .read_file_text(&path) + .await + .map(|text| format_lines_numbered(&text, None, None)); (path, result) } }) @@ -758,6 +782,18 @@ mod tests { use crate::config::{NativeToolOptions, SessionOptions, ToolSecrets}; use crate::event::{Emitter, SessionBoundEmitter}; use crate::sandbox::*; + + #[test] + fn format_lines_numbered_numbers_every_line() { + let result = format_lines_numbered("hello\nworld\nfoo", None, None); + assert_eq!(result, "1 | hello\n2 | world\n3 | foo\n"); + } + + #[test] + fn format_lines_numbered_honors_offset_and_limit() { + let result = format_lines_numbered("a\nb\nc\nd\ne", Some(2), Some(2)); + assert_eq!(result, "2 | b\n3 | c\n"); + } use crate::test_support::MockSandbox; use crate::tool_registry::{ToolContext, ToolDefinitionExt}; use crate::types::SessionEvent; diff --git a/lib/components/fabro-sandbox/src/clone_source.rs b/lib/components/fabro-sandbox/src/clone_source.rs index ea112262c..dbd0e3264 100644 --- a/lib/components/fabro-sandbox/src/clone_source.rs +++ b/lib/components/fabro-sandbox/src/clone_source.rs @@ -1,3 +1,5 @@ +use fabro_util::shell; + use crate::sandbox; #[derive(Clone, Debug, PartialEq, Eq)] @@ -68,8 +70,8 @@ fn validate_path_component(label: &str, component: &str) -> crate::Result<()> { pub(crate) fn repo_symlink_command(layout: &GitHubRepoLayout) -> String { format!( "ln -s {} {}", - sandbox::shell_quote(&layout.primary_repo_path), - sandbox::shell_quote(&layout.primary_repo_link), + shell::shell_quote(&layout.primary_repo_path), + shell::shell_quote(&layout.primary_repo_link), ) } diff --git a/lib/components/fabro-sandbox/src/driver_sandbox.rs b/lib/components/fabro-sandbox/src/driver_sandbox.rs index aadca5d0b..6cca51848 100644 --- a/lib/components/fabro-sandbox/src/driver_sandbox.rs +++ b/lib/components/fabro-sandbox/src/driver_sandbox.rs @@ -19,6 +19,7 @@ use std::time::{Duration, Instant}; use fabro_github::GitHubCredentials; use fabro_github::token_source::{InstallationTokenSource, TokenSnapshot}; use fabro_types::SandboxProviderKind; +use fabro_util::shell; use fabro_util::workspace_glob::WorkspaceGlob; use sandbox_driver::{ DirEntry, EventContext, ExecControls, ExecResult, ExecSpec, ExecStreamingResult, FileKind, @@ -659,20 +660,6 @@ impl RunSandbox { .map_err(|err| crate::Error::context("File is not valid UTF-8", err)) } - /// A file's text with line numbers, from `offset` for `limit` lines. - pub async fn read_file( - &self, - path: &str, - offset: Option, - limit: Option, - ) -> crate::Result { - Ok(sandbox::format_lines_numbered( - &self.read_file_text(path).await?, - offset, - limit, - )) - } - pub async fn write_file(&self, path: &str, content: &str) -> crate::Result<()> { self.handle()? .fs() @@ -1006,8 +993,8 @@ impl RunSandbox { } vec![format!( "git fetch origin {} && git checkout {}", - sandbox::shell_quote(run_branch), - sandbox::shell_quote(run_branch) + shell::shell_quote(run_branch), + shell::shell_quote(run_branch) )] } @@ -1186,7 +1173,7 @@ mod tests { assert!(missing.to_string().contains("does not exist"), "{missing}"); let read = f .sandbox - .read_file("nonexistent.txt", None, None) + .read_file_text("nonexistent.txt") .await .unwrap_err(); assert!( diff --git a/lib/components/fabro-sandbox/src/lib.rs b/lib/components/fabro-sandbox/src/lib.rs index 1f215e8a5..c971578bc 100644 --- a/lib/components/fabro-sandbox/src/lib.rs +++ b/lib/components/fabro-sandbox/src/lib.rs @@ -56,8 +56,7 @@ pub use reconnect::{ }; pub use sandbox::{ DEFAULT_EXEC_OUTPUT_TAIL_BYTES, GitRunInfo, GitSetupIntent, PushAttempt, PushError, PushReport, - SandboxFile, SandboxWorkspaceLayout, format_lines_numbered, redacted_output_tail, setup_git, - shell_quote, + SandboxFile, SandboxWorkspaceLayout, redacted_output_tail, setup_git, }; /// Driver types a run sandbox speaks: what a command is and how it ended, /// what the file and search operations return, and what an environment diff --git a/lib/components/fabro-sandbox/src/sandbox.rs b/lib/components/fabro-sandbox/src/sandbox.rs index 8b64ea0e1..7f37c2d52 100644 --- a/lib/components/fabro-sandbox/src/sandbox.rs +++ b/lib/components/fabro-sandbox/src/sandbox.rs @@ -1,9 +1,7 @@ -use std::fmt::Write; use std::time::Duration; use chrono::{DateTime, Utc}; use fabro_github::token_source::TokenSnapshot; -use fabro_util::shell; use sandbox_driver::{ Git as _, GitAttempt, GitCheckoutOptions, GitFetchOptions, GitPushOptions, GitRetryError, GitRetryPolicy, retry_git, @@ -50,25 +48,6 @@ pub enum GitSetupIntent { }, } -/// Formats file content with line numbers for display. -/// -/// Applies optional offset (1-based starting line number) and limit (max lines -/// to return). Line numbers are 1-based and right-aligned. -#[must_use] -pub fn format_lines_numbered(content: &str, offset: Option, limit: Option) -> String { - let all_lines: Vec<&str> = content.lines().collect(); - let skip = offset.unwrap_or(1).saturating_sub(1); - let take = limit.unwrap_or(all_lines.len()); - let selected: Vec<&str> = all_lines.into_iter().skip(skip).take(take).collect(); - let width = (skip + selected.len()).to_string().len().max(1); - let mut result = String::new(); - for (i, line) in selected.iter().enumerate() { - let line_num = skip + i + 1; - let _ = writeln!(result, "{line_num:>width$} | {line}"); - } - result -} - /// Build a redacted `ExecOutputTail` from stdout/stderr text without /// fabricating a synthetic `ExecResult`. Each stream is redacted, then /// capped to its newest `max_bytes_per_stream`. Terminal control sequences @@ -140,13 +119,6 @@ pub(crate) fn join_sandbox_path(base: &str, relative_path: &str) -> String { format!("{}/{relative_path}", base.trim_end_matches('/')) } -/// Shell-quote a string using `shlex::try_quote`, with a fallback for edge -/// cases. Re-exported from [`fabro_util::shell::shell_quote`] so sandbox code -/// and the config resolve layer share one audited implementation. -pub fn shell_quote(s: &str) -> String { - shell::shell_quote(s) -} - /// Creates the run branch in the sandbox's checkout through the driver's /// git facet: a new run branches from `HEAD`, a fork from the source run's /// checkpoint. The branch is created at that base, or moved to it when an @@ -896,8 +868,6 @@ mod push_tests { #[cfg(test)] mod tests { - use super::*; - #[test] fn sandbox_tracing_events_do_not_log_raw_command_or_stdin_fields() { let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); @@ -910,27 +880,6 @@ mod tests { ); } - #[test] - fn format_lines_numbered_basic() { - let result = format_lines_numbered("hello\nworld\nfoo", None, None); - assert_eq!(result, "1 | hello\n2 | world\n3 | foo\n"); - } - - #[test] - fn format_lines_numbered_with_offset_limit() { - let result = format_lines_numbered("a\nb\nc\nd\ne", Some(2), Some(2)); - assert!(result.contains("2 | b")); - assert!(result.contains("3 | c")); - assert!(!result.contains("1 | a")); - assert!(!result.contains("4 | d")); - } - - #[test] - fn shell_quote_basic() { - assert_eq!(shell_quote("hello"), "hello"); - assert_eq!(shell_quote("hello world"), "'hello world'"); - } - #[expect( clippy::disallowed_methods, reason = "unit test performs a small synchronous source scan of local Rust files" diff --git a/lib/components/fabro-workflow/src/handler/llm/acp.rs b/lib/components/fabro-workflow/src/handler/llm/acp.rs index 65f137668..a3ddb6503 100644 --- a/lib/components/fabro-workflow/src/handler/llm/acp.rs +++ b/lib/components/fabro-workflow/src/handler/llm/acp.rs @@ -636,10 +636,11 @@ mod tests { use fabro_acp::test_support::fake_acp_agent_script; use fabro_acp::{AcpError, AcpProcessExit}; - use fabro_agent::{RunSandbox, TokenProvenance, TokenSnapshot, local_sandbox, shell_quote}; + use fabro_agent::{RunSandbox, TokenProvenance, TokenSnapshot, local_sandbox}; use fabro_graphviz::graph::{AttrValue, Node}; use fabro_sandbox::test_support::MockSandbox; use fabro_types::{CommandTermination, EventBody, ExecOutputTail}; + use fabro_util::shell; use tokio_util::sync::CancellationToken; use super::{ @@ -931,7 +932,7 @@ mod tests { "acp.command".to_string(), AttrValue::String(format!( "python3 {}", - shell_quote(&script_path.to_string_lossy()) + shell::shell_quote(&script_path.to_string_lossy()) )), ); @@ -1037,7 +1038,7 @@ mod tests { "acp.command".to_string(), AttrValue::String(format!( "python3 {}", - shell_quote(&script_path.to_string_lossy()) + shell::shell_quote(&script_path.to_string_lossy()) )), ); @@ -1100,7 +1101,7 @@ mod tests { "acp.command".to_string(), AttrValue::String(format!( "python3 {}", - shell_quote(&script_path.to_string_lossy()) + shell::shell_quote(&script_path.to_string_lossy()) )), ); @@ -1184,7 +1185,7 @@ mod tests { "acp.command".to_string(), AttrValue::String(format!( "python3 {}", - shell_quote(&script_path.to_string_lossy()) + shell::shell_quote(&script_path.to_string_lossy()) )), ); diff --git a/lib/components/fabro-workflow/src/handler/llm/changed_files.rs b/lib/components/fabro-workflow/src/handler/llm/changed_files.rs index 51c003efd..716eaf3a2 100644 --- a/lib/components/fabro-workflow/src/handler/llm/changed_files.rs +++ b/lib/components/fabro-workflow/src/handler/llm/changed_files.rs @@ -1,7 +1,8 @@ use std::collections::HashSet; use std::sync::Arc; -use fabro_agent::{RunSandbox, shell_quote}; +use fabro_agent::RunSandbox; +use fabro_util::shell; use sandbox_driver::{Git as _, GitDiffOptions, GitRevisionRange}; /// The paths the working tree changed against `HEAD`, plus the untracked @@ -43,8 +44,10 @@ pub async fn files_touched_since( let last_file_touched = if files_touched.is_empty() { None } else { - let quoted_files: Vec = - files_touched.iter().map(|file| shell_quote(file)).collect(); + let quoted_files: Vec = files_touched + .iter() + .map(|file| shell::shell_quote(file)) + .collect(); let cmd = format!("ls -t {} | head -1", quoted_files.join(" ")); sandbox .exec_command(&cmd, 5_000, None, None, None) diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index 780bb719c..055557c8f 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -1357,7 +1357,7 @@ mod tests { "acp.command".to_string(), AttrValue::String(format!( "python3 {}", - fabro_sandbox::shell_quote(&script_path.to_string_lossy()) + fabro_util::shell::shell_quote(&script_path.to_string_lossy()) )), ); let mut exit = Node::new("exit"); diff --git a/lib/components/fabro-workflow/src/sandbox_git_runtime.rs b/lib/components/fabro-workflow/src/sandbox_git_runtime.rs index 97b5daec2..0a735202b 100644 --- a/lib/components/fabro-workflow/src/sandbox_git_runtime.rs +++ b/lib/components/fabro-workflow/src/sandbox_git_runtime.rs @@ -1,6 +1,7 @@ use fabro_agent::RunSandbox; -use fabro_sandbox::{ExecResult, ExecResultExt, Termination, shell_quote}; +use fabro_sandbox::{ExecResult, ExecResultExt, Termination}; use fabro_util::error::SharedError; +use fabro_util::shell; use tokio::sync::OnceCell; use crate::sandbox_git::GitCommandError; @@ -66,9 +67,9 @@ async fn probe_sandbox_git(sandbox: &RunSandbox) -> Result<(), SharedError> { GIT_INDEX_FILE={index_q} {git} update-index --add --cacheinfo 100644,$blob,probe.txt\n\ GIT_INDEX_FILE={index_q} {git} write-tree >/dev/null\n\ rm -rf {temp_q}", - temp_q = shell_quote(&temp), - probe_file_q = shell_quote(&probe_file), - index_q = shell_quote(&index), + temp_q = shell::shell_quote(&temp), + probe_file_q = shell::shell_quote(&probe_file), + index_q = shell::shell_quote(&index), git = "git -c maintenance.auto=0 -c gc.auto=0", ); exec_ok(sandbox, &command).await diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index 02bcb096b..e6fd9a815 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -376,7 +376,7 @@ async fn daytona_file_round_trip() { assert!(env.file_exists(test_path).await.unwrap()); // Read - let read_back = env.read_file(test_path, None, None).await.unwrap(); + let read_back = env.read_file_text(test_path).await.unwrap(); assert!(read_back.contains(content)); // Delete @@ -491,7 +491,7 @@ async fn daytona_artifact_sync_uploads_and_rewrites_pointer() { "artifact file should exist in Daytona sandbox at {remote_path}" ); - let remote_content = env.read_file(remote_path, None, None).await.unwrap(); + let remote_content = env.read_file_text(remote_path).await.unwrap(); assert!( remote_content.len() > 100 * 1024, "remote artifact should be >100KB, got {} bytes", @@ -1593,10 +1593,7 @@ async fn daytona_cp_upload_download_round_trip() { env.file_exists("cp_test_upload.txt").await.unwrap(), "uploaded file should exist in the sandbox" ); - let remote_content = env - .read_file("cp_test_upload.txt", None, None) - .await - .unwrap(); + let remote_content = env.read_file_text("cp_test_upload.txt").await.unwrap(); assert!( remote_content.contains("hello from fabro cp e2e test"), "expected uploaded content in sandbox, got: {remote_content}"