Add FS abstraction and use in view_image#14960
Conversation
# Conflicts: # codex-rs/core/src/tools/handlers/view_image.rs
| /// Session-scoped model client shared across turns. | ||
| pub(crate) model_client: ModelClient, | ||
| pub(crate) code_mode_service: CodeModeService, | ||
| pub(crate) environment: Arc<Environment>, |
There was a problem hiding this comment.
This is intended to be an ExecutorEnvironment, is that right?
There was a problem hiding this comment.
yes, do you want it renamed? type or type and field?
|
|
||
| if !metadata.is_file() { | ||
| let abs_path = | ||
| AbsolutePathBuf::try_from(turn.resolve_path(Some(args.path))).map_err(|error| { |
There was a problem hiding this comment.
Hmm, so ultimately, this view_image tool should be run on the executor? So resolve_path() will work since it will be speaking in terms of its own local path?
Though I don't know if it will ultimately have TurnContext?
There was a problem hiding this comment.
We'll move resolve_path to be remote path aware.
| } | ||
| } | ||
|
|
||
| fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> { |
There was a problem hiding this comment.
I see, given the recursion, I guess sync makes this all a bit simpler. Maybe we add a _blocking suffix to this name?
| source: &Path, | ||
| destination: &Path, | ||
| ) -> io::Result<bool> { | ||
| let source = std::fs::canonicalize(source)?; |
There was a problem hiding this comment.
Hmm, this is tricky. Canonicalization is not always what you want here?
| } | ||
| } | ||
|
|
||
| fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> { |
There was a problem hiding this comment.
Should we just use fs_extra?
3 participants
Adds an environment crate and environment + file system abstraction.
Environment is a combination of attributes and services specific to environment the agent is connected to:
File system, process management, OS, default shell.
The goal is to move most of agent logic that assumes environment to work through the environment abstraction.