diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 3578fd8c6..a05a5af36 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2878,7 +2878,6 @@ name = "codex-utils-absolute-path" version = "0.0.0" dependencies = [ "dirs", - "path-absolutize", "pretty_assertions", "schemars 0.8.22", "serde", diff --git a/codex-rs/app-server/src/fs_watch.rs b/codex-rs/app-server/src/fs_watch.rs index 309bee4a6..c8214c2b8 100644 --- a/codex-rs/app-server/src/fs_watch.rs +++ b/codex-rs/app-server/src/fs_watch.rs @@ -158,19 +158,7 @@ impl FsWatchManager { let mut changed_paths = event .paths .into_iter() - .filter_map(|path| { - match AbsolutePathBuf::resolve_path_against_base(&path, &watch_root) { - Ok(path) => Some(path), - Err(err) => { - warn!( - "failed to normalize watch event path ({}) for {}: {err}", - path.display(), - watch_root.display() - ); - None - } - } - }) + .map(|path| AbsolutePathBuf::resolve_path_against_base(&path, &watch_root)) .collect::>(); changed_paths.sort_by(|left, right| left.as_path().cmp(right.as_path())); if !changed_paths.is_empty() { diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index 23c9a6c6c..99e33eb6b 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -546,7 +546,7 @@ model = "gpt-old" ) .await??; let write: ConfigWriteResponse = to_response(write_resp)?; - let expected_file_path = AbsolutePathBuf::resolve_path_against_base("config.toml", codex_home)?; + let expected_file_path = AbsolutePathBuf::resolve_path_against_base("config.toml", codex_home); assert_eq!(write.status, WriteStatus::Ok); assert_eq!(write.file_path, expected_file_path); @@ -647,7 +647,7 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> { .await??; let batch_write: ConfigWriteResponse = to_response(batch_resp)?; assert_eq!(batch_write.status, WriteStatus::Ok); - let expected_file_path = AbsolutePathBuf::resolve_path_against_base("config.toml", codex_home)?; + let expected_file_path = AbsolutePathBuf::resolve_path_against_base("config.toml", codex_home); assert_eq!(batch_write.file_path, expected_file_path); let read_id = mcp diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index ab6324efa..aef0a92d7 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -528,64 +528,60 @@ async fn handle_patch_approval( } = event; let approval_id = call_id.clone(); let guardian_decision = if routes_approval_to_guardian(parent_ctx) { - let maybe_files = changes + let files = changes .keys() - .map(|path| parent_ctx.cwd.join(path).ok()) - .collect::>>(); - if let Some(files) = maybe_files { - let review_cancel = cancel_token.child_token(); - let patch = changes - .iter() - .map(|(path, change)| match change { - codex_protocol::protocol::FileChange::Add { content } => { - format!("*** Add File: {}\n{}", path.display(), content) + .map(|path| parent_ctx.cwd.join(path)) + .collect::>(); + let review_cancel = cancel_token.child_token(); + let patch = changes + .iter() + .map(|(path, change)| match change { + codex_protocol::protocol::FileChange::Add { content } => { + format!("*** Add File: {}\n{}", path.display(), content) + } + codex_protocol::protocol::FileChange::Delete { content } => { + format!("*** Delete File: {}\n{}", path.display(), content) + } + codex_protocol::protocol::FileChange::Update { + unified_diff, + move_path, + } => { + if let Some(move_path) = move_path { + format!( + "*** Update File: {}\n*** Move to: {}\n{}", + path.display(), + move_path.display(), + unified_diff + ) + } else { + format!("*** Update File: {}\n{}", path.display(), unified_diff) } - codex_protocol::protocol::FileChange::Delete { content } => { - format!("*** Delete File: {}\n{}", path.display(), content) - } - codex_protocol::protocol::FileChange::Update { - unified_diff, - move_path, - } => { - if let Some(move_path) = move_path { - format!( - "*** Update File: {}\n*** Move to: {}\n{}", - path.display(), - move_path.display(), - unified_diff - ) - } else { - format!("*** Update File: {}\n{}", path.display(), unified_diff) - } - } - }) - .collect::>() - .join("\n"); - let review_rx = spawn_guardian_review( - Arc::clone(parent_session), - Arc::clone(parent_ctx), - GuardianApprovalRequest::ApplyPatch { - id: approval_id.clone(), - cwd: parent_ctx.cwd.to_path_buf(), - files, - patch, - }, - reason.clone(), - review_cancel.clone(), - ); - Some( - await_approval_with_cancel( - async move { review_rx.await.unwrap_or_default() }, - parent_session, - &approval_id, - cancel_token, - Some(&review_cancel), - ) - .await, + } + }) + .collect::>() + .join("\n"); + let review_rx = spawn_guardian_review( + Arc::clone(parent_session), + Arc::clone(parent_ctx), + GuardianApprovalRequest::ApplyPatch { + id: approval_id.clone(), + cwd: parent_ctx.cwd.to_path_buf(), + files, + patch, + }, + reason.clone(), + review_cancel.clone(), + ); + Some( + await_approval_with_cancel( + async move { review_rx.await.unwrap_or_default() }, + parent_session, + &approval_id, + cancel_token, + Some(&review_cancel), ) - } else { - None - } + .await, + ) } else { None }; diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 6e11dd825..29fe9df89 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2512,10 +2512,7 @@ async fn session_configuration_apply_rederives_legacy_file_system_policy_on_cwd_ #[tokio::test] async fn session_update_settings_keeps_runtime_cwds_absolute() { let (session, turn_context) = make_session_and_context().await; - let updated_cwd = turn_context - .cwd - .join("project") - .expect("resolve project dir"); + let updated_cwd = turn_context.cwd.join("project"); std::fs::create_dir_all(updated_cwd.as_path()).expect("create project dir"); session diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 84873baf0..3e9626866 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1449,7 +1449,7 @@ impl Config { let mut additional_writable_roots: Vec = additional_writable_roots .into_iter() .map(|path| AbsolutePathBuf::resolve_path_against_base(path, resolved_cwd.as_path())) - .collect::, _>>()?; + .collect(); let active_project = cfg .get_active_project(resolved_cwd.as_path()) .unwrap_or(ProjectConfig { trust_level: None }); diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index f8284d4ae..826d0139b 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -182,7 +182,7 @@ fn compile_scoped_filesystem_path( let subpath = parse_relative_subpath(subpath)?; let base = parse_absolute_path(path)?; - let path = AbsolutePathBuf::resolve_path_against_base(&subpath, base.as_path())?; + let path = AbsolutePathBuf::resolve_path_against_base(&subpath, base.as_path()); Ok(FileSystemPath::Path { path }) } diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index d2426b138..80d4b6bdd 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -265,8 +265,7 @@ impl ConfigService { edits: Vec<(String, JsonValue, MergeStrategy)>, ) -> Result { let allowed_path = - AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, &self.codex_home) - .map_err(|err| ConfigServiceError::io("failed to resolve user config path", err))?; + AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, &self.codex_home); let provided_path = match file_path { Some(path) => AbsolutePathBuf::from_absolute_path(PathBuf::from(path)) .map_err(|err| ConfigServiceError::io("failed to resolve user config path", err))?, diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 8961c00aa..f40482fd5 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -187,7 +187,7 @@ pub async fn load_config_layers_state( // Add a layer for $CODEX_HOME/config.toml if it exists. Note if the file // exists, but is malformed, then this error should be propagated to the // user. - let user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home)?; + let user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home); let user_layer = load_config_toml_for_required_layer(&user_file, |config_toml| { ConfigLayerEntry::new( ConfigLayerSource::User { @@ -809,7 +809,7 @@ async fn load_project_layers( if dot_codex_abs == codex_home_abs || dot_codex_normalized == codex_home_normalized { continue; } - let config_file = dot_codex_abs.join(CONFIG_TOML_FILE)?; + let config_file = dot_codex_abs.join(CONFIG_TOML_FILE); match tokio::fs::read_to_string(&config_file).await { Ok(contents) => { let config: TomlValue = match toml::from_str(&contents) { @@ -945,7 +945,7 @@ foo = "xyzzy" expected_toml_value.insert( "model_instructions_file".to_string(), TomlValue::String( - AbsolutePathBuf::resolve_path_against_base("./some_file.md", base_dir)? + AbsolutePathBuf::resolve_path_against_base("./some_file.md", base_dir) .as_path() .to_string_lossy() .to_string(), diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 629730cbf..d476b8e6f 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -78,7 +78,7 @@ async fn cli_overrides_resolve_relative_paths_against_cwd() -> std::io::Result<( .build() .await?; - let expected = AbsolutePathBuf::resolve_path_against_base("run-logs", cwd_path)?; + let expected = AbsolutePathBuf::resolve_path_against_base("run-logs", cwd_path); assert_eq!(config.log_dir, expected.to_path_buf()); Ok(()) } @@ -250,7 +250,6 @@ async fn returns_empty_when_all_layers_missing() { &ConfigLayerEntry { name: super::ConfigLayerSource::User { file: AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, tmp.path()) - .expect("resolve user config.toml path") }, config: TomlValue::Table(toml::map::Map::new()), raw_toml: None, diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index dbfab81d3..98bb815e9 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -494,8 +494,7 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result Vec { let path = match &layer.name { ConfigLayerSource::System { file } => Some(file.as_path().to_path_buf()), ConfigLayerSource::User { file } => Some(file.as_path().to_path_buf()), - ConfigLayerSource::Project { dot_codex_folder } => dot_codex_folder - .join(CONFIG_TOML_FILE) - .ok() - .map(|p| p.as_path().to_path_buf()), + ConfigLayerSource::Project { dot_codex_folder } => Some( + dot_codex_folder + .join(CONFIG_TOML_FILE) + .as_path() + .to_path_buf(), + ), ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => { Some(file.as_path().to_path_buf()) } diff --git a/codex-rs/core/src/plugins/marketplace.rs b/codex-rs/core/src/plugins/marketplace.rs index ee775262a..0d7c68c1e 100644 --- a/codex-rs/core/src/plugins/marketplace.rs +++ b/codex-rs/core/src/plugins/marketplace.rs @@ -299,20 +299,18 @@ fn discover_marketplace_paths_from_roots( for root in additional_roots { // Curated marketplaces can now come from an HTTP-downloaded directory that is not a git // checkout, so check the root directly before falling back to repo-root discovery. - if let Ok(path) = root.join(MARKETPLACE_RELATIVE_PATH) - && path.as_path().is_file() - && !paths.contains(&path) - { + let path = root.join(MARKETPLACE_RELATIVE_PATH); + if path.as_path().is_file() && !paths.contains(&path) { paths.push(path); continue; } if let Some(repo_root) = get_git_repo_root(root.as_path()) && let Ok(repo_root) = AbsolutePathBuf::try_from(repo_root) - && let Ok(path) = repo_root.join(MARKETPLACE_RELATIVE_PATH) - && path.as_path().is_file() - && !paths.contains(&path) { - paths.push(path); + let path = repo_root.join(MARKETPLACE_RELATIVE_PATH); + if path.as_path().is_file() && !paths.contains(&path) { + paths.push(path); + } } } @@ -370,12 +368,7 @@ fn resolve_plugin_source_path( // `marketplace.json` lives under `/.agents/plugins/`, but local plugin paths // are resolved relative to ``, not relative to the `plugins/` directory. - marketplace_root_dir(marketplace_path)? - .join(relative_source_path) - .map_err(|err| MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: format!("plugin source path must resolve to an absolute path: {err}"), - }) + Ok(marketplace_root_dir(marketplace_path)?.join(relative_source_path)) } } } diff --git a/codex-rs/core/src/plugins/marketplace_tests.rs b/codex-rs/core/src/plugins/marketplace_tests.rs index d3cf55425..226ae3ff6 100644 --- a/codex-rs/core/src/plugins/marketplace_tests.rs +++ b/codex-rs/core/src/plugins/marketplace_tests.rs @@ -756,18 +756,16 @@ fn resolve_marketplace_plugin_rejects_non_relative_local_paths() { ) .unwrap(); - let err = resolve_marketplace_plugin( - &AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(), - "local-plugin", - Some(Product::Codex), - ) - .unwrap_err(); + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(); + let err = resolve_marketplace_plugin(&marketplace_path, "local-plugin", Some(Product::Codex)) + .unwrap_err(); assert_eq!( err.to_string(), format!( "invalid marketplace file `{}`: local plugin source path must start with `./`", - repo_root.join(".agents/plugins/marketplace.json").display() + marketplace_path.display() ) ); } diff --git a/codex-rs/core/src/project_doc.rs b/codex-rs/core/src/project_doc.rs index 236c25a3c..e7321695a 100644 --- a/codex-rs/core/src/project_doc.rs +++ b/codex-rs/core/src/project_doc.rs @@ -288,7 +288,7 @@ pub async fn discover_project_doc_paths( let candidate_filenames = candidate_filenames(config); for d in search_dirs { for name in &candidate_filenames { - let candidate = d.join(name)?; + let candidate = d.join(name); match fs.get_metadata(&candidate).await { Ok(md) if md.is_file => { found.push(candidate); diff --git a/codex-rs/core/src/project_doc_tests.rs b/codex-rs/core/src/project_doc_tests.rs index be9cb44b2..c8caf2ff9 100644 --- a/codex-rs/core/src/project_doc_tests.rs +++ b/codex-rs/core/src/project_doc_tests.rs @@ -334,8 +334,7 @@ async fn project_root_markers_are_honored_for_agents_discovery() { ) .expect("absolute parent doc path"); let expected_child = AbsolutePathBuf::try_from( - dunce::canonicalize(cfg.cwd.join("AGENTS.md").expect("absolute child doc path")) - .expect("canonical child doc path"), + dunce::canonicalize(cfg.cwd.join("AGENTS.md")).expect("canonical child doc path"), ) .expect("absolute child doc path"); assert_eq!(discovery.len(), 2); diff --git a/codex-rs/core/src/safety_tests.rs b/codex-rs/core/src/safety_tests.rs index 5ebeb64e0..6e5795e59 100644 --- a/codex-rs/core/src/safety_tests.rs +++ b/codex-rs/core/src/safety_tests.rs @@ -243,7 +243,7 @@ fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() { let tmp = TempDir::new().unwrap(); let cwd = tmp.path().to_path_buf(); let blocked_path = cwd.join("docs").join("blocked.txt"); - let docs_absolute = AbsolutePathBuf::resolve_path_against_base("docs", &cwd).unwrap(); + let docs_absolute = AbsolutePathBuf::resolve_path_against_base("docs", &cwd); let action = ApplyPatchAction::new_add_for_test(&blocked_path, "".to_string()); let sandbox_policy = SandboxPolicy::ExternalSandbox { network_access: codex_protocol::protocol::NetworkAccess::Restricted, diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 00b1173aa..a972566b9 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -56,7 +56,7 @@ fn file_paths_for_action(action: &ApplyPatchAction) -> Vec { } fn to_abs_path(cwd: &Path, path: &Path) -> Option { - AbsolutePathBuf::resolve_path_against_base(path, cwd).ok() + Some(AbsolutePathBuf::resolve_path_against_base(path, cwd)) } fn write_permissions_for_paths( diff --git a/codex-rs/core/src/tools/js_repl/mod_tests.rs b/codex-rs/core/src/tools/js_repl/mod_tests.rs index bbc0f50dc..bdf693133 100644 --- a/codex-rs/core/src/tools/js_repl/mod_tests.rs +++ b/codex-rs/core/src/tools/js_repl/mod_tests.rs @@ -1028,7 +1028,7 @@ async fn js_repl_waits_for_unawaited_tool_calls_before_completion() -> anyhow::R let marker = turn .cwd - .join(format!("js-repl-unawaited-marker-{}.txt", Uuid::new_v4()))?; + .join(format!("js-repl-unawaited-marker-{}.txt", Uuid::new_v4())); let marker_json = serde_json::to_string(&marker.to_string_lossy().to_string())?; let result = manager .execute( @@ -1073,10 +1073,10 @@ async fn js_repl_persisted_tool_helpers_work_across_cells() -> anyhow::Result<() let global_marker = turn .cwd - .join(format!("js-repl-global-helper-{}.txt", Uuid::new_v4()))?; + .join(format!("js-repl-global-helper-{}.txt", Uuid::new_v4())); let lexical_marker = turn .cwd - .join(format!("js-repl-lexical-helper-{}.txt", Uuid::new_v4()))?; + .join(format!("js-repl-lexical-helper-{}.txt", Uuid::new_v4())); let global_marker_json = serde_json::to_string(&global_marker.to_string_lossy().to_string())?; let lexical_marker_json = serde_json::to_string(&lexical_marker.to_string_lossy().to_string())?; diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index 7fbcc0703..ed62c9038 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -31,10 +31,8 @@ async fn agents_instructions(mut builder: TestCodexBuilder) -> Result { async fn agents_override_is_preferred_over_agents_md() -> Result<()> { let instructions = agents_instructions(test_codex().with_workspace_setup(|cwd, fs| async move { - let agents_md = cwd.join("AGENTS.md").expect("absolute AGENTS.md path"); - let override_md = cwd - .join("AGENTS.override.md") - .expect("absolute AGENTS.override.md path"); + let agents_md = cwd.join("AGENTS.md"); + let override_md = cwd.join("AGENTS.override.md"); fs.write_file(&agents_md, b"base doc".to_vec()).await?; fs.write_file(&override_md, b"override doc".to_vec()) .await?; @@ -62,8 +60,8 @@ async fn configured_fallback_is_used_when_agents_candidate_is_directory() -> Res config.project_doc_fallback_filenames = vec!["WORKFLOW.md".to_string()]; }) .with_workspace_setup(|cwd, fs| async move { - let agents_dir = cwd.join("AGENTS.md").expect("absolute AGENTS.md path"); - let fallback = cwd.join("WORKFLOW.md").expect("absolute WORKFLOW.md path"); + let agents_dir = cwd.join("AGENTS.md"); + let fallback = cwd.join("WORKFLOW.md"); fs.create_directory(&agents_dir, CreateDirectoryOptions { recursive: true }) .await?; fs.write_file(&fallback, b"fallback doc".to_vec()).await?; @@ -85,10 +83,7 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> { let instructions = agents_instructions( test_codex() .with_config(|config| { - config.cwd = config - .cwd - .join("nested/workspace") - .expect("absolute nested workspace path"); + config.cwd = config.cwd.join("nested/workspace"); }) .with_workspace_setup(|cwd, fs| async move { let nested = cwd.clone(); @@ -96,13 +91,9 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> { .parent() .and_then(|parent| parent.parent()) .expect("nested workspace should have a project root ancestor"); - let root_agents = root - .join("AGENTS.md") - .expect("absolute root AGENTS.md path"); - let git_marker = root.join(".git").expect("absolute .git path"); - let nested_agents = nested - .join("AGENTS.md") - .expect("absolute nested AGENTS.md path"); + let root_agents = root.join("AGENTS.md"); + let git_marker = root.join(".git"); + let nested_agents = nested.join("AGENTS.md"); fs.create_directory(&nested, CreateDirectoryOptions { recursive: true }) .await?; diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index d47ed7c94..8f4f2ae75 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -567,7 +567,7 @@ async fn snapshot_rollback_followup_turn_trims_context_updates() -> Result<()> { user_turn(&conversation, TURN_ONE_USER).await; - let override_cwd = config.cwd.join(PRETURN_CONTEXT_DIFF_CWD)?; + let override_cwd = config.cwd.join(PRETURN_CONTEXT_DIFF_CWD); std::fs::create_dir_all(&override_cwd)?; conversation .submit(Op::OverrideTurnContext { diff --git a/codex-rs/core/tests/suite/hierarchical_agents.rs b/codex-rs/core/tests/suite/hierarchical_agents.rs index bddc1a527..f0960c072 100644 --- a/codex-rs/core/tests/suite/hierarchical_agents.rs +++ b/codex-rs/core/tests/suite/hierarchical_agents.rs @@ -26,7 +26,7 @@ async fn hierarchical_agents_appends_to_project_doc_in_user_instructions() { .expect("test config should allow feature update"); }) .with_workspace_setup(|cwd, fs| async move { - let agents_md = cwd.join("AGENTS.md").expect("absolute AGENTS.md path"); + let agents_md = cwd.join("AGENTS.md"); fs.write_file(&agents_md, b"be nice".to_vec()).await?; Ok::<(), anyhow::Error>(()) }); diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index f16d3c2b1..b213e305e 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -85,7 +85,7 @@ fn png_bytes(width: u32, height: u32, rgba: [u8; 4]) -> anyhow::Result> } async fn create_workspace_directory(test: &TestCodex, rel_path: &str) -> anyhow::Result { - let abs_path = test.config.cwd.join(rel_path)?; + let abs_path = test.config.cwd.join(rel_path); test.fs() .create_directory(&abs_path, CreateDirectoryOptions { recursive: true }) .await?; @@ -97,7 +97,7 @@ async fn write_workspace_file( rel_path: &str, contents: Vec, ) -> anyhow::Result { - let abs_path = test.config.cwd.join(rel_path)?; + let abs_path = test.config.cwd.join(rel_path); if let Some(parent) = abs_path.parent() { test.fs() .create_directory(&parent, CreateDirectoryOptions { recursive: true }) @@ -226,7 +226,7 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { let cwd = config.cwd.clone(); let rel_path = "assets/example.png"; - let abs_path = cwd.join(rel_path)?; + let abs_path = cwd.join(rel_path); let original_width = 2304; let original_height = 864; write_workspace_png( @@ -1258,7 +1258,7 @@ async fn view_image_tool_errors_when_file_missing() -> anyhow::Result<()> { } = &test; let rel_path = "missing/example.png"; - let abs_path = config.cwd.join(rel_path)?; + let abs_path = config.cwd.join(rel_path); let call_id = "view-image-missing"; let arguments = serde_json::json!({ "path": rel_path }).to_string(); diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 06e39b565..048a8cedb 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -632,9 +632,7 @@ pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option { } let git_dir_path = canonicalize_or_raw( - AbsolutePathBuf::resolve_path_against_base(git_dir_rel, &repo_root) - .ok()? - .into_path_buf(), + AbsolutePathBuf::resolve_path_against_base(git_dir_rel, &repo_root).into_path_buf(), ); let worktrees_dir = git_dir_path.parent()?; if worktrees_dir.file_name() != Some(OsStr::new("worktrees")) { diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index f39eb7743..465caa376 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -35,16 +35,7 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - let Some(folder) = layer.config_folder() else { continue; }; - let source_path = match folder.join("hooks.json") { - Ok(source_path) => source_path, - Err(err) => { - warnings.push(format!( - "failed to resolve hooks config path from {}: {err}", - folder.display() - )); - continue; - } - }; + let source_path = folder.join("hooks.json"); if !source_path.as_path().is_file() { continue; } diff --git a/codex-rs/model-provider-info/src/model_provider_info_tests.rs b/codex-rs/model-provider-info/src/model_provider_info_tests.rs index e456a6a5c..bdc2e73b1 100644 --- a/codex-rs/model-provider-info/src/model_provider_info_tests.rs +++ b/codex-rs/model-provider-info/src/model_provider_info_tests.rs @@ -152,7 +152,7 @@ args = ["--format=text"] args: vec!["--format=text".to_string()], timeout_ms: NonZeroU64::new(5_000).unwrap(), refresh_interval_ms: 300_000, - cwd: AbsolutePathBuf::resolve_path_against_base(".", base_dir.path()).unwrap(), + cwd: AbsolutePathBuf::resolve_path_against_base(".", base_dir.path()), }) ); } diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 4aa1c7b36..416d291fd 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -522,7 +522,7 @@ impl FileSystemSandboxPolicy { if suffix.as_os_str().is_empty() { return None; } - root.join(suffix).ok() + Some(root.join(suffix)) }) } } else { @@ -921,7 +921,7 @@ fn resolve_candidate_path(path: &Path, cwd: &Path) -> Option { if path.is_absolute() { AbsolutePathBuf::from_absolute_path(path).ok() } else { - AbsolutePathBuf::resolve_path_against_base(path, cwd).ok() + Some(AbsolutePathBuf::resolve_path_against_base(path, cwd)) } } @@ -1032,9 +1032,10 @@ fn resolve_file_system_special_path( FileSystemSpecialPath::ProjectRoots { subpath } => { let cwd = cwd?; match subpath.as_ref() { - Some(subpath) => { - AbsolutePathBuf::resolve_path_against_base(subpath, cwd.as_path()).ok() - } + Some(subpath) => Some(AbsolutePathBuf::resolve_path_against_base( + subpath, + cwd.as_path(), + )), None => Some(cwd.clone()), } } @@ -1100,10 +1101,7 @@ fn default_read_only_subpaths_for_writable_root( protect_missing_dot_codex: bool, ) -> Vec { let mut subpaths: Vec = Vec::new(); - #[allow(clippy::expect_used)] - let top_level_git = writable_root - .join(".git") - .expect(".git is a valid relative path"); + let top_level_git = writable_root.join(".git"); // This applies to typical repos (directory .git), worktrees/submodules // (file .git with gitdir pointer), and bare repos when the gitdir is the // writable root itself. @@ -1119,8 +1117,7 @@ fn default_read_only_subpaths_for_writable_root( subpaths.push(top_level_git); } - #[allow(clippy::expect_used)] - let top_level_agents = writable_root.join(".agents").expect("valid relative path"); + let top_level_agents = writable_root.join(".agents"); if top_level_agents.as_path().is_dir() { subpaths.push(top_level_agents); } @@ -1129,8 +1126,7 @@ fn default_read_only_subpaths_for_writable_root( // default. For the workspace root itself, protect it even before the // directory exists so first-time creation still goes through the // protected-path approval flow. - #[allow(clippy::expect_used)] - let top_level_codex = writable_root.join(".codex").expect("valid relative path"); + let top_level_codex = writable_root.join(".codex"); if protect_missing_dot_codex || top_level_codex.as_path().is_dir() { subpaths.push(top_level_codex); } @@ -1227,16 +1223,7 @@ fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option path, - Err(err) => { - error!( - "Failed to resolve gitdir path {gitdir_raw} from {path}: {err}", - path = dot_git.as_path().display() - ); - return None; - } - }; + let gitdir_path = AbsolutePathBuf::resolve_path_against_base(gitdir_raw, base); if !gitdir_path.as_path().exists() { error!( "Resolved gitdir path {path} does not exist.", @@ -1302,7 +1289,7 @@ mod tests { cwd.path().canonicalize().expect("canonicalize cwd"), ) .expect("absolute canonical root"); - let expected_dot_codex = expected_root.join(".codex").expect("expected .codex path"); + let expected_dot_codex = expected_root.join(".codex"); let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -1329,7 +1316,7 @@ mod tests { cwd.path().canonicalize().expect("canonicalize cwd"), ) .expect("absolute canonical root"); - let explicit_dot_codex = expected_root.join(".codex").expect("expected .codex path"); + let explicit_dot_codex = expected_root.join(".codex"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { @@ -1359,10 +1346,7 @@ mod tests { ); assert!( policy.can_write_path_with_cwd( - explicit_dot_codex - .join("config.toml") - .expect("config.toml") - .as_path(), + explicit_dot_codex.join("config.toml").as_path(), cwd.path() ) ); @@ -1451,7 +1435,7 @@ mod tests { let link_root = AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); - let link_blocked = link_root.join("blocked").expect("symlinked blocked path"); + let link_blocked = link_root.join("blocked"); let expected_root = AbsolutePathBuf::from_absolute_path( real_root.canonicalize().expect("canonicalize real root"), ) @@ -1632,16 +1616,12 @@ mod tests { let link_root = AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); - let link_private = link_root - .join("linked-private") - .expect("symlinked linked-private path"); + let link_private = link_root.join("linked-private"); let expected_root = AbsolutePathBuf::from_absolute_path( real_root.canonicalize().expect("canonicalize real root"), ) .expect("absolute canonical root"); - let expected_linked_private = expected_root - .join("linked-private") - .expect("expected linked-private path"); + let expected_linked_private = expected_root.join("linked-private"); let unexpected_decoy = AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) .expect("absolute canonical decoy"); @@ -1686,16 +1666,12 @@ mod tests { let link_root = AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); - let link_private = link_root - .join("linked-private") - .expect("symlinked linked-private path"); + let link_private = link_root.join("linked-private"); let expected_root = AbsolutePathBuf::from_absolute_path( real_root.canonicalize().expect("canonicalize real root"), ) .expect("absolute canonical root"); - let expected_linked_private = expected_root - .join("linked-private") - .expect("expected linked-private path"); + let expected_linked_private = expected_root.join("linked-private"); let unexpected_decoy = AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) .expect("absolute canonical decoy"); @@ -1735,14 +1711,12 @@ mod tests { symlink_dir(&root, &alias).expect("create alias symlink"); let root = AbsolutePathBuf::from_absolute_path(&root).expect("absolute root"); - let alias = root.join("alias-root").expect("alias root path"); + let alias = root.join("alias-root"); let expected_root = AbsolutePathBuf::from_absolute_path( root.as_path().canonicalize().expect("canonicalize root"), ) .expect("absolute canonical root"); - let expected_alias = expected_root - .join("alias-root") - .expect("expected alias path"); + let expected_alias = expected_root.join("alias-root"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { @@ -1848,13 +1822,10 @@ mod tests { #[test] fn resolve_access_with_cwd_uses_most_specific_entry() { let cwd = TempDir::new().expect("tempdir"); - let docs = - AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); - let docs_private = AbsolutePathBuf::resolve_path_against_base("docs/private", cwd.path()) - .expect("resolve docs/private"); + let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); + let docs_private = AbsolutePathBuf::resolve_path_against_base("docs/private", cwd.path()); let docs_private_public = - AbsolutePathBuf::resolve_path_against_base("docs/private/public", cwd.path()) - .expect("resolve docs/private/public"); + AbsolutePathBuf::resolve_path_against_base("docs/private/public", cwd.path()); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -1901,8 +1872,7 @@ mod tests { #[test] fn split_only_nested_carveouts_need_direct_runtime_enforcement() { let cwd = TempDir::new().expect("tempdir"); - let docs = - AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); + let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -1933,8 +1903,7 @@ mod tests { #[test] fn root_write_with_read_only_child_is_not_full_disk_write() { let cwd = TempDir::new().expect("tempdir"); - let docs = - AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); + let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -1961,8 +1930,7 @@ mod tests { #[test] fn root_deny_does_not_materialize_as_unreadable_root() { let cwd = TempDir::new().expect("tempdir"); - let docs = - AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); + let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); let expected_docs = AbsolutePathBuf::from_absolute_path( cwd.path() .canonicalize() @@ -2025,8 +1993,7 @@ mod tests { #[test] fn same_specificity_write_override_keeps_full_disk_write_access() { let cwd = TempDir::new().expect("tempdir"); - let docs = - AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); + let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 4be5620fc..1f199b149 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1097,10 +1097,7 @@ fn default_read_only_subpaths_for_writable_root( protect_missing_dot_codex: bool, ) -> Vec { let mut subpaths: Vec = Vec::new(); - #[allow(clippy::expect_used)] - let top_level_git = writable_root - .join(".git") - .expect(".git is a valid relative path"); + let top_level_git = writable_root.join(".git"); // This applies to typical repos (directory .git), worktrees/submodules // (file .git with gitdir pointer), and bare repos when the gitdir is the // writable root itself. @@ -1116,8 +1113,7 @@ fn default_read_only_subpaths_for_writable_root( subpaths.push(top_level_git); } - #[allow(clippy::expect_used)] - let top_level_agents = writable_root.join(".agents").expect("valid relative path"); + let top_level_agents = writable_root.join(".agents"); if top_level_agents.as_path().is_dir() { subpaths.push(top_level_agents); } @@ -1126,8 +1122,7 @@ fn default_read_only_subpaths_for_writable_root( // default. For the workspace root itself, protect it even before the // directory exists so first-time creation still goes through the // protected-path approval flow. - #[allow(clippy::expect_used)] - let top_level_codex = writable_root.join(".codex").expect("valid relative path"); + let top_level_codex = writable_root.join(".codex"); if protect_missing_dot_codex || top_level_codex.as_path().is_dir() { subpaths.push(top_level_codex); } @@ -1187,16 +1182,7 @@ fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option path, - Err(err) => { - error!( - "Failed to resolve gitdir path {gitdir_raw} from {path}: {err}", - path = dot_git.as_path().display() - ); - return None; - } - }; + let gitdir_path = AbsolutePathBuf::resolve_path_against_base(gitdir_raw, base); if !gitdir_path.as_path().exists() { error!( "Resolved gitdir path {path} does not exist.", @@ -4034,8 +4020,7 @@ mod tests { .last() .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) .expect("filesystem root"); - let blocked = AbsolutePathBuf::resolve_path_against_base("blocked", cwd.path()) - .expect("resolve blocked"); + let blocked = AbsolutePathBuf::resolve_path_against_base("blocked", cwd.path()); let expected_blocked = AbsolutePathBuf::from_absolute_path( cwd.path() .canonicalize() @@ -4086,8 +4071,7 @@ mod tests { let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); let cwd_absolute = AbsolutePathBuf::from_absolute_path(&canonical_cwd).expect("absolute tempdir"); - let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path()) - .expect("resolve unreadable path"); + let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path()); let expected_secret = AbsolutePathBuf::from_absolute_path(canonical_cwd.join("secret")) .expect("canonical secret"); let expected_agents = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".agents")) @@ -4152,10 +4136,8 @@ mod tests { fn restricted_file_system_policy_treats_read_entries_as_read_only_subpaths() { let cwd = TempDir::new().expect("tempdir"); let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); - let docs = - AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); - let docs_public = AbsolutePathBuf::resolve_path_against_base("docs/public", cwd.path()) - .expect("resolve docs/public"); + let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); + let docs_public = AbsolutePathBuf::resolve_path_against_base("docs/public", cwd.path()); let expected_docs = AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs")) .expect("canonical docs"); let expected_docs_public = @@ -4199,8 +4181,7 @@ mod tests { #[test] fn legacy_workspace_write_nested_readable_root_stays_writable() { let cwd = TempDir::new().expect("tempdir"); - let docs = - AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); + let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex")) .expect("canonical .codex"); @@ -4257,12 +4238,9 @@ mod tests { #[test] fn legacy_sandbox_policy_semantics_survive_split_bridge() { let cwd = TempDir::new().expect("tempdir"); - let readable_root = AbsolutePathBuf::resolve_path_against_base("readable", cwd.path()) - .expect("resolve readable root"); - let writable_root = AbsolutePathBuf::resolve_path_against_base("writable", cwd.path()) - .expect("resolve writable root"); - let nested_readable_root = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()) - .expect("resolve nested readable root"); + let readable_root = AbsolutePathBuf::resolve_path_against_base("readable", cwd.path()); + let writable_root = AbsolutePathBuf::resolve_path_against_base("writable", cwd.path()); + let nested_readable_root = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); let policies = [ SandboxPolicy::DangerFullAccess, SandboxPolicy::ExternalSandbox { diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index 6fc34c963..9c2821eda 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -173,8 +173,8 @@ fn transform_additional_permissions_preserves_denied_entries() { canonicalize(temp_dir.path()).expect("canonicalize temp dir"), ) .expect("absolute temp dir"); - let allowed_path = workspace_root.join("allowed").expect("allowed path"); - let denied_path = workspace_root.join("denied").expect("denied path"); + let allowed_path = workspace_root.join("allowed"); + let denied_path = workspace_root.join("denied"); let exec_request = manager .transform(SandboxTransformRequest { command: SandboxCommand { diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index d287bd243..10f49d95f 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -52,8 +52,7 @@ fn root_write_policy_with_carveouts_still_uses_platform_sandbox() { let blocked = AbsolutePathBuf::resolve_path_against_base( "blocked", std::env::current_dir().expect("current dir"), - ) - .expect("blocked path"); + ); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -317,8 +316,8 @@ fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roo canonicalize(temp_dir.path()).expect("canonicalize temp dir"), ) .expect("absolute temp dir"); - let allowed_path = cwd.join("allowed").expect("allowed path"); - let denied_path = cwd.join("denied").expect("denied path"); + let allowed_path = cwd.join("allowed"); + let denied_path = cwd.join("denied"); let merged_policy = merge_file_system_policy_with_additional_permissions( &FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { @@ -361,7 +360,7 @@ fn effective_file_system_sandbox_policy_returns_base_policy_without_additional_p canonicalize(temp_dir.path()).expect("canonicalize temp dir"), ) .expect("absolute temp dir"); - let denied_path = cwd.join("denied").expect("denied path"); + let denied_path = cwd.join("denied"); let base_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -388,8 +387,8 @@ fn effective_file_system_sandbox_policy_merges_additional_write_roots() { canonicalize(temp_dir.path()).expect("canonicalize temp dir"), ) .expect("absolute temp dir"); - let allowed_path = cwd.join("allowed").expect("allowed path"); - let denied_path = cwd.join("denied").expect("denied path"); + let allowed_path = cwd.join("allowed"); + let denied_path = cwd.join("denied"); let base_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { diff --git a/codex-rs/shell-command/src/powershell.rs b/codex-rs/shell-command/src/powershell.rs index 4d3b55333..d6ae79245 100644 --- a/codex-rs/shell-command/src/powershell.rs +++ b/codex-rs/shell-command/src/powershell.rs @@ -112,10 +112,8 @@ pub fn try_find_pwsh_executable_blocking() -> Option { { let candidate = AbsolutePathBuf::resolve_path_against_base("pwsh.exe", &ps_home); - if let Ok(candidate_abs_path) = candidate - && is_powershellish_executable_available(candidate_abs_path.as_path()) - { - return Some(candidate_abs_path); + if is_powershellish_executable_available(candidate.as_path()) { + return Some(candidate); } } diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index 34b856216..2f31324ef 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -274,7 +274,7 @@ async fn handle_escalate_session_with_policy( _ = parent_cancellation_token.cancelled() => return Ok(()), _ = session_cancellation_token.cancelled() => return Ok(()), }; - let program = AbsolutePathBuf::resolve_path_against_base(file, workdir.as_path())?; + let program = AbsolutePathBuf::resolve_path_against_base(file, workdir.as_path()); let decision = tokio::select! { decision = policy.determine_action(&program, &argv, &workdir) => { decision.context("failed to determine escalation action")? @@ -718,7 +718,7 @@ mod tests { let workdir = tmp.path().join("workspace"); std::fs::create_dir(&workdir)?; let workdir = AbsolutePathBuf::try_from(workdir)?; - let expected_file = workdir.join("bin/tool")?; + let expected_file = workdir.join("bin/tool"); let server_task = tokio::spawn(handle_escalate_session_with_policy( server, Arc::new(AssertingEscalationPolicy { diff --git a/codex-rs/skills/src/lib.rs b/codex-rs/skills/src/lib.rs index 99cde06f6..3ea802f20 100644 --- a/codex-rs/skills/src/lib.rs +++ b/codex-rs/skills/src/lib.rs @@ -21,7 +21,7 @@ const SYSTEM_SKILLS_MARKER_SALT: &str = "v1"; /// This is typically located at `CODEX_HOME/skills/.system`. pub fn system_cache_root_dir(codex_home: &Path) -> PathBuf { AbsolutePathBuf::try_from(codex_home) - .and_then(|codex_home| system_cache_root_dir_abs(&codex_home)) + .map(|codex_home| system_cache_root_dir_abs(&codex_home)) .map(AbsolutePathBuf::into_path_buf) .unwrap_or_else(|_| { codex_home @@ -30,9 +30,9 @@ pub fn system_cache_root_dir(codex_home: &Path) -> PathBuf { }) } -fn system_cache_root_dir_abs(codex_home: &AbsolutePathBuf) -> std::io::Result { +fn system_cache_root_dir_abs(codex_home: &AbsolutePathBuf) -> AbsolutePathBuf { codex_home - .join(SKILLS_DIR_NAME)? + .join(SKILLS_DIR_NAME) .join(SYSTEM_SKILLS_DIR_NAME) } @@ -47,18 +47,13 @@ fn system_cache_root_dir_abs(codex_home: &AbsolutePathBuf) -> std::io::Result Result<(), SystemSkillsError> { let codex_home = AbsolutePathBuf::try_from(codex_home) .map_err(|source| SystemSkillsError::io("normalize codex home dir", source))?; - let skills_root_dir = codex_home - .join(SKILLS_DIR_NAME) - .map_err(|source| SystemSkillsError::io("resolve skills root dir", source))?; + let skills_root_dir = codex_home.join(SKILLS_DIR_NAME); fs::create_dir_all(skills_root_dir.as_path()) .map_err(|source| SystemSkillsError::io("create skills root dir", source))?; - let dest_system = system_cache_root_dir_abs(&codex_home) - .map_err(|source| SystemSkillsError::io("resolve system skills cache root dir", source))?; + let dest_system = system_cache_root_dir_abs(&codex_home); - let marker_path = dest_system - .join(SYSTEM_SKILLS_MARKER_FILENAME) - .map_err(|source| SystemSkillsError::io("resolve system skills marker path", source))?; + let marker_path = dest_system.join(SYSTEM_SKILLS_MARKER_FILENAME); let expected_fingerprint = embedded_system_skills_fingerprint(); if dest_system.as_path().is_dir() && read_marker(&marker_path).is_ok_and(|marker| marker == expected_fingerprint) @@ -127,18 +122,14 @@ fn write_embedded_dir(dir: &Dir<'_>, dest: &AbsolutePathBuf) -> Result<(), Syste for entry in dir.entries() { match entry { include_dir::DirEntry::Dir(subdir) => { - let subdir_dest = dest.join(subdir.path()).map_err(|source| { - SystemSkillsError::io("resolve system skills subdir", source) - })?; + let subdir_dest = dest.join(subdir.path()); fs::create_dir_all(subdir_dest.as_path()).map_err(|source| { SystemSkillsError::io("create system skills subdir", source) })?; write_embedded_dir(subdir, dest)?; } include_dir::DirEntry::File(file) => { - let path = dest.join(file.path()).map_err(|source| { - SystemSkillsError::io("resolve system skills file", source) - })?; + let path = dest.join(file.path()); if let Some(parent) = path.as_path().parent() { fs::create_dir_all(parent).map_err(|source| { SystemSkillsError::io("create system skills file parent", source) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 668a56876..dad10fa76 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1024,7 +1024,7 @@ fn normalize_harness_overrides_for_cwd( let mut normalized = Vec::with_capacity(overrides.additional_writable_roots.len()); for root in overrides.additional_writable_roots.drain(..) { - let absolute = AbsolutePathBuf::resolve_path_against_base(root, base_cwd)?; + let absolute = AbsolutePathBuf::resolve_path_against_base(root, base_cwd); normalized.push(absolute.into_path_buf()); } overrides.additional_writable_roots = normalized; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e8713704d..86e11b49c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -5028,15 +5028,7 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::ForkCurrentSession); } SlashCommand::Init => { - let init_target = match self.config.cwd.join(DEFAULT_PROJECT_DOC_FILENAME) { - Ok(path) => path, - Err(err) => { - self.add_error_message(format!( - "Failed to prepare {DEFAULT_PROJECT_DOC_FILENAME}: {err}", - )); - return; - } - }; + let init_target = self.config.cwd.join(DEFAULT_PROJECT_DOC_FILENAME); if init_target.exists() { let message = format!( "{DEFAULT_PROJECT_DOC_FILENAME} already exists here. Skipping /init to avoid overwriting it." diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index 68a565924..d271fc007 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -866,11 +866,7 @@ async fn unified_exec_non_empty_then_empty_snapshots() { #[tokio::test] async fn view_image_tool_call_adds_history_cell() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - let image_path = chat - .config - .cwd - .join("example.png") - .expect("absolute image path"); + let image_path = chat.config.cwd.join("example.png"); chat.handle_codex_event(Event { id: "sub-image".into(), diff --git a/codex-rs/utils/absolute-path/Cargo.toml b/codex-rs/utils/absolute-path/Cargo.toml index f4fa8bf78..801bc9c1f 100644 --- a/codex-rs/utils/absolute-path/Cargo.toml +++ b/codex-rs/utils/absolute-path/Cargo.toml @@ -10,7 +10,6 @@ workspace = true [dependencies] dirs = { workspace = true } -path-absolutize = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } ts-rs = { workspace = true, features = [ diff --git a/codex-rs/utils/absolute-path/src/absolutize.rs b/codex-rs/utils/absolute-path/src/absolutize.rs new file mode 100644 index 000000000..7965c81f7 --- /dev/null +++ b/codex-rs/utils/absolute-path/src/absolutize.rs @@ -0,0 +1,167 @@ +// Adapted from path-absolutize 3.1.1: +// Copyright (c) 2018 magiclen.org (Ron Li) +// Licensed under the MIT License. +// +// Keep this implementation local so explicit-base normalization can be +// infallible for `AbsolutePathBuf::resolve_path_against_base` and +// `AbsolutePathBuf::join`; only current-working-directory lookup remains +// fallible. + +use std::path::Component; +use std::path::Path; +use std::path::PathBuf; + +pub(super) fn absolutize(path: &Path) -> std::io::Result { + Ok(absolutize_from(path, &std::env::current_dir()?)) +} + +pub(super) fn absolutize_from(path: &Path, base_path: &Path) -> PathBuf { + normalize_path(&path_with_base(path, base_path)) +} + +fn normalize_path(path: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + normalized.pop(); + } + Component::Prefix(_) | Component::RootDir | Component::Normal(_) => { + normalized.push(component.as_os_str()); + } + } + } + + if normalized.as_os_str().is_empty() { + PathBuf::from(".") + } else { + normalized + } +} + +#[cfg(not(windows))] +fn path_with_base(path: &Path, base_path: &Path) -> PathBuf { + if path.is_absolute() { + path.to_path_buf() + } else { + base_path.join(path) + } +} + +#[cfg(windows)] +fn path_with_base(path: &Path, base_path: &Path) -> PathBuf { + if path.is_absolute() || path.has_root() { + return base_path.join(path); + } + + let mut components = path.components(); + let Some(Component::Prefix(prefix)) = components.next() else { + return base_path.join(path); + }; + + let mut path = PathBuf::new(); + path.push(prefix.as_os_str()); + + if components.clone().next().is_none() { + path.push(std::path::MAIN_SEPARATOR_STR); + return path; + } + + let skip_base_prefix = matches!(base_path.components().next(), Some(Component::Prefix(_))); + for component in base_path + .components() + .skip(usize::from(skip_base_prefix)) + .chain(components) + { + path.push(component.as_os_str()); + } + path +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[cfg(unix)] + #[test] + fn absolute_path_without_dots_is_unchanged() { + assert_eq!( + absolutize_from(Path::new("/path/to/123/456"), Path::new("/base")), + PathBuf::from("/path/to/123/456") + ); + } + + #[cfg(unix)] + #[test] + fn absolute_path_dots_are_removed() { + assert_eq!( + absolutize_from(Path::new("/path/to/./123/../456"), Path::new("/base")), + PathBuf::from("/path/to/456") + ); + } + + #[cfg(unix)] + #[test] + fn relative_path_without_dot_uses_base() { + assert_eq!( + absolutize_from(Path::new("path/to/123/456"), Path::new("/base")), + PathBuf::from("/base/path/to/123/456") + ); + } + + #[cfg(unix)] + #[test] + fn relative_path_with_current_dir_uses_base() { + assert_eq!( + absolutize_from(Path::new("./path/to/123/456"), Path::new("/base")), + PathBuf::from("/base/path/to/123/456") + ); + } + + #[cfg(unix)] + #[test] + fn relative_path_with_parent_dir_uses_base_parent() { + assert_eq!( + absolutize_from(Path::new("../path/to/123/456"), Path::new("/base/cwd")), + PathBuf::from("/base/path/to/123/456") + ); + } + + #[cfg(unix)] + #[test] + fn parent_dir_above_root_stays_at_root() { + assert_eq!( + absolutize_from(Path::new("../../path/to/123/456"), Path::new("/")), + PathBuf::from("/path/to/123/456") + ); + } + + #[cfg(unix)] + #[test] + fn empty_path_uses_base() { + assert_eq!( + absolutize_from(Path::new(""), Path::new("/base/cwd")), + PathBuf::from("/base/cwd") + ); + } + + #[cfg(windows)] + #[test] + fn windows_root_relative_path_uses_base_prefix() { + assert_eq!( + absolutize_from(Path::new(r"\path\to\file"), Path::new(r"C:\base\cwd")), + PathBuf::from(r"C:\path\to\file") + ); + } + + #[cfg(windows)] + #[test] + fn windows_drive_relative_path_uses_path_prefix_and_base_tail() { + assert_eq!( + absolutize_from(Path::new(r"D:path\to\file"), Path::new(r"C:\base\cwd")), + PathBuf::from(r"D:\base\cwd\path\to\file") + ); + } +} diff --git a/codex-rs/utils/absolute-path/src/lib.rs b/codex-rs/utils/absolute-path/src/lib.rs index 0de0f4e02..2b7ca4c30 100644 --- a/codex-rs/utils/absolute-path/src/lib.rs +++ b/codex-rs/utils/absolute-path/src/lib.rs @@ -1,5 +1,4 @@ use dirs::home_dir; -use path_absolutize::Absolutize; use schemars::JsonSchema; use serde::Deserialize; use serde::Deserializer; @@ -11,6 +10,8 @@ use std::path::Path; use std::path::PathBuf; use ts_rs::TS; +mod absolutize; + /// A path that is guaranteed to be absolute and normalized (though it is not /// guaranteed to be canonicalized or exist on the filesystem). /// @@ -43,30 +44,34 @@ impl AbsolutePathBuf { pub fn resolve_path_against_base, B: AsRef>( path: P, base_path: B, - ) -> std::io::Result { + ) -> Self { let expanded = Self::maybe_expand_home_directory(path.as_ref()); - let absolute_path = expanded.absolutize_from(base_path.as_ref())?; - Ok(Self(absolute_path.into_owned())) + Self(absolutize::absolutize_from(&expanded, base_path.as_ref())) } pub fn from_absolute_path>(path: P) -> std::io::Result { let expanded = Self::maybe_expand_home_directory(path.as_ref()); - let absolute_path = expanded.absolutize()?; - Ok(Self(absolute_path.into_owned())) + Ok(Self(absolutize::absolutize(&expanded)?)) } pub fn current_dir() -> std::io::Result { let current_dir = std::env::current_dir()?; - Self::from_absolute_path(current_dir) + Ok(Self(absolutize::absolutize_from( + ¤t_dir, + ¤t_dir, + ))) } /// Construct an absolute path from `path`, resolving relative paths against /// the process current working directory. pub fn relative_to_current_dir>(path: P) -> std::io::Result { - Self::resolve_path_against_base(path, std::env::current_dir()?) + Ok(Self::resolve_path_against_base( + path, + std::env::current_dir()?, + )) } - pub fn join>(&self, path: P) -> std::io::Result { + pub fn join>(&self, path: P) -> Self { Self::resolve_path_against_base(path, &self.0) } @@ -187,9 +192,7 @@ impl<'de> Deserialize<'de> for AbsolutePathBuf { { let path = PathBuf::deserialize(deserializer)?; ABSOLUTE_PATH_BASE.with(|cell| match cell.borrow().as_deref() { - Some(base) => { - Ok(Self::resolve_path_against_base(path, base).map_err(SerdeError::custom)?) - } + Some(base) => Ok(Self::resolve_path_against_base(path, base)), None if path.is_absolute() => { Self::from_absolute_path(path).map_err(SerdeError::custom) } @@ -213,8 +216,7 @@ mod tests { let base_path = base_dir.path(); let absolute_path = absolute_dir.path().join("file.txt"); let abs_path_buf = - AbsolutePathBuf::resolve_path_against_base(absolute_path.clone(), base_path) - .expect("failed to create"); + AbsolutePathBuf::resolve_path_against_base(absolute_path.clone(), base_path); assert_eq!(abs_path_buf.as_path(), absolute_path.as_path()); } @@ -222,8 +224,16 @@ mod tests { fn relative_path_is_resolved_against_base_path() { let temp_dir = tempdir().expect("base dir"); let base_dir = temp_dir.path(); - let abs_path_buf = AbsolutePathBuf::resolve_path_against_base("file.txt", base_dir) - .expect("failed to create"); + let abs_path_buf = AbsolutePathBuf::resolve_path_against_base("file.txt", base_dir); + assert_eq!(abs_path_buf.as_path(), base_dir.join("file.txt").as_path()); + } + + #[test] + fn relative_path_dots_are_normalized_against_base_path() { + let temp_dir = tempdir().expect("base dir"); + let base_dir = temp_dir.path(); + let abs_path_buf = + AbsolutePathBuf::resolve_path_against_base("./nested/../file.txt", base_dir); assert_eq!(abs_path_buf.as_path(), base_dir.join("file.txt").as_path()); } diff --git a/codex-rs/utils/path-utils/src/lib.rs b/codex-rs/utils/path-utils/src/lib.rs index 4e053c039..7e44e06d1 100644 --- a/codex-rs/utils/path-utils/src/lib.rs +++ b/codex-rs/utils/path-utils/src/lib.rs @@ -84,7 +84,7 @@ pub fn resolve_symlink_write_paths(path: &Path) -> io::Result let next = if target.is_absolute() { AbsolutePathBuf::from_absolute_path(&target) } else if let Some(parent) = current.parent() { - AbsolutePathBuf::resolve_path_against_base(&target, parent) + Ok(AbsolutePathBuf::resolve_path_against_base(&target, parent)) } else { return Ok(SymlinkWritePaths { read_path: None,