diff --git a/.github/fork-ci-known-failures.txt b/.github/fork-ci-known-failures.txt index 12e9ba2b0ee6..35c9bbaf3c55 100644 --- a/.github/fork-ci-known-failures.txt +++ b/.github/fork-ci-known-failures.txt @@ -1,7 +1,24 @@ # Tests that fail on unmodified fork/main. One nextest filterset per line, # with a `#` reason that cites the baseline run showing it fail. A line is # only added after that evidence exists; the file starts empty. -test(pending_input_persistence::background_user_input) # timing flake: fails on PR #6/#12 in files the PR does not touch; green on the fork/main baseline -test(pending_input_persistence::synchronous_store) # timing flake: fails on PR #6/#12 in files the PR does not touch; green on the fork/main baseline -test(remote_env::future_pending_environment) # timing flake: fails on PR #6/#12 in files the PR does not touch; green on the fork/main baseline -test(hooks::permission_request_hook_allows_exec_command_without_user_approval) # timing flake: fails on PR #6/#12 in files the PR does not touch; green on the fork/main baseline +# Fail on unmodified fork/main on gha-spot-1 (fork-ci run 38089490933, workflow_dispatch +# with these packages forced). Environmental, not caused by a fork branch. +test(auto_daemon_start_attaches_to_shared_server) # fails on unmodified fork/main, fork-ci run 38089490933 +test(incompatible_daemon_can_cancel) # fails on unmodified fork/main, fork-ci run 38089490933 +test(incompatible_daemon_can_restart_with_confirmed_settings) # fails on unmodified fork/main, fork-ci run 38089490933 +test(managed_daemon_restarts_from_deleted_working_directory) # fails on unmodified fork/main, fork-ci run 38089490933 +test(managed_identity_survives_locale_and_timezone_changes) # fails on unmodified fork/main, fork-ci run 38089490933 +test(managed_start_keeps_updater_on_marker_mismatch_but_stops_it_for_pin) # fails on unmodified fork/main, fork-ci run 38089490933 +test(managed_start_succeeds_when_updater_record_is_invalid) # fails on unmodified fork/main, fork-ci run 38089490933 +test(managed_starts_ensure_one_updater_and_recover_a_missing_one) # fails on unmodified fork/main, fork-ci run 38089490933 +test(packaged_daemon_bootstrap_seeds_local_package) # fails on unmodified fork/main, fork-ci run 38089490933 +test(packaged_daemon_explicit_replacement_migrates_running_legacy) # fails on unmodified fork/main, fork-ci run 38089490933 +test(packaged_daemon_restart_seeds_local_package) # fails on unmodified fork/main, fork-ci run 38089490933 +test(packaged_daemon_start_and_explicit_replacement) # fails on unmodified fork/main, fork-ci run 38089490933 +test(restart_applies_saved_updater_preference) # fails on unmodified fork/main, fork-ci run 38089490933 +test(wall_clock_shift_keeps_server_and_updater_managed) # fails on unmodified fork/main, fork-ci run 38089490933 +test(shell_snapshot::shell_snapshot_concurrent_replays_keep_independent_readers::blocked_descriptor_path) # fails on unmodified fork/main, fork-ci run 38089490933 +test(shell_snapshot_v2_capture_failure_falls_back_and_retries::remote_pipe_recovery) # fails on unmodified fork/main, fork-ci run 38089490933 +test(shell_snapshot_v2_capture_failure_falls_back_and_retries::remote_tty_recovery) # fails on unmodified fork/main, fork-ci run 38089490933 +test(shell_snapshot_v2_filters_profile_exports_and_stays_in_memory::remote_sandbox) # fails on unmodified fork/main, fork-ci run 38089490933 + diff --git a/.github/workflows/fork-ci.yml b/.github/workflows/fork-ci.yml index 1a532e1da686..1b8e0a0e1601 100644 --- a/.github/workflows/fork-ci.yml +++ b/.github/workflows/fork-ci.yml @@ -158,7 +158,12 @@ jobs: # every later cargo invocation would compile against it. trap 'mv /tmp/Cargo.lock.head Cargo.lock' EXIT git show "$base:codex-rs/Cargo.lock" > Cargo.lock - cargo metadata --format-version 1 --locked > "$base_meta" + # Not --locked. The base lock describes the base manifests, and + # reading it against HEAD's manifests (a new edge on an existing + # package) makes cargo 1.95 call the lock stale even though the + # resolve is identical. --offline keeps it from moving a version; + # the copy above is discarded either way. + cargo metadata --format-version 1 --offline > "$base_meta" mv /tmp/Cargo.lock.head Cargo.lock trap - EXIT mapfile -t locked < <(python3 "${GITHUB_WORKSPACE}/.github/scripts/fork-lock-affected.py" "$base_meta" "$head_meta") @@ -210,16 +215,22 @@ jobs: for crate in "${crates[@]}"; do args+=(-p "$crate"); done cargo fmt "${args[@]}" -- --check - # -D warnings would fail the PR on upstream lint debt in files it never + # -D warnings would fail the PR on upstream lint debt in lines it never # touched, and a fork patch must not have to fix that. Clippy runs without - # it, and only a warning or error whose primary span is in a file this PR - # changed fails the build. Compile errors are fatal regardless, which + # it, and only a warning or error whose primary span overlaps a hunk this + # PR changed fails the build. Compile errors are fatal regardless, which # `cargo check` above already enforces. - name: cargo clippy shell: bash run: | set -euo pipefail - printf '%s\n' "${{ steps.crates.outputs.files }}" | sed 's|^codex-rs/||' > /tmp/changed-files + base="$(git merge-base HEAD origin/fork/main 2>/dev/null || git merge-base HEAD fork/main)" + # One changed line range per hunk, as "file start count" with the + # codex-rs/ prefix stripped. A count of 0 is a pure deletion. + git diff -U0 "$base"...HEAD -- codex-rs \ + | awk '/^diff --git/{file=$4; sub(/^b\/codex-rs\//, "", file)} + /^@@/{split($3,a,","); sub(/^\+/, "", a[1]); print file, a[1], (a[2] == "" ? 1 : a[2])}' \ + > /tmp/changed-lines mapfile -t crates < <(printf '%s\n' "${{ steps.crates.outputs.list }}" | sed '/^$/d') (( ${#crates[@]} )) || exit 0 args=() @@ -233,18 +244,22 @@ jobs: cargo clippy "${args[@]}" --all-targets --timings --message-format=json -- -W clippy::all \ > /tmp/clippy.json || true [[ -f cargo-timing.html ]] && cp cargo-timing.html /tmp/cargo-timings/clippy-timing.html - # The gate is by changed file path, so it reads the whole stream once. + # The gate is by changed hunk, so it reads the whole stream once. # Splitting per crate first drops every lint from a crate whose # package id omits the name (cargo does that when the name equals the # directory's last segment: codex-home, codex-api, ...). rm -f /tmp/jq-err - offenders="$(jq -r --slurpfile changed <(jq -R . /tmp/changed-files) ' + offenders="$(jq -r --slurpfile lines <(jq -R 'split(" ") | {file:.[0], start:.[1]|tonumber, count:.[2]|tonumber}' /tmp/changed-lines) ' select(.reason == "compiler-message") | .message | select(.level == "warning" or .level == "error") | . as $m | ($m.spans[] | select(.is_primary)) as $s - | select(any($changed[]; . == $s.file_name)) + | select(any($lines[]; + .file == $s.file_name + and .count > 0 + and $s.line_start <= (.start + .count - 1) + and $s.line_end >= .start)) | "\($s.file_name):\($s.line_start):\($s.column_start): \($m.level): \($m.message)" ' /tmp/clippy.json 2>/tmp/jq-err)" if [[ -s /tmp/jq-err ]]; then @@ -254,7 +269,7 @@ jobs: fi if [[ -n "$offenders" ]]; then echo "$offenders" - echo "clippy findings in files this PR changes" >&2 + echo "clippy findings in lines this PR changes" >&2 exit 1 fi diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 1cee1174c2e9..b6363c00db16 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3671,7 +3671,9 @@ version = "0.0.0" dependencies = [ "codex-extension-api", "codex-utils-absolute-path", + "dirs", "pretty_assertions", + "sha2 0.10.9", "tempfile", "tokio", "tracing", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index c105c371955a..8a4919d25773 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -10375,6 +10375,11 @@ "null" ] }, + "failClosed": { + "default": false, + "description": "When true, a failure of this hook blocks the tool instead of failing open.", + "type": "boolean" + }, "statusMessage": { "type": [ "string", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 1a38e4fe91b7..5e4aa94961ba 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -6211,6 +6211,11 @@ "null" ] }, + "failClosed": { + "default": false, + "description": "When true, a failure of this hook blocks the tool instead of failing open.", + "type": "boolean" + }, "statusMessage": { "type": [ "string", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json index f2faa534962d..f8f730eb3b19 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json @@ -652,6 +652,11 @@ "null" ] }, + "failClosed": { + "default": false, + "description": "When true, a failure of this hook blocks the tool instead of failing open.", + "type": "boolean" + }, "statusMessage": { "type": [ "string", diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index eef430182091..4ff8f33dd72c 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index eb2ee812b34f..e72125317d44 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookHandler.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookHandler.ts index 6ee1b16ba36e..77b8a25110fa 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookHandler.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfiguredHookHandler.ts @@ -10,4 +10,8 @@ export type ConfiguredHookHandler = { "type": "command", command: string, comman * evaluated against the original context; a spilled preview also includes recovery * metadata. */ -additionalContextLimit: number | null, } | { "type": "mcp_tool", server: string, tool: string, input: { [key in string]?: JsonValue }, timeoutSec: bigint | null, statusMessage: string | null, } | { "type": "prompt", } | { "type": "agent", }; +additionalContextLimit: number | null, +/** + * When true, a failure of this hook blocks the tool instead of failing open. + */ +failClosed: boolean, } | { "type": "mcp_tool", server: string, tool: string, input: { [key in string]?: JsonValue }, timeoutSec: bigint | null, statusMessage: string | null, } | { "type": "prompt", } | { "type": "agent", }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index 29539116025c..ee5c0ec5c585 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -685,6 +685,10 @@ pub enum ConfiguredHookHandler { #[serde(rename = "additionalContextLimit")] #[ts(rename = "additionalContextLimit")] additional_context_limit: Option, + /// When true, a failure of this hook blocks the tool instead of failing open. + #[serde(default, rename = "failClosed")] + #[ts(rename = "failClosed")] + fail_closed: bool, }, #[serde(rename = "mcp_tool")] #[ts(rename = "mcp_tool")] diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index ac965523fb22..4e30262c9c85 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -182,7 +182,7 @@ mod tests { use codex_core::thread_store_from_config; use codex_exec_server::EnvironmentManager; use codex_extension_api::NoopExtensionEventSink; - use codex_home::CodexHomeUserInstructionsProvider; + use codex_home::user_instructions_provider; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_protocol::protocol::SessionSource; @@ -490,9 +490,11 @@ enabled = false queue_service: None, turn_start_admission: None, }), - Arc::new(CodexHomeUserInstructionsProvider::new( + user_instructions_provider( good_config.codex_home.clone(), - )), + good_config.claude_compat_instructions, + good_config.project_doc_max_bytes, + ), /*analytics_events_client*/ None, codex_core::passthrough_image_store(), Arc::clone(&thread_store), diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 6b705c55565e..96c0e38b35e4 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -82,7 +82,7 @@ use codex_exec_server::EnvironmentManager; use codex_extension_api::TurnStartAdmission; use codex_feedback::CodexFeedback; use codex_goal_extension::GoalService; -use codex_home::CodexHomeUserInstructionsProvider; +use codex_home::user_instructions_provider; use codex_login::AuthManager; use codex_otel::auth_storage::AuthStorageOriginator; use codex_protocol::ThreadId; @@ -354,9 +354,11 @@ impl MessageProcessor { queue_service: queue_service.clone(), turn_start_admission: Some(Arc::clone(&turn_start_admission)), }), - Arc::new(CodexHomeUserInstructionsProvider::new( + user_instructions_provider( config.codex_home.clone(), - )), + config.claude_compat_instructions, + config.project_doc_max_bytes, + ), Some(analytics_events_client.clone()), codex_core::passthrough_image_store(), Arc::clone(&thread_store), @@ -366,11 +368,10 @@ impl MessageProcessor { outgoing.clone(), thread_state_manager.clone(), )), - Some({ - let time_provider = - app_server_time_provider(outgoing.clone(), thread_state_manager.clone()); - time_provider - }), + Some(app_server_time_provider( + outgoing.clone(), + thread_state_manager.clone(), + )), ); match code_mode_session_provider { Some(provider) => manager.with_code_mode_session_provider(provider), diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index 914abf4fd8b8..63599b6ab395 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -766,6 +766,7 @@ fn map_hook_handler_to_api(handler: CoreHookHandlerConfig) -> ConfiguredHookHand r#async, status_message, additional_context_limit, + fail_closed, } => ConfiguredHookHandler::Command { command, command_windows, @@ -773,6 +774,7 @@ fn map_hook_handler_to_api(handler: CoreHookHandlerConfig) -> ConfiguredHookHand r#async, status_message, additional_context_limit, + fail_closed, }, CoreHookHandlerConfig::McpTool { server, 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 c4ba5937d13e..e13422905a7c 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -256,6 +256,7 @@ statusMessage = "Scanning file" r#async: false, status_message: None, additional_context_limit: Some(4_096), + fail_closed: false, }, ConfiguredHookHandler::McpTool { server: "security".to_string(), diff --git a/codex-rs/app-server/tests/suite/v2/hooks_list.rs b/codex-rs/app-server/tests/suite/v2/hooks_list.rs index beebe306a148..fb7c05bc06df 100644 --- a/codex-rs/app-server/tests/suite/v2/hooks_list.rs +++ b/codex-rs/app-server/tests/suite/v2/hooks_list.rs @@ -67,6 +67,7 @@ fn command_hook_hash( r#async, status_message: status_message.map(ToOwned::to_owned), additional_context_limit, + fail_closed: false, }], }, }; diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 8d062a23979e..1336c9944c7e 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -101,7 +101,7 @@ use codex_core::config::resolve_profile_v2_config_path; use codex_features::FEATURES; use codex_features::Stage; use codex_features::is_known_feature_key; -use codex_home::CodexHomeUserInstructionsProvider; +use codex_home::user_instructions_provider; use codex_login::AuthManager; use codex_login::is_workload_identity_selected; use codex_memories_write::clear_memory_roots_contents; @@ -2054,9 +2054,11 @@ async fn run_debug_prompt_input_command( }); } - let user_instructions_provider = Arc::new(CodexHomeUserInstructionsProvider::new( + let user_instructions_provider = user_instructions_provider( config.codex_home.clone(), - )); + config.claude_compat_instructions, + config.project_doc_max_bytes, + ); let auth_manager = AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await?; let mut extensions = codex_extension_api::ExtensionRegistryBuilder::new(); diff --git a/codex-rs/codex-home/Cargo.toml b/codex-rs/codex-home/Cargo.toml index 068ec55b4e27..7fd26c4ad5b9 100644 --- a/codex-rs/codex-home/Cargo.toml +++ b/codex-rs/codex-home/Cargo.toml @@ -13,7 +13,9 @@ workspace = true [dependencies] codex-extension-api = { workspace = true } codex-utils-absolute-path = { workspace = true } -tokio = { workspace = true, features = ["fs"] } +dirs = { workspace = true } +sha2 = { workspace = true } +tokio = { workspace = true, features = ["fs", "macros", "rt"] } tracing = { workspace = true } [dev-dependencies] diff --git a/codex-rs/codex-home/src/instructions/claude_compat.rs b/codex-rs/codex-home/src/instructions/claude_compat.rs new file mode 100644 index 000000000000..a545b8e9b146 --- /dev/null +++ b/codex-rs/codex-home/src/instructions/claude_compat.rs @@ -0,0 +1,123 @@ +//! Global user instructions with Claude Code compatibility. +//! +//! Fork-owned (WI-1131). Wraps [`CodexHomeUserInstructionsProvider`] and, when +//! enabled, appends `~/.claude/CLAUDE.md` and then `~/.claude/rules/*.md` in +//! sorted order. Both go through the shared `@import` expander, so a line that +//! is only `@path` is replaced by that file, to depth 5, deduplicated by path, +//! symlink and content, and counted against `project_doc_max_bytes`. + +use std::path::PathBuf; +use std::sync::Arc; + +use codex_extension_api::Instructions; +use codex_extension_api::LoadInstructionsFuture; +use codex_extension_api::LoadedUserInstructions; +use codex_extension_api::UserInstructionsProvider; +use codex_utils_absolute_path::AbsolutePathBuf; + +use super::CodexHomeUserInstructionsProvider; +use super::imports::expand_imports_all; + +const CLAUDE_DIR: &str = ".claude"; +const CLAUDE_MD: &str = "CLAUDE.md"; +const RULES_DIR: &str = "rules"; + +/// Loads the Codex home instructions plus the user's Claude Code files. +#[derive(Clone, Debug)] +pub struct ClaudeCompatUserInstructionsProvider { + inner: CodexHomeUserInstructionsProvider, + /// The user's home directory, where `~/.claude` lives. + user_home: Option, + /// Bytes the appended Claude files may consume in total. + max_bytes: usize, +} + +impl ClaudeCompatUserInstructionsProvider { + /// Creates a provider. `user_home` is the directory holding `.claude`; pass + /// `None` when it cannot be determined and only the Codex home is loaded. + pub fn new(codex_home: AbsolutePathBuf, user_home: Option, max_bytes: usize) -> Self { + Self { + inner: CodexHomeUserInstructionsProvider::new(codex_home), + user_home, + max_bytes, + } + } + + async fn load(&self) -> LoadedUserInstructions { + let mut loaded = self.inner.load_user_instructions().await; + let Some(user_home) = &self.user_home else { + return loaded; + }; + let claude_dir = user_home.join(CLAUDE_DIR); + let mut paths = vec![claude_dir.join(CLAUDE_MD)]; + if let Some(rules) = rule_files(&claude_dir.join(RULES_DIR)).await { + paths.extend(rules); + } + let expanded = expand_imports_all(&paths, Some(user_home), self.max_bytes, None).await; + loaded.warnings.extend(expanded.warnings); + if expanded.files.is_empty() { + return loaded; + } + let claude_text = expanded + .files + .iter() + .map(|file| format!("Contents of {}:\n\n{}", file.path.display(), file.text)) + .collect::>() + .join("\n\n"); + let text = match loaded.instructions.take() { + Some(existing) => format!("{}\n\n{claude_text}", existing.text), + None => claude_text, + }; + loaded.instructions = Some(Instructions { + text, + // The snapshot now spans several files, so no single source path + // describes it. + source: None, + }); + loaded + } +} + +impl UserInstructionsProvider for ClaudeCompatUserInstructionsProvider { + fn load_user_instructions(&self) -> LoadInstructionsFuture<'_> { + Box::pin(async move { self.load().await }) + } +} + +/// `*.md` files directly inside `dir`, sorted by name. `None` when the +/// directory does not exist. +async fn rule_files(dir: &std::path::Path) -> Option> { + let mut entries = match tokio::fs::read_dir(dir).await { + Ok(entries) => entries, + Err(_) => return None, + }; + let mut files = Vec::new(); + while let Ok(Some(entry)) = entries.next_entry().await { + let path = entry.path(); + if path.extension().and_then(|ext| ext.to_str()) == Some("md") { + files.push(path); + } + } + files.sort(); + Some(files) +} + +/// Selects the global user-instructions provider for a runtime. +/// +/// `claude_compat` is the fork's config switch: off keeps upstream behaviour +/// exactly, on appends the user's `~/.claude` files. +pub fn user_instructions_provider( + codex_home: AbsolutePathBuf, + claude_compat: bool, + max_bytes: usize, +) -> Arc { + if claude_compat { + Arc::new(ClaudeCompatUserInstructionsProvider::new( + codex_home, + dirs::home_dir(), + max_bytes, + )) + } else { + Arc::new(CodexHomeUserInstructionsProvider::new(codex_home)) + } +} diff --git a/codex-rs/codex-home/src/instructions/claude_compat_tests.rs b/codex-rs/codex-home/src/instructions/claude_compat_tests.rs new file mode 100644 index 000000000000..9cb2b4ccb6f5 --- /dev/null +++ b/codex-rs/codex-home/src/instructions/claude_compat_tests.rs @@ -0,0 +1,80 @@ +use std::fs; + +use codex_extension_api::UserInstructionsProvider; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; + +use super::ClaudeCompatUserInstructionsProvider; + +fn provider(user_home: &std::path::Path, max_bytes: usize) -> ClaudeCompatUserInstructionsProvider { + let codex_home = tempfile::tempdir().expect("codex home"); + // Leak: the provider only stores the path, and the dir lives until process exit. + let codex_home = Box::leak(Box::new(codex_home)); + ClaudeCompatUserInstructionsProvider::new( + AbsolutePathBuf::try_from(codex_home.path().to_path_buf()).expect("absolute"), + Some(user_home.to_path_buf()), + max_bytes, + ) +} + +#[tokio::test] +async fn claude_md_then_rules_in_sorted_order() { + let home = tempfile::tempdir().expect("home"); + let claude = home.path().join(".claude"); + fs::create_dir_all(claude.join("rules")).expect("rules dir"); + fs::write(claude.join("CLAUDE.md"), "global claude\n").expect("claude md"); + fs::write(claude.join("rules").join("b.md"), "rule b\n").expect("rule b"); + fs::write(claude.join("rules").join("a.md"), "rule a\n").expect("rule a"); + fs::write(claude.join("rules").join("notes.txt"), "not a rule").expect("txt"); + + let loaded = provider(home.path(), 32 * 1024) + .load_user_instructions() + .await; + + let text = loaded.instructions.expect("instructions").text; + let claude_at = text.find("global claude").expect("claude text"); + let a_at = text.find("rule a").expect("rule a"); + let b_at = text.find("rule b").expect("rule b"); + assert!(claude_at < a_at && a_at < b_at, "order was {text:?}"); + assert!(!text.contains("not a rule")); + assert!(loaded.warnings.is_empty()); +} + +#[tokio::test] +async fn rules_import_is_expanded_and_deduplicated() { + let home = tempfile::tempdir().expect("home"); + let claude = home.path().join(".claude"); + fs::create_dir_all(claude.join("rules")).expect("rules dir"); + fs::write(claude.join("shared.md"), "shared once\n").expect("shared"); + fs::write(claude.join("CLAUDE.md"), "@shared.md\n").expect("claude md"); + fs::write(claude.join("rules").join("r.md"), "@../shared.md\n").expect("rule"); + + let loaded = provider(home.path(), 32 * 1024) + .load_user_instructions() + .await; + + let text = loaded.instructions.expect("instructions").text; + assert_eq!(text.matches("shared once").count(), 1, "{text:?}"); +} + +#[tokio::test] +async fn the_budget_covers_claude_files_and_rules_together() { + let home = tempfile::tempdir().expect("home"); + let claude = home.path().join(".claude"); + fs::create_dir_all(claude.join("rules")).expect("rules dir"); + let claude_md = "0123456789abcdef"; + fs::write(claude.join("CLAUDE.md"), claude_md).expect("claude md"); + fs::write(claude.join("rules").join("r.md"), "rule body").expect("rule"); + + let loaded = provider(home.path(), claude_md.len()) + .load_user_instructions() + .await; + + let text = loaded.instructions.expect("instructions").text; + assert!(text.contains(claude_md)); + assert!( + !text.contains("rule body"), + "rule must be past the budget: {text:?}" + ); + assert_eq!(loaded.warnings.len(), 1); +} diff --git a/codex-rs/codex-home/src/instructions/imports.rs b/codex-rs/codex-home/src/instructions/imports.rs new file mode 100644 index 000000000000..1cdcb07b5163 --- /dev/null +++ b/codex-rs/codex-home/src/instructions/imports.rs @@ -0,0 +1,385 @@ +//! Shared `@import` expansion for Claude-compat instruction files. +//! +//! Fork-owned (WI-1131). Both the global user-instructions provider and the +//! per-directory project-doc loader (WI-1132) expand imports through here, so +//! the rules live in one place: +//! +//! - only a line that is nothing but `@path` is an import, and only outside a +//! fenced code block (` ``` ` or `~~~`); +//! - the path is resolved relative to the importing file, `~/` means the home +//! directory, and an absolute path is taken as written; +//! - expansion stops at depth 5; +//! - a file reached twice (by path, by symlink, or by identical body) is kept +//! once, which also ends cycles; +//! - every byte read counts against the caller's remaining budget. + +use std::collections::HashSet; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +use sha2::Digest; +use sha2::Sha256; + +/// Deepest import nesting that is still expanded. A file at this depth is +/// included verbatim; its own imports are left as written. +pub const MAX_IMPORT_DEPTH: usize = 5; + +/// One file an expansion read, in the order it was first reached. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ImportedFile { + /// Canonical path of the file, after symlink resolution. + pub path: PathBuf, + /// The file's text after its own imports were expanded. + pub text: String, +} + +/// Outcome of expanding one instruction file. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct ImportExpansion { + /// Files that contributed text, in first-seen order. + pub files: Vec, + /// Bytes consumed from the budget, including files that were deduplicated. + pub bytes_read: usize, + /// Recoverable problems: a file that could not be read, or a file skipped + /// because the byte budget was exhausted. + pub warnings: Vec, +} + +/// Expands `@path` imports starting at `path`. +/// +/// `budget` is the number of bytes still available; the expansion stops reading +/// once it is spent and reports the skipped file as a warning. `home` resolves +/// `~/` references and may be `None` when no home directory is known. +/// +/// `allowed_roots` confines every file the expansion reads. After a path is +/// canonicalized (so a symlink or `..` cannot slip past), it must sit inside +/// one of the roots; anything else is refused and reported as a warning. +/// `None` confines nothing, which is what the global user-instructions path +/// uses. An empty slice confines everything. +pub async fn expand_imports( + path: &Path, + home: Option<&Path>, + budget: usize, + allowed_roots: Option<&[PathBuf]>, +) -> ImportExpansion { + let mut expansion = ImportExpansion::default(); + expand_with( + &mut expansion, + std::slice::from_ref(&path.to_path_buf()), + home, + budget, + allowed_roots, + ) + .await; + expansion +} + +/// Expands several files as one snapshot. +/// +/// Files share the byte budget and the path and body deduplication, so a file +/// reached from two of them is kept once and every byte counts once. `budget` +/// is the total still available across all of `paths`. `allowed_roots` is the +/// same confinement as [`expand_imports`]. +pub async fn expand_imports_all( + paths: &[PathBuf], + home: Option<&Path>, + budget: usize, + allowed_roots: Option<&[PathBuf]>, +) -> ImportExpansion { + let mut expansion = ImportExpansion::default(); + expand_with(&mut expansion, paths, home, budget, allowed_roots).await; + expansion +} + +async fn expand_with( + expansion: &mut ImportExpansion, + paths: &[PathBuf], + home: Option<&Path>, + budget: usize, + allowed_roots: Option<&[PathBuf]>, +) { + let mut expander = Expander { + home: home.map(Path::to_path_buf), + allowed_roots: canonicalize_roots(allowed_roots).await, + seen_paths: HashSet::new(), + seen_bodies: HashSet::new(), + files: Vec::new(), + bytes_read: 0, + budget, + warnings: Vec::new(), + }; + for path in paths { + expander.add(path, 0).await; + } + expansion.files.append(&mut expander.files); + expansion.bytes_read += expander.bytes_read; + expansion.warnings.append(&mut expander.warnings); +} + +/// Canonicalizes each root that exists. A root that does not is dropped, which +/// only ever makes the confinement stricter. +async fn canonicalize_roots(roots: Option<&[PathBuf]>) -> Option> { + let roots = roots?; + let mut canonical = Vec::with_capacity(roots.len()); + for root in roots { + if let Ok(path) = tokio::fs::canonicalize(root).await { + canonical.push(path); + } + } + Some(canonical) +} + +struct Expander { + home: Option, + /// Canonical roots a file must fall inside, when confinement is on. + allowed_roots: Option>, + seen_paths: HashSet, + seen_bodies: HashSet<[u8; 32]>, + files: Vec, + bytes_read: usize, + budget: usize, + warnings: Vec, +} + +impl Expander { + /// Loads `path` and what it imports. A file already loaded is skipped. + async fn add(&mut self, path: &Path, depth: usize) { + let Some((canonical, text)) = self.read(path).await else { + return; + }; + if !self.seen_paths.insert(canonical.clone()) { + return; + } + let index = self.files.len(); + self.files.push(ImportedFile { + path: canonical.clone(), + text: String::new(), + }); + let body = self.expand(&text, canonical.parent(), depth).await; + let body = body.trim().to_string(); + // A file that is one import which could not be read contributed + // nothing, so it is not kept. A file that did expand is kept even when + // its own text matches what it imported. + let unresolved_import = text.trim().lines().count() == 1 + && line_only_import(text.trim()).is_some() + && body == text.trim(); + let identity = if unresolved_import { + format!("{depth}\n{}", text.trim()) + } else { + text.trim().to_string() + }; + let digest = Sha256::digest(identity.as_bytes()); + let fresh = self.seen_bodies.insert(digest.into()); + if body.is_empty() || (unresolved_import && depth < MAX_IMPORT_DEPTH) || !fresh { + self.files.remove(index); + return; + } + self.files[index].text = body; + } + + /// Whether `path` resolves inside an allowed root. + /// + /// Walks up to the nearest ancestor that exists and canonicalizes that, so a + /// reference to a file that has not been created yet is still confined. With + /// no roots configured nothing is confined. + async fn within_allowed_roots(&self, path: &Path) -> bool { + let Some(roots) = &self.allowed_roots else { + return true; + }; + let mut ancestor = path; + let mut rest = PathBuf::new(); + let canonical = loop { + if ancestor.as_os_str().is_empty() { + return false; + } + match tokio::fs::canonicalize(ancestor).await { + Ok(canonical) => break canonical, + Err(_) => match ancestor.parent() { + Some(parent) => { + if let Some(name) = ancestor.file_name() { + rest = Path::new(name).join(&rest); + } + ancestor = parent; + } + None => return false, + }, + } + }; + let resolved = canonical.join(rest); + roots.iter().any(|root| resolved.starts_with(root)) + } + + /// Reads a regular file within the remaining budget. + async fn read(&mut self, path: &Path) -> Option<(PathBuf, String)> { + let canonical = match tokio::fs::canonicalize(path).await { + Ok(path) => path, + Err(err) if err.kind() == io::ErrorKind::NotFound => { + // A missing file is normally not an import. Once confinement is + // on it is also the way past it: a path that cannot be + // canonicalized cannot be shown to fall inside a root, and + // returning None would leave it unreported. Only a path that + // resolves inside a root may be treated as absent. + if self.allowed_roots.is_some() && !self.within_allowed_roots(path).await { + self.warnings.push(format!( + "Refused instructions from `{}`: it is outside the allowed project roots", + path.display() + )); + } + return None; + } + Err(err) => { + self.warnings.push(format!( + "Failed to read instructions from `{}`: {err}", + path.display() + )); + return None; + } + }; + if let Some(roots) = &self.allowed_roots + && !roots.iter().any(|root| canonical.starts_with(root)) + { + self.warnings.push(format!( + "Refused instructions from `{}`: it is outside the allowed project roots", + canonical.display() + )); + return None; + } + let metadata = match tokio::fs::metadata(&canonical).await { + Ok(metadata) if metadata.is_file() => metadata, + Ok(_) => return None, + Err(err) => { + self.warnings.push(format!( + "Failed to read instructions from `{}`: {err}", + canonical.display() + )); + return None; + } + }; + let len = usize::try_from(metadata.len()).unwrap_or(usize::MAX); + if self.bytes_read.saturating_add(len) > self.budget { + self.warnings.push(format!( + "Skipped instructions from `{}`: the {} byte project_doc_max_bytes budget is exhausted", + canonical.display(), + self.budget + )); + return None; + } + let data = match tokio::fs::read(&canonical).await { + Ok(data) => data, + Err(err) => { + self.warnings.push(format!( + "Failed to read instructions from `{}`: {err}", + canonical.display() + )); + return None; + } + }; + self.bytes_read += data.len(); + Some((canonical, String::from_utf8_lossy(&data).into_owned())) + } + + /// Strips HTML comments and resolves line-only imports in `text`. + async fn expand(&mut self, text: &str, dir: Option<&Path>, depth: usize) -> String { + let text = strip_html_comments(text); + let mut out = Vec::new(); + let mut in_fence = false; + for line in text.split('\n') { + let trimmed = line.trim(); + if trimmed.starts_with("```") || trimmed.starts_with("~~~") { + in_fence = !in_fence; + out.push(line.to_string()); + continue; + } + if in_fence || depth >= MAX_IMPORT_DEPTH { + out.push(line.to_string()); + continue; + } + if let Some(reference) = line_only_import(trimmed) + && let Some(imported) = Box::pin(self.import_file(reference, dir, depth)).await + { + if !imported.is_empty() { + out.push(imported); + } + continue; + } + out.push(line.to_string()); + } + out.join("\n") + } + + /// Resolves one `@path` reference. `None` means it does not name a readable + /// file, so the caller leaves the line as written. + async fn import_file( + &mut self, + reference: &str, + dir: Option<&Path>, + depth: usize, + ) -> Option { + let path = self.resolve(reference, dir)?; + let (canonical, text) = self.read(&path).await?; + if !self.seen_paths.insert(canonical.clone()) { + return Some(String::new()); + } + let body = Box::pin(self.expand(&text, canonical.parent(), depth + 1)) + .await + .trim() + .to_string(); + if body.is_empty() { + return Some(String::new()); + } + // A file that is only an import has no body of its own, so its identity + // is the depth it was reached at plus what it resolved to. Two links in + // a chain otherwise hash to the same text and the whole chain collapses. + let pure_import = + text.trim().lines().count() == 1 && line_only_import(text.trim()).is_some(); + let identity = if pure_import { + format!("{}\n{body}", depth + 1) + } else { + text.trim().to_string() + }; + let digest = Sha256::digest(identity.as_bytes()); + if !self.seen_bodies.insert(digest.into()) { + return Some(String::new()); + } + Some(body) + } + + fn resolve(&self, reference: &str, dir: Option<&Path>) -> Option { + if let Some(rest) = reference.strip_prefix("~/") { + return Some(self.home.as_ref()?.join(rest)); + } + let path = Path::new(reference); + if path.is_absolute() { + return Some(path.to_path_buf()); + } + Some(dir?.join(path)) + } +} + +/// A line that is exactly `@path`, with the path characters Klaudia accepts. +fn line_only_import(trimmed: &str) -> Option<&str> { + let reference = trimmed.strip_prefix('@')?; + if reference.is_empty() + || !reference + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '_' | '.' | '/' | '-' | '~')) + { + return None; + } + Some(reference) +} + +fn strip_html_comments(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut rest = text; + while let Some(start) = rest.find("") { + Some(end) => rest = &rest[start + 4 + end + 3..], + None => return out, + } + } + out.push_str(rest); + out +} diff --git a/codex-rs/codex-home/src/instructions/imports_tests.rs b/codex-rs/codex-home/src/instructions/imports_tests.rs new file mode 100644 index 000000000000..67e4b13c56b2 --- /dev/null +++ b/codex-rs/codex-home/src/instructions/imports_tests.rs @@ -0,0 +1,216 @@ +use std::fs; +use std::os::unix::fs::symlink; +use std::path::Path; + +use pretty_assertions::assert_eq; + +use super::MAX_IMPORT_DEPTH; +use super::expand_imports; + +fn write(path: &Path, text: &str) { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).expect("create parent"); + } + fs::write(path, text).expect("write file"); +} + +#[tokio::test] +async fn line_only_import_is_inlined_and_counted() { + let dir = tempfile::tempdir().expect("temp dir"); + write(&dir.path().join("AGENTS.md"), "shared rules\n"); + write( + &dir.path().join("CLAUDE.md"), + "preamble\n@AGENTS.md\nclosing\n", + ); + + let expansion = expand_imports(&dir.path().join("CLAUDE.md"), None, 1024, None).await; + + assert_eq!(expansion.warnings, Vec::::new()); + assert_eq!(expansion.files.len(), 1); + assert_eq!(expansion.files[0].text, "preamble\nshared rules\nclosing"); + assert_eq!( + expansion.bytes_read, + fs::read(dir.path().join("CLAUDE.md")).unwrap().len() + + fs::read(dir.path().join("AGENTS.md")).unwrap().len() + ); +} + +#[tokio::test] +async fn imports_inside_fences_and_mid_line_are_left_alone() { + let dir = tempfile::tempdir().expect("temp dir"); + write(&dir.path().join("real.md"), "imported\n"); + write( + &dir.path().join("CLAUDE.md"), + "see @real.md inline\n```\n@real.md\n```\n~~~@real.md\n", + ); + + let expansion = expand_imports(&dir.path().join("CLAUDE.md"), None, 1024, None).await; + + assert_eq!(expansion.files.len(), 1); + assert_eq!( + expansion.files[0].text, + "see @real.md inline\n```\n@real.md\n```\n~~~@real.md" + ); + assert_eq!( + expansion.bytes_read, + fs::read(dir.path().join("CLAUDE.md")).unwrap().len() + ); +} + +#[tokio::test] +async fn expansion_stops_at_depth_five() { + let dir = tempfile::tempdir().expect("temp dir"); + write(&dir.path().join("level6.md"), "too deep\n"); + let mut next = "level6.md".to_string(); + for level in (1..=MAX_IMPORT_DEPTH).rev() { + let name = format!("level{level}.md"); + write(&dir.path().join(&name), &format!("@{next}\n")); + next = name; + } + write(&dir.path().join("CLAUDE.md"), &format!("@{next}\n")); + + let expansion = expand_imports(&dir.path().join("CLAUDE.md"), None, 1024, None).await; + + let text = &expansion.files[0].text; + assert!( + text.contains("@level6.md"), + "the import past depth {MAX_IMPORT_DEPTH} must stay literal, got {text:?}" + ); + assert!(!text.contains("too deep")); +} + +#[tokio::test] +async fn duplicate_path_symlink_and_body_are_kept_once() { + let dir = tempfile::tempdir().expect("temp dir"); + write(&dir.path().join("shared.md"), "same body\n"); + symlink("shared.md", dir.path().join("link.md")).expect("symlink"); + write(&dir.path().join("copy.md"), "same body\n"); + write( + &dir.path().join("CLAUDE.md"), + "@shared.md\n@link.md\n@copy.md\n@shared.md\n", + ); + + let expansion = expand_imports(&dir.path().join("CLAUDE.md"), None, 1024, None).await; + + assert_eq!(expansion.files[0].text, "same body"); + assert_eq!(expansion.files.len(), 1); +} + +#[tokio::test] +async fn byte_budget_stops_the_expansion() { + let dir = tempfile::tempdir().expect("temp dir"); + write(&dir.path().join("big.md"), "0123456789"); + write(&dir.path().join("CLAUDE.md"), "@big.md\n"); + let budget = fs::read(dir.path().join("CLAUDE.md")).unwrap().len(); + + let expansion = expand_imports(&dir.path().join("CLAUDE.md"), None, budget, None).await; + + assert!(expansion.files.is_empty()); + assert_eq!(expansion.warnings.len(), 1); + assert!(expansion.warnings[0].contains("project_doc_max_bytes")); +} + +#[tokio::test] +async fn home_prefix_resolves_against_the_given_home() { + let home = tempfile::tempdir().expect("temp dir"); + let dir = tempfile::tempdir().expect("temp dir"); + write(&home.path().join("shared.md"), "from home\n"); + write(&dir.path().join("CLAUDE.md"), "@~/shared.md\n"); + + let expansion = + expand_imports(&dir.path().join("CLAUDE.md"), Some(home.path()), 1024, None).await; + + assert_eq!(expansion.files[0].text, "from home"); +} + +#[tokio::test] +async fn a_cycle_is_included_once() { + let dir = tempfile::tempdir().expect("temp dir"); + write(&dir.path().join("a.md"), "alpha\n@b.md\n"); + write(&dir.path().join("b.md"), "beta\n@a.md\n"); + + let expansion = expand_imports(&dir.path().join("a.md"), None, 1024, None).await; + + assert_eq!(expansion.files.len(), 1); + assert_eq!(expansion.files[0].text, "alpha\nbeta"); +} + +/// A file outside the project root must not be read, however the import +/// names it: `~/`, an absolute path, a `..` climb, or a symlink that resolves +/// outside the root. +#[tokio::test] +async fn imports_outside_the_allowed_roots_are_refused() { + let project = tempfile::tempdir().expect("temp dir"); + let outside = tempfile::tempdir().expect("temp dir"); + let home = tempfile::tempdir().expect("temp dir"); + write(&outside.path().join("secret.md"), "secret\n"); + write(&home.path().join("secret.md"), "secret\n"); + symlink( + outside.path().join("secret.md"), + project.path().join("escape.md"), + ) + .expect("symlink"); + write( + &project.path().join("CLAUDE.md"), + "@~/secret.md\n@/etc/hostname\n@../secret.md\n@escape.md\n", + ); + let roots = vec![project.path().to_path_buf()]; + + let expansion = expand_imports( + &project.path().join("CLAUDE.md"), + Some(home.path()), + 1024, + Some(&roots), + ) + .await; + + let text = expansion + .files + .iter() + .map(|file| file.text.as_str()) + .collect::>() + .join("\n"); + // A refused import stays as the line that named it, so the text still + // contains the references; only the project file itself may be read. + assert_eq!( + expansion.bytes_read, + fs::read(project.path().join("CLAUDE.md")).unwrap().len(), + "an import outside the root was read: {text:?}" + ); + assert!( + expansion.warnings.len() >= 3, + "each refused import must be reported, got {:?}", + expansion.warnings + ); + assert!( + expansion + .warnings + .iter() + .all(|warning| warning.contains("outside the allowed project roots")), + "unexpected warning: {:?}", + expansion.warnings + ); +} + +/// A reference to a file that does not exist must still be confined. Treating +/// "not found" as "not an import" would let `@/etc/hostname` through on a host +/// where that file is absent, which is exactly where the confinement matters. +#[tokio::test] +async fn a_missing_file_outside_the_roots_is_refused() { + let project = tempfile::tempdir().expect("temp dir"); + write( + &project.path().join("CLAUDE.md"), + "@missing.md\n@/no/such/file.md\n", + ); + let roots = vec![project.path().to_path_buf()]; + + let expansion = + expand_imports(&project.path().join("CLAUDE.md"), None, 1024, Some(&roots)).await; + + assert_eq!(expansion.warnings.len(), 1, "{:?}", expansion.warnings); + assert!(expansion.warnings[0].contains("/no/such/file.md")); + assert_eq!( + expansion.bytes_read, + fs::read(project.path().join("CLAUDE.md")).unwrap().len() + ); +} diff --git a/codex-rs/codex-home/src/instructions/mod.rs b/codex-rs/codex-home/src/instructions/mod.rs index a16ad668af3c..dcb4fc79922a 100644 --- a/codex-rs/codex-home/src/instructions/mod.rs +++ b/codex-rs/codex-home/src/instructions/mod.rs @@ -12,6 +12,18 @@ use codex_utils_absolute_path::AbsolutePathBuf; const DEFAULT_AGENTS_MD_FILENAME: &str = "AGENTS.md"; const LOCAL_AGENTS_MD_FILENAME: &str = "AGENTS.override.md"; +// Fork (WI-1131): Claude Code global instructions and the shared @import +// expander. `imports` is public so the per-directory loader (WI-1132) reuses it. +pub mod claude_compat; +pub mod imports; + +#[cfg(test)] +pub(crate) use claude_compat::ClaudeCompatUserInstructionsProvider; +#[cfg(test)] +pub(crate) use imports::MAX_IMPORT_DEPTH; +#[cfg(test)] +pub(crate) use imports::expand_imports; + /// Loads user instructions from a Codex home directory. #[derive(Clone, Debug)] pub struct CodexHomeUserInstructionsProvider { @@ -111,3 +123,9 @@ impl UserInstructionsProvider for CodexHomeUserInstructionsProvider { #[cfg(test)] mod tests; + +#[cfg(test)] +mod imports_tests; + +#[cfg(test)] +mod claude_compat_tests; diff --git a/codex-rs/codex-home/src/lib.rs b/codex-rs/codex-home/src/lib.rs index 7ca5e5807258..cac8f8a4c027 100644 --- a/codex-rs/codex-home/src/lib.rs +++ b/codex-rs/codex-home/src/lib.rs @@ -1,3 +1,6 @@ mod instructions; pub use instructions::CodexHomeUserInstructionsProvider; +pub use instructions::claude_compat::ClaudeCompatUserInstructionsProvider; +pub use instructions::claude_compat::user_instructions_provider; +pub use instructions::imports::expand_imports; diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index 3e5cc07e6ca6..1611d7f07221 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -336,6 +336,21 @@ pub struct ConfigToml { #[serde(default = "default_project_doc_fallback_filenames")] pub project_doc_fallback_filenames: Option>, + /// Fork (WI-1131): also load the user's global Claude Code instructions + /// (`~/.claude/CLAUDE.md` and `~/.claude/rules/*.md`) into the user + /// instructions snapshot. Off by default, so upstream behaviour is unchanged + /// unless a config sets it. + #[serde(default)] + pub claude_compat_instructions: Option, + + /// Fork (WI-1132): how a directory's project docs are chosen. `first` (the + /// default) keeps the upstream behaviour of the first matching filename per + /// directory. `all` loads every matching filename, plus `.claude/rules/*.md` + /// in sorted order, and expands `@path` imports confined to the project + /// root. An unrecognised value falls back to `first`. + #[serde(default)] + pub project_doc_mode: Option, + /// Token budget applied when storing tool/function outputs in the context manager. pub tool_output_token_limit: Option, diff --git a/codex-rs/config/src/hook_config.rs b/codex-rs/config/src/hook_config.rs index 7575fa5fcb10..8fbcd7ac79bd 100644 --- a/codex-rs/config/src/hook_config.rs +++ b/codex-rs/config/src/hook_config.rs @@ -182,6 +182,15 @@ pub enum HookHandlerConfig { skip_serializing_if = "Option::is_none" )] additional_context_limit: Option, + /// When true, a failure of this command hook blocks the tool instead of + /// failing open. Defaults to false, so existing handlers are unchanged. + /// Only command hooks carry this field; MCP tool hooks always fail open. + /// It also has no effect on a hook that never runs, such as an untrusted + /// user or project hook, or when the hooks feature is disabled. An async + /// handler that sets it runs synchronously, because a block can only be + /// applied before the tool starts. + #[serde(default, rename = "failClosed")] + fail_closed: bool, }, #[serde(rename = "mcp_tool")] McpTool { diff --git a/codex-rs/config/src/hooks_tests.rs b/codex-rs/config/src/hooks_tests.rs index bd71a7f0e323..8c83854fc69f 100644 --- a/codex-rs/config/src/hooks_tests.rs +++ b/codex-rs/config/src/hooks_tests.rs @@ -48,6 +48,7 @@ fn hooks_file_deserializes_existing_json_shape() { r#async: false, status_message: Some("checking".to_string()), additional_context_limit: Some(4096), + fail_closed: false, }], }], ..Default::default() @@ -56,6 +57,33 @@ fn hooks_file_deserializes_existing_json_shape() { ); } +#[test] +fn command_hook_fail_closed_defaults_false_and_round_trips() { + let omitted = serde_json::from_str::( + r#"{ "type": "command", "command": "python3 /tmp/pre.py" }"#, + ) + .expect("deserialize"); + assert!(matches!( + omitted, + HookHandlerConfig::Command { + fail_closed: false, + .. + } + )); + + let set = serde_json::from_str::( + r#"{ "type": "command", "command": "python3 /tmp/pre.py", "failClosed": true }"#, + ) + .expect("deserialize"); + assert!(matches!( + set, + HookHandlerConfig::Command { + fail_closed: true, + .. + } + )); +} + #[test] fn hooks_file_deserializes_mcp_tool_handler_with_json_inputs() { let parsed: HooksFile = serde_json::from_value(serde_json::json!({ @@ -182,6 +210,7 @@ additionalContextLimit = 4096 r#async: false, status_message: Some("checking".to_string()), additional_context_limit: Some(4096), + fail_closed: false, }], }], ..Default::default() @@ -220,6 +249,7 @@ command = "python3 /tmp/pre.py" r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -266,6 +296,7 @@ command = "python3 /enterprise/place/pre.py" r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -303,6 +334,7 @@ command_windows = "powershell -File C:\\enterprise\\hooks\\pre.ps1" r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -339,6 +371,7 @@ commandWindows = "powershell -File C:\\enterprise\\hooks\\pre.ps1" r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -355,6 +388,7 @@ fn hook_handler_omits_unset_additional_context_limit() { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }; let serialized = serde_json::to_value(handler).expect("hook handler should serialize"); diff --git a/codex-rs/config/src/skills_config.rs b/codex-rs/config/src/skills_config.rs index 9a8a338fe1e0..1ffe63a59c01 100644 --- a/codex-rs/config/src/skills_config.rs +++ b/codex-rs/config/src/skills_config.rs @@ -53,6 +53,16 @@ pub struct SkillsConfig { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub config: Vec, + + /// Fork (WI-1133): extra user skill roots, relative to the user config + /// directory. The built-in roots are always loaded; these are added. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub extra_user_roots: Vec, + + /// Fork (WI-1133): extra project skill roots, relative to each directory + /// between the project root and the working directory. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub extra_project_roots: Vec, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 5864a5df77c6..5eee067c89b9 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2110,6 +2110,11 @@ "default": null, "type": "string" }, + "failClosed": { + "default": false, + "description": "When true, a failure of this command hook blocks the tool instead of failing open. Defaults to false, so existing handlers are unchanged. Only command hooks carry this field; MCP tool hooks always fail open. It also has no effect on a hook that never runs, such as an untrusted user or project hook, or when the hooks feature is disabled. An async handler that sets it runs synchronously, because a block can only be applied before the tool starts.", + "type": "boolean" + }, "statusMessage": { "default": null, "type": "string" @@ -4269,6 +4274,20 @@ }, "type": "array" }, + "extra_project_roots": { + "description": "Fork (WI-1133): extra project skill roots, relative to each directory between the project root and the working directory.", + "items": { + "type": "string" + }, + "type": "array" + }, + "extra_user_roots": { + "description": "Fork (WI-1133): extra user skill roots, relative to the user config directory. The built-in roots are always loaded; these are added.", + "items": { + "type": "string" + }, + "type": "array" + }, "include_instructions": { "description": "Whether turns receive the automatic skills instructions block.", "type": "boolean" @@ -6850,6 +6869,11 @@ "description": "When `true`, checks for Codex updates on startup and surfaces update prompts. Set to `false` only if your Codex updates are centrally managed. Defaults to `true`.", "type": "boolean" }, + "claude_compat_instructions": { + "default": null, + "description": "Fork (WI-1131): also load the user's global Claude Code instructions (`~/.claude/CLAUDE.md` and `~/.claude/rules/*.md`) into the user instructions snapshot. Off by default, so upstream behaviour is unchanged unless a config sets it.", + "type": "boolean" + }, "cli_auth_credentials_store": { "allOf": [ { @@ -7815,6 +7839,11 @@ "minimum": 0.0, "type": "integer" }, + "project_doc_mode": { + "default": null, + "description": "Fork (WI-1132): how a directory's project docs are chosen. `first` (the default) keeps the upstream behaviour of the first matching filename per directory. `all` loads every matching filename, plus `.claude/rules/*.md` in sorted order, and expands `@path` imports confined to the project root. An unrecognised value falls back to `first`.", + "type": "string" + }, "project_root_markers": { "default": null, "description": "Markers used to detect the project root when searching parent directories for `.codex` folders. Defaults to [\".git\"] when unset.", diff --git a/codex-rs/core/src/agent/control/spawn.rs b/codex-rs/core/src/agent/control/spawn.rs index 20e28a8bfed7..8c51f05a5c23 100644 --- a/codex-rs/core/src/agent/control/spawn.rs +++ b/codex-rs/core/src/agent/control/spawn.rs @@ -925,6 +925,7 @@ impl LocalAgentControl { turn_trigger: options.turn_trigger, root_turn_id: options.root_turn_id, cyber_access_program: options.cyber_access_program, + final_output_json_schema: options.final_output_json_schema, ..Default::default() }; let input_admission_started_at = Instant::now(); diff --git a/codex-rs/core/src/agent/types.rs b/codex-rs/core/src/agent/types.rs index 852a51671364..7b80eeae9395 100644 --- a/codex-rs/core/src/agent/types.rs +++ b/codex-rs/core/src/agent/types.rs @@ -36,6 +36,9 @@ pub struct SpawnAgentOptions { pub environments: Option, pub multi_agent_v2_usage_hints: Option, pub cyber_access_program: Option, + /// JSON schema the child's first turn must satisfy as its final output. + /// Plumbed into `TurnStartOptions::final_output_json_schema`; not re-validated here. + pub final_output_json_schema: Option, } /// Identity and status observed from a loaded agent, without a handle to its runtime. diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 8e0c475d9033..77fff8278962 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -139,6 +139,30 @@ async fn read_agents_md( return Ok(None); } + if config.project_doc_all { + let root = project_root(config, cwd, fs, sandbox).await?; + let root = root.as_ref().unwrap_or(cwd); + let docs = + crate::agents_md_compat::expand_project_docs(fs, &paths, root, max_total, sandbox) + .await?; + let mut loaded = LoadedAgentsMd::default(); + for doc in docs { + loaded.entries.push(InstructionEntry { + contents: doc.text, + provenance: InstructionProvenance::Project { + source_path: doc.path, + environment_id: environment_id.to_string(), + cwd: cwd.clone(), + }, + }); + } + return if loaded.is_empty() { + Ok(None) + } else { + Ok(Some(loaded)) + }; + } + let mut remaining: u64 = max_total as u64; let mut loaded = LoadedAgentsMd::default(); @@ -195,31 +219,8 @@ async fn agents_md_paths( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> io::Result> { + let project_root = project_root(config, cwd, fs, sandbox).await?; let dir = cwd.clone(); - - let mut merged = TomlValue::Table(toml::map::Map::new()); - for layer in config.config_layer_stack.layers_low_to_high() { - if matches!(layer.name, ConfigLayerSource::Project { .. }) { - continue; - } - merge_toml_values(&mut merged, &layer.config); - } - let project_root_markers = match project_root_markers_from_config(&merged) { - Ok(Some(markers)) => markers, - Ok(None) => default_project_root_markers(), - Err(err) => { - tracing::warn!("invalid project_root_markers: {err}"); - default_project_root_markers() - } - }; - let project_root = find_nearest_ancestor_with_markers( - fs, - &dir, - project_root_markers, - FindUpErrorPolicy::Ignore, - sandbox, - ) - .await?; let search_dirs = if let Some(root) = project_root { let mut dirs = Vec::new(); let mut cursor = dir.clone(); @@ -240,6 +241,23 @@ async fn agents_md_paths( }; let candidate_filenames = candidate_filenames(config, cwd); + if config.project_doc_all { + let mut found = Vec::new(); + for directory in &search_dirs { + let root = search_dirs.first().unwrap_or(directory); + found.extend( + crate::agents_md_compat::discover_compat_docs( + fs, + directory, + &candidate_filenames, + root, + sandbox, + ) + .await?, + ); + } + return Ok(found); + } let candidate_filenames = &candidate_filenames; let mut results = futures::stream::iter(search_dirs) .map(|directory| async move { @@ -269,6 +287,34 @@ async fn agents_md_paths( Ok(found) } +/// The nearest ancestor of `cwd` carrying a project-root marker. +/// +/// Markers come from the config layers above the project layer, so a project +/// cannot widen its own root and reach files outside it. +async fn project_root( + config: &Config, + cwd: &PathUri, + fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, +) -> io::Result> { + let mut merged = TomlValue::Table(toml::map::Map::new()); + for layer in config.config_layer_stack.layers_low_to_high() { + if matches!(layer.name, ConfigLayerSource::Project { .. }) { + continue; + } + merge_toml_values(&mut merged, &layer.config); + } + let markers = match project_root_markers_from_config(&merged) { + Ok(Some(markers)) => markers, + Ok(None) => default_project_root_markers(), + Err(err) => { + tracing::warn!("invalid project_root_markers: {err}"); + default_project_root_markers() + } + }; + find_nearest_ancestor_with_markers(fs, cwd, markers, FindUpErrorPolicy::Ignore, sandbox).await +} + fn candidate_filenames<'a>(config: &'a Config, cwd: &PathUri) -> Vec<&'a str> { let mut names: Vec<&str> = Vec::with_capacity(2 + config.project_doc_fallback_filenames.len()); names.push(LOCAL_AGENTS_MD_FILENAME); diff --git a/codex-rs/core/src/agents_md_compat.rs b/codex-rs/core/src/agents_md_compat.rs new file mode 100644 index 000000000000..7a7681a33cde --- /dev/null +++ b/codex-rs/core/src/agents_md_compat.rs @@ -0,0 +1,254 @@ +//! Fork (WI-1132): the `project_doc_mode = "all"` project-doc loader. +//! +//! Where the upstream discovery keeps the first matching filename in each +//! directory, this loads every matching filename and then `.claude/rules/*.md` +//! in sorted order, and expands line-only `@path` imports. Every file is read +//! through the executor filesystem and, after canonicalization, must fall +//! inside the project root, so a hostile repo cannot pull in `~/.codex`, +//! `/etc`, a `..` climb or a symlink that escapes the project. + +use std::collections::HashSet; +use std::hash::DefaultHasher; +use std::hash::Hash; +use std::hash::Hasher; +use std::io; + +use codex_file_system::ExecutorFileSystem; +use codex_file_system::FileSystemSandboxContext; +use codex_file_system::GetMetadataOptions; +use codex_file_system::ReadFileOptions; +use codex_utils_path_uri::PathUri; + +/// A file the `all` mode wants loaded, in the order it should appear. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct CompatDoc { + pub path: PathUri, + pub text: String, +} + +const RULES_DIR: &str = ".claude/rules"; +const MAX_IMPORT_DEPTH: usize = 5; + +/// Discovers the docs for one directory. +/// +/// `names` is the candidate filename list. Every name that names a file is +/// kept, in the order given, and `.claude/rules/*.md` follows in sorted order. +/// `root` is the project root the caller already computed; a doc or an import +/// that canonicalizes outside it is skipped. +pub(crate) async fn discover_compat_docs( + fs: &dyn ExecutorFileSystem, + directory: &PathUri, + names: &[&str], + root: &PathUri, + sandbox: Option<&FileSystemSandboxContext>, +) -> io::Result> { + let mut found = Vec::new(); + for name in names { + let candidate = directory + .join(name) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; + if is_file(fs, &candidate, sandbox).await? { + found.push(candidate); + } + } + + let rules_dir = directory + .join(RULES_DIR) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; + if let Some(mut rules) = rule_files(fs, &rules_dir, root, sandbox).await? { + rules.sort_by_key(ToString::to_string); + found.extend(rules); + } + Ok(found) +} + +/// Reads `paths` and expands their `@path` imports. +/// +/// `budget` is the number of bytes still available and counts every file read, +/// including one reached only through an import. Files are deduped by their +/// canonical path and by their body, and expansion stops at depth 5. +pub(crate) async fn expand_project_docs( + fs: &dyn ExecutorFileSystem, + paths: &[PathUri], + root: &PathUri, + budget: usize, + sandbox: Option<&FileSystemSandboxContext>, +) -> io::Result> { + let mut seen_paths = HashSet::new(); + let mut seen_bodies = HashSet::new(); + let mut docs = Vec::new(); + let mut remaining = budget; + for path in paths { + read_doc( + fs, + path, + root, + /*depth*/ 0, + sandbox, + &mut seen_paths, + &mut seen_bodies, + &mut docs, + &mut remaining, + ) + .await?; + } + Ok(docs) +} + +#[allow(clippy::too_many_arguments)] +async fn read_doc( + fs: &dyn ExecutorFileSystem, + path: &PathUri, + root: &PathUri, + depth: usize, + sandbox: Option<&FileSystemSandboxContext>, + seen_paths: &mut HashSet, + seen_bodies: &mut HashSet, + docs: &mut Vec, + remaining: &mut usize, +) -> io::Result<()> { + if *remaining == 0 || depth > MAX_IMPORT_DEPTH { + return Ok(()); + } + let canonical = match fs.canonicalize(path, sandbox).await { + Ok(path) => path, + Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(err) => return Err(err), + }; + if !canonical.starts_with(root) { + tracing::warn!(path = %canonical, "refusing project doc outside the project root"); + return Ok(()); + } + if !seen_paths.insert(canonical.to_string()) { + return Ok(()); + } + + let mut data = fs + .read_file(&canonical, ReadFileOptions::default(), sandbox) + .await?; + if data.len() > *remaining { + data.truncate(*remaining); + } + *remaining = remaining.saturating_sub(data.len()); + let text = String::from_utf8_lossy(&data).to_string(); + + let mut body = String::new(); + if depth < MAX_IMPORT_DEPTH { + for line in text.lines() { + let trimmed = line.trim(); + if let Some(reference) = line_only_import(trimmed) { + let imported = resolve_import(path, reference)?; + let before = docs.len(); + Box::pin(read_doc( + fs, + &imported, + root, + depth + 1, + sandbox, + seen_paths, + seen_bodies, + docs, + remaining, + )) + .await?; + if docs.len() == before { + body.push_str(line); + body.push('\n'); + } + } else { + body.push_str(line); + body.push('\n'); + } + } + } else { + body = text; + } + + let digest = { + let mut hasher = DefaultHasher::new(); + body.trim().hash(&mut hasher); + hasher.finish() + }; + if body.trim().is_empty() || !seen_bodies.insert(digest) { + return Ok(()); + } + docs.push(CompatDoc { + path: canonical, + text: body.trim().to_string(), + }); + Ok(()) +} + +/// A line that is nothing but an `@path` reference, or `None`. +fn line_only_import(line: &str) -> Option<&str> { + let reference = line.strip_prefix('@')?.trim(); + if reference.is_empty() || reference.contains(char::is_whitespace) { + None + } else { + Some(reference) + } +} + +fn resolve_import(from: &PathUri, reference: &str) -> io::Result { + if reference.starts_with('~') || reference.starts_with('/') || reference.contains(':') { + PathUri::from_host_native_path(reference) + } else { + let parent = from + .parent() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "import has no parent"))?; + parent + .join(reference) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err)) + } +} + +async fn is_file( + fs: &dyn ExecutorFileSystem, + path: &PathUri, + sandbox: Option<&FileSystemSandboxContext>, +) -> io::Result { + match fs + .get_metadata(path, GetMetadataOptions::default(), sandbox) + .await + { + Ok(metadata) => Ok(metadata.is_file), + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(false), + Err(err) => Err(err), + } +} + +async fn rule_files( + fs: &dyn ExecutorFileSystem, + rules_dir: &PathUri, + root: &PathUri, + sandbox: Option<&FileSystemSandboxContext>, +) -> io::Result>> { + let canonical = match fs.canonicalize(rules_dir, sandbox).await { + Ok(path) => path, + Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + if !canonical.starts_with(root) { + tracing::warn!(path = %canonical, "refusing .claude/rules outside the project root"); + return Ok(None); + } + let entries = match fs.read_directory(&canonical, sandbox).await { + Ok(entries) => entries, + Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + let mut files = Vec::new(); + for entry in entries { + if entry.is_directory || !entry.file_name.ends_with(".md") { + continue; + } + if let Ok(path) = canonical.join(&entry.file_name) { + files.push(path); + } + } + Ok(Some(files)) +} + +#[cfg(test)] +#[path = "agents_md_compat_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/agents_md_compat_tests.rs b/codex-rs/core/src/agents_md_compat_tests.rs new file mode 100644 index 000000000000..3badf1e300d3 --- /dev/null +++ b/codex-rs/core/src/agents_md_compat_tests.rs @@ -0,0 +1,126 @@ +//! Fork (WI-1132): the `all` project-doc mode. + +use std::fs; + +use codex_exec_server::LOCAL_FS; +use codex_utils_path_uri::PathUri; + +use super::discover_compat_docs; +use super::expand_project_docs; + +fn uri(path: &std::path::Path) -> PathUri { + PathUri::from_host_native_path(path).expect("path uri") +} + +fn write(path: &std::path::Path, contents: &str) { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).expect("parent"); + } + fs::write(path, contents).expect("write"); +} + +/// Every matching filename is kept, and `.claude/rules/*.md` follows in order. +#[tokio::test] +async fn all_mode_keeps_every_doc_and_sorted_rules() { + let dir = tempfile::tempdir().expect("temp dir"); + write(&dir.path().join("AGENTS.md"), "agents\n"); + write(&dir.path().join("CLAUDE.md"), "claude\n"); + write(&dir.path().join(".claude/rules/b.md"), "rule b\n"); + write(&dir.path().join(".claude/rules/a.md"), "rule a\n"); + write(&dir.path().join(".claude/rules/notes.txt"), "not a rule\n"); + let root = uri(dir.path()); + + let found = discover_compat_docs( + LOCAL_FS.as_ref(), + &root, + &["AGENTS.md", "CLAUDE.md"], + &root, + None, + ) + .await + .expect("discover"); + + let names = found + .iter() + .map(|path| path.basename().unwrap_or_default()) + .collect::>(); + assert_eq!(names, ["AGENTS.md", "CLAUDE.md", "a.md", "b.md"]); +} + +/// An import that leaves the project is not read, whether it escapes by an +/// absolute path, `~/`, a `..` climb or a symlink. +#[tokio::test] +async fn imports_that_escape_the_project_root_are_refused() { + let project = tempfile::tempdir().expect("project"); + let outside = tempfile::tempdir().expect("outside"); + write(&outside.path().join("secret.md"), "TOPSECRET\n"); + std::os::unix::fs::symlink( + outside.path().join("secret.md"), + project.path().join("escape.md"), + ) + .expect("symlink"); + let home = tempfile::tempdir().expect("home"); + write(&home.path().join("secret.md"), "TOPSECRET\n"); + // Safety: this test runs alone and only points HOME at a temp dir. + unsafe { std::env::set_var("HOME", home.path()) }; + write( + &project.path().join("CLAUDE.md"), + "@escape.md\n@../secret.md\n@/etc/hostname\n@~/secret.md\n", + ); + let root = uri(project.path()); + + let docs = expand_project_docs( + LOCAL_FS.as_ref(), + &[root.join("CLAUDE.md").expect("join")], + &root, + 4096, + None, + ) + .await + .expect("expand"); + + let text = docs + .iter() + .map(|doc| doc.text.as_str()) + .collect::>() + .join("\n"); + assert!( + !text.contains("TOPSECRET"), + "an import outside the project was read: {text}" + ); + let read = docs.iter().map(|doc| doc.text.len()).sum::(); + let own = fs::read(project.path().join("CLAUDE.md")).unwrap().len(); + assert!( + read <= own, + "read {read} bytes, the project file is only {own}" + ); +} + +/// A relative import that stays inside the project is expanded. +#[tokio::test] +async fn imports_inside_the_project_root_are_expanded() { + let project = tempfile::tempdir().expect("project"); + write(&project.path().join("CLAUDE.md"), "head\n@docs/more.md\n"); + write(&project.path().join("docs/more.md"), "body\n"); + let root = uri(project.path()); + + let docs = expand_project_docs( + LOCAL_FS.as_ref(), + &[root.join("CLAUDE.md").expect("join")], + &root, + 4096, + None, + ) + .await + .expect("expand"); + + let text = docs + .iter() + .map(|doc| doc.text.as_str()) + .collect::>() + .join("\n"); + assert!( + text.contains("body"), + "the in-project import was not read: {text}" + ); +} diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 806bd8082ff8..1dc4a353d6ae 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -439,6 +439,8 @@ enabled = false include_instructions: Some(false), max_context_tokens: std::num::NonZeroUsize::new(1_200), config: Vec::new(), + extra_user_roots: Vec::new(), + extra_project_roots: Vec::new(), }) ); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 1eb2cdf83fe8..ffef5599f45e 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -932,6 +932,15 @@ pub struct Config { /// Additional filenames to try when looking for project-level docs. pub project_doc_fallback_filenames: Vec, + /// Fork (WI-1131): load `~/.claude/CLAUDE.md` and `~/.claude/rules/*.md` + /// into the global user-instructions snapshot. Off unless the config sets it. + pub claude_compat_instructions: bool, + + /// Fork (WI-1132): load every matching project doc in a directory, plus + /// `.claude/rules/*.md`, and expand `@path` imports confined to the project + /// root. Off unless the config sets `project_doc_mode = "all"`. + pub project_doc_all: bool, + /// Token budget applied when storing tool/function outputs in the context manager. pub tool_output_token_limit: Option, @@ -4391,6 +4400,11 @@ impl Config { } }) .collect(), + claude_compat_instructions: cfg.claude_compat_instructions.unwrap_or(false), + project_doc_all: cfg + .project_doc_mode + .as_deref() + .is_some_and(|mode| mode.eq_ignore_ascii_case("all")), tool_output_token_limit: cfg.tool_output_token_limit, agents_enabled, agent_max_threads, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index a2b44e3f1fb6..dbd190343b95 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -171,6 +171,7 @@ pub type NewConversation = NewThread; #[deprecated(note = "use CodexThread")] pub type CodexConversation = CodexThread; pub(crate) mod agents_md; +mod agents_md_compat; mod agents_md_manager; pub use agents_md::DEFAULT_AGENTS_MD_FILENAME; pub use agents_md::LOCAL_AGENTS_MD_FILENAME; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 496114a113a6..a793afea8c49 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1997,6 +1997,8 @@ async fn refresh_runtime_config_refreshes_hooks() -> anyhow::Result<()> { r#async: false, status_message: None, additional_context_limit: None, + // serde default for failClosed: unset means fail open. + fail_closed: false, }], }, }; diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 94079e0ebad4..0efe9c56e7f2 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -134,6 +134,7 @@ async fn handle_spawn_agent( environments: Some(step_context.environments.clone()), multi_agent_v2_usage_hints: None, cyber_access_program: turn.cyber_access_program, + final_output_json_schema: args.output_schema, }, }) .await @@ -224,6 +225,9 @@ struct SpawnAgentArgs { reasoning_effort: Option, #[serde(default)] fork_context: bool, + /// JSON schema the child's first turn must satisfy as its final output. + #[serde(default)] + output_schema: Option, } #[derive(Debug, Serialize)] diff --git a/codex-rs/core/src/tools/handlers/multi_agents_spec.rs b/codex-rs/core/src/tools/handlers/multi_agents_spec.rs index edc9467b4d02..0b895a94579c 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_spec.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_spec.rs @@ -643,6 +643,16 @@ fn spawn_agent_common_properties_v1(agent_type_description: &str) -> BTreeMap BTreeMap, fork_turns: Option, fork_context: Option, + /// JSON schema the child's first turn must satisfy as its final output. + #[serde(default)] + output_schema: Option, } impl SpawnAgentArgs { diff --git a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs index 3a6aa8d96f8d..1dbec8ddfcec 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs @@ -6,6 +6,7 @@ use crate::tools::context::ToolPayload; use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::resolve_tool_environment; +use crate::tools::hook_names::HookToolName; use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; @@ -141,11 +142,30 @@ impl CoreToolRuntime for WriteStdinHandler { matches!(payload, ToolPayload::Function { .. }) } - fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option { - // `write_stdin` is transport for an existing exec session. Empty writes - // are background polls, and non-empty writes continue a command that - // already ran PreToolUse as Bash, so do not emit a second pre hook here. - None + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + let ToolPayload::Function { arguments } = &invocation.payload else { + return None; + }; + // Empty writes are background polls and carry nothing to review. A + // non-empty write is text the model is typing into the open shell, so + // review it as a Bash command before it reaches the process. + let args = parse_arguments::(arguments).ok()?; + if args.chars.is_empty() { + return None; + } + // The directory the shell was started in, not its current directory: a + // `cd` typed into the shell is not tracked. Null when the process is gone. + let workdir = invocation + .session + .services + .unified_exec_manager + .start_cwd_for_process(args.session_id) + .map(|cwd| serde_json::Value::String(cwd.to_string())) + .unwrap_or(serde_json::Value::Null); + Some(PreToolUsePayload { + tool_name: HookToolName::bash(), + tool_input: serde_json::json!({ "command": args.chars, "workdir": workdir }), + }) } fn post_tool_use_payload( diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 61d379bf4d6d..6729a5fd79c6 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -632,9 +632,10 @@ async fn exec_command_pre_tool_use_payload_resolves_workdir() { } #[tokio::test] -async fn exec_command_pre_tool_use_payload_skips_write_stdin() { +async fn write_stdin_pre_tool_use_payload_reviews_nonempty_input_as_bash() { let payload = ToolPayload::Function { - arguments: serde_json::json!({ "chars": "echo hi" }).to_string(), + arguments: serde_json::json!({ "session_id": 1, "chars": "git reset --hard\n" }) + .to_string(), }; let (session, turn) = make_session_and_context().await; let turn = Arc::new(turn); @@ -652,6 +653,34 @@ async fn exec_command_pre_tool_use_payload_skips_write_stdin() { source: crate::tools::context::ToolCallSource::Direct, payload, }), + Some(crate::tools::registry::PreToolUsePayload { + tool_name: HookToolName::bash(), + tool_input: serde_json::json!({ "command": "git reset --hard\n", "workdir": null }), + }) + ); +} + +#[tokio::test] +async fn write_stdin_pre_tool_use_payload_skips_empty_poll() { + let payload = ToolPayload::Function { + arguments: serde_json::json!({ "session_id": 1, "chars": "" }).to_string(), + }; + let (session, turn) = make_session_and_context().await; + let turn = Arc::new(turn); + let handler = WriteStdinHandler; + + assert_eq!( + handler.pre_tool_use_payload(&ToolInvocation { + session: session.into(), + step_context: StepContext::for_test(Arc::clone(&turn)), + turn, + cancellation_token: tokio_util::sync::CancellationToken::new(), + tracker: Arc::new(Mutex::new(TurnDiffTracker::new())), + call_id: "call-45".to_string(), + tool_name: codex_tools::ToolName::plain("write_stdin"), + source: crate::tools::context::ToolCallSource::Direct, + payload, + }), None ); } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 2940235c6a3b..c650702111d1 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -1820,6 +1820,19 @@ impl UnifiedExecProcessManager { .map(|entry| entry.environment_id.clone()) } + /// The directory the exec process was started in. This is the start cwd, + /// not the shell's current directory: a `cd` typed into the shell is not tracked. + /// Returns `None` when the process is gone or the store is busy, so the caller + /// never blocks on the lock. + pub(crate) fn start_cwd_for_process(&self, process_id: i32) -> Option { + self.process_store + .try_lock() + .ok()? + .processes + .get(&process_id) + .map(|entry| entry.cwd.clone()) + } + pub(crate) async fn list_processes(&self) -> Vec { let store = self.process_store.lock().await; let mut entries = store diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index ce56681e68b4..9f4e2ef12fe1 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -678,3 +678,65 @@ async fn pruning_does_not_evict_live_process_while_exited_process_is_finalizing( (None, MAX_UNIFIED_EXEC_PROCESSES) ); } + +#[tokio::test] +async fn start_cwd_for_process_returns_the_spawn_directory() { + let (_, turn) = crate::session::tests::make_session_and_context().await; + let manager = UnifiedExecProcessManager::default(); + let process_id = manager.allocate_process_id().await; + let cwd = PathUri::parse("file:///tmp/other-repo").expect("cwd"); + let process = Arc::new( + crate::unified_exec::process_tests::remote_process( + codex_exec_server::WriteStatus::Accepted, + /*terminate_error*/ None, + codex_sandboxing::SandboxType::None, + ) + .await, + ); + manager.process_store.lock().await.processes.insert( + process_id, + ProcessEntry { + process, + plugin_metrics_sidecar: None, + call_id: "call".to_string(), + process_id, + cwd: cwd.clone(), + initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(false)), + hook_command: "bash".to_string(), + tty: true, + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + permissions: super::super::TerminalPermissions::for_launch( + turn.initial_environments + .primary() + .expect("turn environment"), + &turn, + super::super::TerminalSandboxSource::Native, + crate::sandboxing::SandboxPermissions::UseDefault, + /*additional_permissions*/ None, + /*internal_permissions*/ None, + ), + network_approval: None, + session: std::sync::Weak::new(), + last_used: Instant::now(), + }, + ); + + assert_eq!( + manager.start_cwd_for_process(process_id).as_ref(), + Some(&cwd) + ); + assert_eq!(manager.start_cwd_for_process(process_id + 1), None); +} + +#[tokio::test] +async fn start_cwd_for_process_returns_none_without_blocking_while_locked() { + let manager = UnifiedExecProcessManager::default(); + let _guard = manager.process_store.lock().await; + + let started = Instant::now(); + assert_eq!(manager.start_cwd_for_process(1), None); + assert!( + started.elapsed() < Duration::from_millis(200), + "lookup blocked on the held lock" + ); +} diff --git a/codex-rs/core/tests/suite/openai_file_mcp.rs b/codex-rs/core/tests/suite/openai_file_mcp.rs index 2a18b1c8540d..43dad210a093 100644 --- a/codex-rs/core/tests/suite/openai_file_mcp.rs +++ b/codex-rs/core/tests/suite/openai_file_mcp.rs @@ -44,7 +44,6 @@ use serde_json::json; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; -use wiremock::matchers::body_json; use wiremock::matchers::body_partial_json; use wiremock::matchers::header; use wiremock::matchers::method; diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index 53369836b060..6021db642432 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -39,7 +39,6 @@ use codex_protocol::items::TurnItem; use codex_protocol::models::ImageReference; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::CodeModeToolMessages; -use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ToolMessage; use codex_protocol::openai_models::ToolMode; use codex_protocol::protocol::EnvironmentConfigState; diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_active_environment_selection.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_active_environment_selection.snap index 2c899e4a82ba..49a7a1a1ec1d 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_active_environment_selection.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_active_environment_selection.snap @@ -6,7 +6,7 @@ Scenario: Astra starts in one workspace, switches for the next step of the activ ## Window 1 -- request 1 (turn) -- -00:additional_tools/developer (3; hash=62160ed28e692c35): +00:additional_tools/developer (3; hash=299a25860327c501): - namespace/functions; hash=4261ffb241688b3d - custom/exec - function/wait @@ -14,7 +14,7 @@ Scenario: Astra starts in one workspace, switches for the next step of the activ - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_async_question_and_answer.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_async_question_and_answer.snap index 815d076e4f2f..50ee5c00eeab 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_async_question_and_answer.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_async_question_and_answer.snap @@ -7,7 +7,7 @@ Scenario: Astra uses the catalog question schema to ask who a launch update is f ## Window 1 -- request 1 (request) -- -00:additional_tools/developer (3; hash=f181967456befa67): +00:additional_tools/developer (3; hash=4bdada565c53ed1b): - namespace/functions; hash=7e611b8ddd5621cf - custom/exec - function/wait @@ -15,7 +15,7 @@ Scenario: Astra uses the catalog question schema to ask who a launch update is f - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_code_mode_call_timing.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_code_mode_call_timing.snap index 63afb9950a21..bc483984d11b 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_code_mode_call_timing.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_code_mode_call_timing.snap @@ -6,7 +6,7 @@ Scenario: Astra receives host and handler timings on completed and failed code-m ## Window 1 -- request 1 (turn) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -14,7 +14,7 @@ Scenario: Astra receives host and handler timings on completed and failed code-m - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_disabled_executor_skills.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_disabled_executor_skills.snap index ce4690a6d0d3..3063cf6ad2bb 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_disabled_executor_skills.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_disabled_executor_skills.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (request) -- -00:additional_tools/developer (3; hash=5e8370540461db70): +00:additional_tools/developer (3; hash=832399eb45cc415c): - namespace/functions; hash=7d26e086c1dbba28 - custom/exec - function/wait @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_interrupts_response.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_interrupts_response.snap index b81315dda0b1..41ce44f825a4 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_interrupts_response.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_interrupts_response.snap @@ -6,7 +6,7 @@ Scenario: Astra receives new user input after its unfinished response is interru ## Window 1 -- request 1 (request) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -14,7 +14,7 @@ Scenario: Astra receives new user input after its unfinished response is interru - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_yields_code_mode_cell.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_yields_code_mode_cell.snap index cb8ed17bf835..bd8688b12bfd 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_yields_code_mode_cell.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_input_yields_code_mode_cell.snap @@ -6,7 +6,7 @@ Scenario: Astra continues after new user input yields a running code cell, then ## Window 1 -- request 1 (turn) -- -00:additional_tools/developer (3; hash=69c8f596545517aa): +00:additional_tools/developer (3; hash=f905819a4ad6047e): - namespace/functions; hash=13a36c9bc9ada856 - custom/exec - function/wait @@ -14,7 +14,7 @@ Scenario: Astra continues after new user input yields a running code cell, then - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_kickoff_remote_compaction_windows.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_kickoff_remote_compaction_windows.snap index 29a6122a3924..70db6d67b7c3 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_kickoff_remote_compaction_windows.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_kickoff_remote_compaction_windows.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (turn) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents @@ -187,7 +187,7 @@ Settings: ## Window 2 (after request 3: input diverged at item 02) Settings: same as window 1 -- request 4 (turn) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -195,7 +195,7 @@ Settings: same as window 1 - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_plugin_refresh.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_plugin_refresh.snap index 5f76c13704db..e3170ce7d9ac 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_plugin_refresh.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_plugin_refresh.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (turn) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_settings_release_check_tool_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_settings_release_check_tool_shapes.snap index ed6a215e264b..07ddab2823d5 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_settings_release_check_tool_shapes.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_settings_release_check_tool_shapes.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (turn) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages.snap index bcf207e89d85..401accdb3215 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages.snap @@ -15,14 +15,14 @@ Settings: stream: true text: {"verbosity":"low"} tool_choice: "auto" - tools (7; hash=7916dd640b46ec6e): + tools (7; hash=cbfcb9f48729810a): - custom/exec: Catalog exec instructions. Default yield: 1234 ms. `image(imageUrlOrItem: string | { image_url: s...; hash=a49b91fd82908e18 - function/wait: Catalog wait instructions: resume the yielded cell.; args=[cell_id, max_tokens, terminate, yield_time_ms]; hash=67bc0f04da3505b3 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - function/request_user_input_async: Ask the user one or more questions during ongoing work. Use this tool only to request missing inf...; args=[questions]; hash=d6d74f33db330162 - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages_ranked_search.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages_ranked_search.snap index 5c669886321f..7de726034fb9 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages_ranked_search.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_catalog_messages_ranked_search.snap @@ -15,14 +15,14 @@ Settings: stream: true text: {"verbosity":"low"} tool_choice: "auto" - tools (7; hash=40e072f613a5e3eb): + tools (7; hash=89b8f928161a14ef): - custom/exec: Catalog exec instructions. Default yield: 1234 ms. `image(imageUrlOrItem: string | { image_url: s...; hash=1b186fc5b7e14af3 - function/wait: Catalog wait instructions: resume the yielded cell.; args=[cell_id, max_tokens, terminate, yield_time_ms]; hash=67bc0f04da3505b3 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - function/request_user_input_async: Ask the user one or more questions during ongoing work. Use this tool only to request missing inf...; args=[questions]; hash=d6d74f33db330162 - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_mcp_schema_limits.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_mcp_schema_limits.snap index 5f527de1474a..e43e807a79c7 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_mcp_schema_limits.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_mcp_schema_limits.snap @@ -15,8 +15,8 @@ Settings: stream: true text: {"verbosity":"low"} tool_choice: "auto" - tools (4; hash=95b65d00fe586b7d): - - custom/exec: Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in...; hash=e02c9856c9cd8430 + tools (4; hash=2231df5ae7e36647): + - custom/exec: Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in...; hash=36e8a7ff542e3082 - function/wait: Waits on a yielded `exec` cell and returns new output or completion. - Use `wait` only after `exe...; args=[cell_id, max_tokens, terminate, yield_time_ms]; hash=6692d8f6c55be489 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - web_search; hash=e214d4db1bc49e89 diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_settled_helpers__code_mode_settled_helpers.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_settled_helpers__code_mode_settled_helpers.snap index f72b39e0b51a..f04e8011313e 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_settled_helpers__code_mode_settled_helpers.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_settled_helpers__code_mode_settled_helpers.snap @@ -6,7 +6,7 @@ Scenario: Settlement helpers deliver fulfilled and rejected records with input i ## Window 1 -- request 1 (turn) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -14,7 +14,7 @@ Scenario: Settlement helpers deliver fulfilled and rejected records with input i - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__compaction__remote_slash_compact.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__compaction__remote_slash_compact.snap index 4e78cadff514..c6aaccfa8e05 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__compaction__remote_slash_compact.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__compaction__remote_slash_compact.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (turn; Before /compact) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents @@ -164,7 +164,7 @@ Settings: unavailable (items only) ## Window 3 (after request 3: input diverged at item 00) Settings: same as window 1 -- request 4 (turn; First turn after /compact) -- -00:additional_tools/developer (3; hash=bf2e30605ff5dc17): +00:additional_tools/developer (3; hash=82ca56c2b440b56b): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -172,7 +172,7 @@ Settings: same as window 1 - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap index a57a69ff96c6..52d07eb9da40 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (turn) -- -00:additional_tools/developer (3; hash=7748a53142f449b7): +00:additional_tools/developer (3; hash=f37fcca6131a95d3): - namespace/functions; hash=124fee69a69ed6b2 - function/exec_command - function/write_stdin @@ -26,7 +26,7 @@ Settings: - function/request_user_input - custom/apply_patch - function/view_image - - namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=2327142bda177c50 + - namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=3d94ea231ad8147c - function/close_agent - function/resume_agent - function/send_input diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__indirect_namespace_prefixes__catalog_indirect_namespace_prefixes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__indirect_namespace_prefixes__catalog_indirect_namespace_prefixes.snap index c7761cf2936c..de829565f836 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__indirect_namespace_prefixes__catalog_indirect_namespace_prefixes.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__indirect_namespace_prefixes__catalog_indirect_namespace_prefixes.snap @@ -15,8 +15,8 @@ Settings: stream: true text: {"verbosity":"low"} tool_choice: "auto" - tools (4; hash=7707adf4d5cde816): - - custom/exec: Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in...; hash=66e0c24dd8b9010d + tools (4; hash=928fbf82e0d338d0): + - custom/exec: Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in...; hash=082164ed597e87db - function/wait: Waits on a yielded `exec` cell and returns new output or completion. - Use `wait` only after `exe...; args=[cell_id, max_tokens, terminate, yield_time_ms]; hash=6692d8f6c55be489 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - web_search; hash=e214d4db1bc49e89 @@ -49,8 +49,8 @@ Settings: ## Window 2 (after request 2) Settings: relative to window 1 model: "prefixes-cleared" - tools (4; hash=d878b195101dccae): - ~ custom/exec: Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in...; hash=881b443f5068d3fb + tools (4; hash=a67435a0067d399c): + ~ custom/exec: Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in...; hash=285b60ac075c51c9 -- request 3 (turn) -- 00:message/developer: diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__mcp_resource_messages__mcp_resource_messages.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__mcp_resource_messages__mcp_resource_messages.snap index 2ec353f71daa..7c00009b0835 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__mcp_resource_messages__mcp_resource_messages.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__mcp_resource_messages__mcp_resource_messages.snap @@ -15,14 +15,14 @@ Settings: stream: true text: {"verbosity":"low"} tool_choice: "auto" - tools (7; hash=dac6a853a6177003): + tools (7; hash=a1883c6617990737): - custom/exec: Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in...; hash=9ad48dbb5eac1f9b - function/wait: Waits on a yielded `exec` cell and returns new output or completion. - Use `wait` only after `exe...; args=[cell_id, max_tokens, terminate, yield_time_ms]; hash=6692d8f6c55be489 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - function/request_user_input_async: Ask the user one or more questions during ongoing work. Use this tool only to request missing inf...; args=[questions]; hash=d6d74f33db330162 - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__model_catalog_refresh_preserves_tools_and_history.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__model_catalog_refresh_preserves_tools_and_history.snap index e7d2ffa51ee7..1df05913d656 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__model_catalog_refresh_preserves_tools_and_history.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__model_catalog_refresh_preserves_tools_and_history.snap @@ -14,12 +14,12 @@ Settings: store: false stream: true tool_choice: "auto" - tools (6; hash=1ea0fa9abde186d4): + tools (6; hash=cfc3c98a78e70ca0): - function/exec_command: Runs a command in a PTY, returning output or a session ID for ongoing interaction.; args=[cmd, justification, login, max_output_tokens, prefix_rule, sandbox_permissions, shell, tty, workdir, yield_time_ms]; hash=c04108e51a8548c5 - function/write_stdin: Writes characters to an existing unified exec session and returns recent output.; args=[chars, max_output_tokens, session_id, yield_time_ms]; hash=7fb62d8a3e2df4e2 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[path]; hash=1908de9c0be7118a - - namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=94c598b3c6ca141b + - namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=3600df73976fc1a7 - function/close_agent - function/resume_agent - function/send_input diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__multi_agent_catalog_parameters.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__multi_agent_catalog_parameters.snap index 5fb3ec889e96..3426e88f57de 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__multi_agent_catalog_parameters.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__multi_agent_catalog_parameters.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (turn) -- -00:additional_tools/developer (3; hash=2b84850bc279e5ed): +00:additional_tools/developer (3; hash=29ec89a35debc3c9): - namespace/functions; hash=fada4a476a3754dd - custom/exec - function/wait @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=7e156cf57f85af87 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=c9dc64d781a79893 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__provider_capabilities__custom_provider_capabilities.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__provider_capabilities__custom_provider_capabilities.snap index ade1be5bcfe3..f5897fd28e24 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__provider_capabilities__custom_provider_capabilities.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__provider_capabilities__custom_provider_capabilities.snap @@ -15,12 +15,12 @@ Settings: stream: true text: {"verbosity":"low"} tool_choice: "auto" - tools (6; hash=dde875513b2c7cff): + tools (6; hash=7034b0984ef6658b): - function/exec_command: Runs a command in a PTY, returning output or a session ID for ongoing interaction.; args=[cmd, justification, login, max_output_tokens, prefix_rule, sandbox_permissions, shell, tty, workdir, yield_time_ms]; hash=c04108e51a8548c5 - function/write_stdin: Writes characters to an existing unified exec session and returns recent output.; args=[chars, max_output_tokens, session_id, yield_time_ms]; hash=7fb62d8a3e2df4e2 - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 - function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[detail, path]; hash=3b766d5bdec5d50e - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=3cbf715dc154e1b7 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=9cb3fe3bf82d4343 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__dedup_before_budgeting.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__dedup_before_budgeting.snap index 3dcc2d2c8ab8..050b4509ac42 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__dedup_before_budgeting.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__dedup_before_budgeting.snap @@ -16,7 +16,7 @@ Settings: text: {"verbosity":"low"} tool_choice: "auto" -- request 1 (turn) -- -00:additional_tools/developer (3; hash=e80da11342c6e6dd): +00:additional_tools/developer (3; hash=11140157658710d9): - namespace/functions; hash=bd9e182b4235cdc1 - custom/exec - function/wait @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap index a4dc574b7dce..27c72b5eb330 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__strict_3p_cache__strict_3p_mcp_catalog_cache.snap @@ -24,7 +24,7 @@ Settings: - function/request_user_input_async - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 - function/sleep - - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=2c8d6295978dbe63 + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=48f7a334813023a7 - function/followup_task - function/interrupt_agent - function/list_agents diff --git a/codex-rs/ext/skills/src/host_roots.rs b/codex-rs/ext/skills/src/host_roots.rs index ac2bf3e9cdf8..fb960d0f164b 100644 --- a/codex-rs/ext/skills/src/host_roots.rs +++ b/codex-rs/ext/skills/src/host_roots.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; +use codex_config::SkillsConfig; use codex_config::default_project_root_markers; use codex_config::merge_toml_values; use codex_config::project_root_markers_from_config; @@ -23,6 +24,40 @@ use crate::loader::HostSkillRoot; const AGENTS_DIR_NAME: &str = ".agents"; const SKILLS_DIR_NAME: &str = "skills"; + +/// The extra roots configured under `[skills]`, or none when the config is +/// absent or invalid. An invalid table warns and changes nothing. +fn extra_skill_roots(config_layer_stack: &ConfigLayerStack, project: bool) -> Vec { + let effective = config_layer_stack.effective_config(); + let Some(value) = effective.as_table().and_then(|table| table.get("skills")) else { + return Vec::new(); + }; + let skills: SkillsConfig = match value.clone().try_into() { + Ok(skills) => skills, + Err(err) => { + tracing::warn!("invalid skills config: {err}"); + return Vec::new(); + } + }; + if project { + skills.extra_project_roots + } else { + skills.extra_user_roots + } +} + +/// Joins `relative` onto `base` and keeps it only when it stays inside `base`. +/// +/// An absolute value, a `..` climb or anything else that resolves outside the +/// base is dropped, so a config cannot point skill loading at arbitrary paths. +fn confined_join(base: &AbsolutePathBuf, relative: &str) -> Option { + let relative = relative.trim().trim_start_matches(['/', '\\']); + if relative.is_empty() { + return None; + } + let joined = base.join(relative); + joined.starts_with(base).then_some(joined) +} const MAX_CONCURRENT_ANCESTOR_PROBES: usize = 256; pub(crate) async fn resolve_skill_roots( @@ -99,6 +134,11 @@ fn roots_from_layer_stack( config_folder.join(SKILLS_DIR_NAME), SkillScope::User, )); + for relative in extra_skill_roots(config_layer_stack, /*project*/ false) { + if let Some(path) = confined_join(&config_folder, &relative) { + roots.push(local_root(path, SkillScope::User)); + } + } if let Some(home_dir) = home_dir { roots.push(local_root( @@ -181,6 +221,25 @@ async fn repo_agents_skill_roots( } } } + for directory in dirs_between_project_root_and_cwd(cwd, &project_root) { + for relative in extra_skill_roots(config_layer_stack, /*project*/ true) { + let Some(path) = confined_join(&directory, &relative) else { + continue; + }; + let uri = PathUri::from_abs_path(&path); + if let Ok(metadata) = repository_file_system + .get_metadata(&uri, GetMetadataOptions::default(), /*sandbox*/ None) + .await + && metadata.is_directory + { + roots.push(HostSkillRoot::host( + path, + SkillScope::Repo, + Arc::clone(&repository_file_system), + )); + } + } + } roots } diff --git a/codex-rs/ext/skills/src/host_roots_tests.rs b/codex-rs/ext/skills/src/host_roots_tests.rs index fa8cb7d8dc2a..4584f15687d1 100644 --- a/codex-rs/ext/skills/src/host_roots_tests.rs +++ b/codex-rs/ext/skills/src/host_roots_tests.rs @@ -406,7 +406,22 @@ async fn repo_ancestry_without_project_marker_does_not_walk_parents() { fs::create_dir_all(outer.join(".agents/skills")).expect("create outer skills"); fs::create_dir_all(cwd.join(".agents/skills")).expect("create cwd skills"); - let roots = repo_agents_skill_roots(Some(Arc::clone(&LOCAL_FS)), &stack(Vec::new()), &cwd) + // Hermetic: disable project-root marker discovery so the test does not + // depend on ancestor directories of the temp dir (e.g. a real .git). + let mut user_config = empty_config(); + user_config.as_table_mut().expect("config table").insert( + "project_root_markers".into(), + toml::Value::Array(Vec::new()), + ); + let config_stack = stack(vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: absolute(temp_dir.path().join("home/codex/config.toml")), + profile: None, + }, + user_config, + )]); + + let roots = repo_agents_skill_roots(Some(Arc::clone(&LOCAL_FS)), &config_stack, &cwd) .await .into_iter() .map(|root| root.path) @@ -676,3 +691,72 @@ async fn resolved_config_and_repo_roots_preserve_order_and_dedupe_paths_not_name ] ); } + +/// Fork (WI-1133): `[skills].extra_project_roots` adds a root without dropping +/// `.agents/skills`, and a value that climbs out of the directory is ignored. +#[tokio::test] +async fn extra_project_roots_are_added_and_confined() { + let temp_dir = TempDir::new().expect("temp dir"); + let repository = absolute(temp_dir.path().join("repo")); + fs::create_dir_all(&repository).expect("create repo"); + fs::write(repository.join(".git"), "gitdir: fake\n").expect("write git marker"); + fs::create_dir_all(repository.join(".agents/skills")).expect("built-in root"); + fs::create_dir_all(repository.join(".claude/skills")).expect("extra root"); + + let config = toml::toml! { + [skills] + extra_project_roots = [".claude/skills", "../outside"] + }; + let config_stack = stack(vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: repository.join("config.toml"), + profile: None, + }, + toml::Value::Table(config), + )]); + + let roots = repo_agents_skill_roots(Some(Arc::clone(&LOCAL_FS)), &config_stack, &repository) + .await + .into_iter() + .map(|root| root.path) + .collect::>(); + + assert!(roots.contains(&repository.join(".agents/skills"))); + assert!(roots.contains(&repository.join(".claude/skills"))); + assert!( + roots.iter().all(|root| root.starts_with(&repository)), + "a root escaped the project: {roots:?}" + ); +} + +/// Fork (WI-1133): `[skills].extra_user_roots` is added beside the built-in +/// user roots, and a value that leaves the config directory is dropped. +#[test] +fn extra_user_roots_are_added_and_confined() { + let temp_dir = TempDir::new().expect("temp dir"); + let home_folder = absolute(temp_dir.path().join("home")); + let user_folder = home_folder.join("codex"); + let config = toml::toml! { + [skills] + extra_user_roots = [".claude/skills", "../escaped"] + }; + let config_stack = stack(vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: user_folder.join("config.toml"), + profile: None, + }, + toml::Value::Table(config), + )]); + + let roots = roots_from_layer_stack(&config_stack, Some(&home_folder), None) + .into_iter() + .map(|root| root.path) + .collect::>(); + + assert!(roots.contains(&user_folder.join("skills"))); + assert!(roots.contains(&user_folder.join(".claude/skills"))); + assert!( + !roots.iter().any(|root| root.ends_with("escaped")), + "a root escaped the config directory: {roots:?}" + ); +} diff --git a/codex-rs/hooks/src/declarations.rs b/codex-rs/hooks/src/declarations.rs index 42ca76a2ed02..4b9a28ae1236 100644 --- a/codex-rs/hooks/src/declarations.rs +++ b/codex-rs/hooks/src/declarations.rs @@ -70,6 +70,7 @@ mod tests { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }, ], }], diff --git a/codex-rs/hooks/src/engine/command_runner_tests.rs b/codex-rs/hooks/src/engine/command_runner_tests.rs index a7b90a8c0ed1..d75f7264b4a2 100644 --- a/codex-rs/hooks/src/engine/command_runner_tests.rs +++ b/codex-rs/hooks/src/engine/command_runner_tests.rs @@ -117,6 +117,7 @@ async fn hook_shell_startup_does_not_stop_on_controlling_terminal() { timeout_sec: 2, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: AbsolutePathBuf::try_from(temp.path().join("hooks.json")) .expect("absolute hook configuration path") .into(), @@ -221,6 +222,7 @@ async fn cmd_shell_runs_quoted_hook_command_path() { timeout_sec: 10, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: source_path.into(), source: HookSource::User, display_order: 0, @@ -271,6 +273,7 @@ async fn fast_exiting_hook_preserves_stdout_when_stdin_is_not_consumed() { timeout_sec: 10, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: source_path.into(), source: HookSource::User, display_order: 0, @@ -359,6 +362,7 @@ async fn command_hook_does_not_expose_configured_noise_auth_token() { timeout_sec: 10, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: source_path.into(), source: HookSource::User, display_order: 0, @@ -497,6 +501,7 @@ fn write_handler(temp: &TempDir, source: &str) -> ConfiguredHandler { timeout_sec: 10, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: AbsolutePathBuf::try_from(temp.path().join("hooks.json")) .expect("absolute test hook path") .into(), diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 0c5a28ef35ef..a5a8709a9d4a 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -77,6 +77,7 @@ struct NormalizedHandler { timeout_sec: u64, status_message: Option, additional_context_limit: Option, + fail_closed: bool, } #[derive(Clone, Copy)] @@ -511,6 +512,7 @@ fn append_matcher_groups( r#async, status_message, additional_context_limit, + fail_closed, } => { let command = if cfg!(windows) { command_windows.unwrap_or(command) @@ -538,6 +540,19 @@ fn append_matcher_groups( source.path.display() )); } + // `failClosed` only takes effect on a synchronous handler, so an + // async handler that asks for it runs synchronously. Skipping it + // would let the tool run with no guard at all. + let runs_async = if runs_async && fail_closed { + warnings.push(format!( + "running async {} hook synchronously in {} because failClosed is set", + hook_event_name_label(event_name), + source.path.display() + )); + false + } else { + runs_async + }; let additional_context_limit = if matches!( event_name, codex_protocol::protocol::HookEventName::PreToolUse @@ -565,6 +580,7 @@ fn append_matcher_groups( r#async, status_message: status_message.clone(), additional_context_limit: normalized_additional_context_limit, + fail_closed, }; let command = source.env.iter().fold(command, |command, (key, value)| { command.replace(&format!("${{{key}}}"), value) @@ -579,6 +595,7 @@ fn append_matcher_groups( timeout_sec, status_message, additional_context_limit, + fail_closed, } } HookHandlerConfig::McpTool { @@ -632,6 +649,7 @@ fn append_matcher_groups( timeout_sec, status_message, additional_context_limit: None, + fail_closed: false, } } HookHandlerConfig::Prompt {} => { @@ -662,6 +680,7 @@ fn append_matcher_groups( timeout_sec, status_message, additional_context_limit, + fail_closed, } = normalized; let current_hash = hook_hash(event_name, matcher, &group, &config); let key = crate::hook_key(&source.key_source, event_name, group_index, handler_index); @@ -728,6 +747,7 @@ fn append_matcher_groups( additional_context_limit: AdditionalContextLimit::from_config( additional_context_limit, ), + fail_closed, source_path: source.path.clone().into(), source: source.source, display_order: *display_order, @@ -979,6 +999,7 @@ mod tests { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], } } @@ -995,6 +1016,7 @@ mod tests { r#async: false, status_message: None, additional_context_limit: Some(additional_context_limit), + fail_closed: false, }], } } @@ -1263,6 +1285,7 @@ mod tests { timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: source_path.clone().into(), source: hook_source(), display_order: 0, @@ -1303,6 +1326,7 @@ mod tests { timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: source_path.clone().into(), source: hook_source(), display_order: 0, @@ -1341,6 +1365,7 @@ mod tests { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }, HookHandlerConfig::Command { command: "echo clamped".to_string(), @@ -1349,6 +1374,7 @@ mod tests { r#async: true, status_message: None, additional_context_limit: None, + fail_closed: false, }, ], }], @@ -1431,6 +1457,7 @@ mod tests { r#async: true, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ); @@ -1475,6 +1502,45 @@ mod tests { ); } + #[test] + fn fail_closed_forces_an_async_handler_to_run_synchronously() { + let source_path = source_path(); + let hook_states = std::collections::HashMap::new(); + let mut handlers = Vec::new(); + let mut warnings = Vec::new(); + + append_matcher_groups( + &mut handlers, + &mut Vec::new(), + &mut warnings, + &mut 0, + &mut hook_handler_source(&source_path, &hook_states), + HookEventName::PreToolUse, + vec![MatcherGroup { + matcher: Some("Bash".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "echo guard".to_string(), + command_windows: None, + timeout_sec: None, + r#async: true, + status_message: None, + additional_context_limit: None, + fail_closed: true, + }], + }], + ); + + assert_eq!(handlers.len(), 1); + assert!(handlers[0].fail_closed); + assert_eq!(handlers[0].execution_mode(), HookExecutionMode::Sync); + assert!( + warnings + .iter() + .any(|warning| warning.contains("failClosed")), + "expected a warning about the async/failClosed combination, got {warnings:?}" + ); + } + #[test] fn bypass_hook_trust_allows_enabled_untrusted_handlers() { let mut handlers = Vec::new(); @@ -1621,6 +1687,7 @@ mod tests { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -1652,6 +1719,7 @@ mod tests { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ); diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index b34118d6f2b8..f90f8d179eac 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -357,6 +357,7 @@ mod tests { timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: HookSource::User, display_order, diff --git a/codex-rs/hooks/src/engine/mcp_runner_tests.rs b/codex-rs/hooks/src/engine/mcp_runner_tests.rs index a16613ea071d..297013a6a3c5 100644 --- a/codex-rs/hooks/src/engine/mcp_runner_tests.rs +++ b/codex-rs/hooks/src/engine/mcp_runner_tests.rs @@ -97,6 +97,7 @@ async fn mcp_tool_results_use_command_hook_output_contract() { timeout_sec: 30, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: HookSource::User, display_order: 0, diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index a007f3b1a459..e71de4bf1961 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -67,6 +67,8 @@ pub(crate) struct ConfiguredHandler { pub timeout_sec: u64, pub status_message: Option, pub additional_context_limit: AdditionalContextLimit, + /// Opt-in: a failed run of this handler blocks the tool instead of failing open. + pub fail_closed: bool, pub source_path: HandlerSourcePath, pub source: HookSource, pub display_order: i64, @@ -327,6 +329,7 @@ impl ClaudeHooksEngine { timeout_sec: timeout_sec.unwrap_or(5).max(1), status_message, additional_context_limit: Default::default(), + fail_closed: false, source_path: HandlerSourcePath::ExecutorScoped { plugin_id: source.plugin_id.clone(), environment_id: source.environment_id.clone(), diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index a185e45029ff..28416ad132f9 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -96,6 +96,7 @@ fn permission_request_timeout_only_counts_synchronous_handlers() { timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: cwd().join("hooks.json").into(), source: HookSource::User, display_order: 0, @@ -156,6 +157,7 @@ fn pre_tool_use_hook_events(command: impl Into) -> HookEventsToml { r#async: false, status_message: Some("checking".to_string()), additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -584,6 +586,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: r#async: false, status_message: Some("checking".to_string()), additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -692,6 +695,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() { r#async: false, status_message: Some("checking".to_string()), additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -773,6 +777,7 @@ fn unknown_requirement_source_hooks_stay_managed() { r#async: false, status_message: Some("checking".to_string()), additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -843,6 +848,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() { r#async: false, status_message: Some("checking".to_string()), additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -1087,6 +1093,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() { r#async: false, status_message: Some("checking".to_string()), additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -1812,6 +1819,7 @@ fn bundled_cleanup_trust_does_not_extend_to_other_handlers() { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }, ]); source.hooks.stop.push(MatcherGroup { @@ -1974,6 +1982,7 @@ fn disabled_hooks_feature_keeps_builtin_cleanup_but_not_trusted_plugin_hooks() { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }); let stack = trusted_plugin_hook_stack(cwd().join("config.toml"), &[source.clone()]); let discovered = super::discovery::discover_handlers( @@ -2039,6 +2048,7 @@ print(json.dumps({ r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -2160,6 +2170,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() { r#async: false, status_message: None, additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() @@ -2318,6 +2329,7 @@ fn executor_stop_hook_fixture() -> ( timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: HandlerSourcePath::ExecutorScoped { plugin_id: PluginId::parse("computer-use@openai-bundled").expect("valid plugin ID"), environment_id: "executor-a".to_string(), @@ -2407,6 +2419,7 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() { timeout_sec: 30, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: cwd().join("hooks.json").into(), source: HookSource::User, display_order: 0, @@ -2442,6 +2455,7 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() { timeout_sec: 30, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: cwd().join("hooks.json").into(), source: HookSource::User, display_order: 1, diff --git a/codex-rs/hooks/src/events/compact.rs b/codex-rs/hooks/src/events/compact.rs index 82bd6c6cf55d..0a123b933816 100644 --- a/codex-rs/hooks/src/events/compact.rs +++ b/codex-rs/hooks/src/events/compact.rs @@ -529,6 +529,7 @@ mod tests { timeout_sec: 5, status_message: Some("running compact hook".to_string()), additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: codex_protocol::protocol::HookSource::User, display_order: 0, diff --git a/codex-rs/hooks/src/events/interrupt_tests.rs b/codex-rs/hooks/src/events/interrupt_tests.rs index 3d8642c8457b..dc255e03b282 100644 --- a/codex-rs/hooks/src/events/interrupt_tests.rs +++ b/codex-rs/hooks/src/events/interrupt_tests.rs @@ -104,6 +104,7 @@ fn handler() -> ConfiguredHandler { timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: codex_protocol::protocol::HookSource::User, display_order: 0, diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 0ca0f6c673d8..69ccffc22203 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -595,6 +595,7 @@ mod tests { timeout_sec: 5, status_message: Some("running post tool use hook".to_string()), additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: codex_protocol::protocol::HookSource::User, display_order: 0, diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index e57b823a1c86..1c57b192bbf4 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -292,6 +292,24 @@ fn parse_completed( }, } + // `failClosed` turns every failure into a block the model sees. A handler + // without the flag keeps failing open, so the tool still runs. + if handler.fail_closed && handler.can_apply_control_effects() && status == HookRunStatus::Failed + { + status = HookRunStatus::Blocked; + should_block = true; + let reason = entries + .iter() + .find(|entry| entry.kind == HookOutputEntryKind::Error) + .map(|entry| format!("hook failed closed: {}", entry.text)) + .unwrap_or_else(|| "hook failed closed".to_string()); + block_reason = Some(reason.clone()); + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Feedback, + text: reason, + }); + } + let completed = HookCompletedEvent { turn_id, run: dispatcher::completed_summary(handler, &run_result, status, entries), @@ -332,6 +350,7 @@ mod tests { use super::PreToolUseHandlerData; use super::command_input_json; + use super::dispatcher; use super::latest_updated_input; use super::parse_completed; use super::preview; @@ -779,6 +798,7 @@ mod tests { timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: codex_protocol::protocol::HookSource::User, display_order: 0, @@ -802,6 +822,137 @@ mod tests { } } + fn run_result_error(error: &str) -> HandlerRunResult { + HandlerRunResult { + started_at: 1, + completed_at: 2, + duration_ms: 1, + exit_code: None, + stdout: String::new(), + stderr: String::new(), + error: Some(error.to_string()), + } + } + + fn assert_failed_open(parsed: &dispatcher::ParsedHandler) { + assert!(!parsed.data.should_block); + assert_eq!(parsed.data.block_reason, None); + assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); + } + + fn assert_failed_closed( + parsed: &dispatcher::ParsedHandler, + reason_contains: &str, + ) { + assert!(parsed.data.should_block); + let reason = parsed.data.block_reason.as_deref().unwrap_or_default(); + assert!( + reason.contains(reason_contains), + "block reason {reason:?} should mention {reason_contains:?}" + ); + assert!(reason.starts_with("hook failed closed: ")); + assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); + assert!( + parsed + .completed + .run + .entries + .iter() + .any(|entry| entry.kind == HookOutputEntryKind::Feedback + && entry.text.contains(reason_contains)) + ); + } + + #[test] + fn fail_closed_blocks_nonzero_exit_other_than_two() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed(&handler, run_result(Some(1), "", "boom"), None); + assert_failed_closed(&parsed, "hook exited with code 1"); + } + + #[test] + fn fail_closed_blocks_exit_two_without_a_reason() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed(&handler, run_result(Some(2), "", " "), None); + assert_failed_closed(&parsed, "did not write a blocking reason"); + } + + #[test] + fn fail_closed_blocks_missing_status_code() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed(&handler, run_result(None, "", ""), None); + assert_failed_closed(&parsed, "exited without a status code"); + } + + #[test] + fn fail_closed_blocks_spawn_failure() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed( + &handler, + run_result_error("No such file or directory (os error 2)"), + None, + ); + assert_failed_closed(&parsed, "No such file or directory"); + } + + #[test] + fn fail_closed_blocks_timeout() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed(&handler, run_result_error("hook timed out after 5s"), None); + assert_failed_closed(&parsed, "timed out"); + } + + #[test] + fn fail_closed_blocks_crash_or_signal() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed( + &handler, + run_result_error("wait_error: process terminated by signal"), + None, + ); + assert_failed_closed(&parsed, "terminated by signal"); + } + + #[test] + fn fail_closed_blocks_invalid_json() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed(&handler, run_result(Some(0), "{not json", ""), None); + assert_failed_closed(&parsed, "invalid pre-tool-use JSON"); + } + + #[test] + fn fail_closed_blocks_unreadable_command_output() { + let mut handler = handler(); + handler.fail_closed = true; + let parsed = parse_completed( + &handler, + run_result_error("stream did not contain valid UTF-8"), + None, + ); + assert_failed_closed(&parsed, "valid UTF-8"); + } + + #[test] + fn handler_without_fail_closed_still_fails_open() { + for result in [ + run_result(Some(1), "", ""), + run_result(Some(2), "", ""), + run_result(None, "", ""), + run_result(Some(0), "{not json", ""), + run_result_error("No such file or directory (os error 2)"), + run_result_error("hook timed out after 5s"), + ] { + assert_failed_open(&parse_completed(&handler(), result, None)); + } + } + fn request_for_tool_use(tool_use_id: &str) -> super::PreToolUseRequest { super::PreToolUseRequest { session_id: ThreadId::new(), diff --git a/codex-rs/hooks/src/events/session_end_tests.rs b/codex-rs/hooks/src/events/session_end_tests.rs index 05fa107d4fa9..dc6c378b3f8a 100644 --- a/codex-rs/hooks/src/events/session_end_tests.rs +++ b/codex-rs/hooks/src/events/session_end_tests.rs @@ -67,6 +67,7 @@ fn handler(matcher: Option<&str>) -> ConfiguredHandler { timeout_sec: 2, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: HookSource::User, display_order: 0, diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index 3aff0fe946eb..95bc42cf274c 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -548,6 +548,7 @@ mod tests { timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: codex_protocol::protocol::HookSource::User, display_order: 0, diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index be2be9100e35..32802fd9705b 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -698,6 +698,7 @@ mod tests { timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: codex_protocol::protocol::HookSource::User, display_order: 0, diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index 9124431bc194..65a9f252f3c2 100644 --- a/codex-rs/hooks/src/events/user_prompt_submit.rs +++ b/codex-rs/hooks/src/events/user_prompt_submit.rs @@ -467,6 +467,7 @@ mod tests { timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), + fail_closed: false, source_path: test_path_buf("/tmp/hooks.json").abs().into(), source: codex_protocol::protocol::HookSource::User, display_order: 0, diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index a70b8c9817f9..0511a5dc18eb 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -323,6 +323,8 @@ async fn new_config( model_providers, project_doc_max_bytes: 32 * 1024, project_doc_fallback_filenames: Vec::new(), + claude_compat_instructions: false, + project_doc_all: false, tool_output_token_limit: None, agents_enabled: true, agent_max_threads: Some(6), diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index 2840e99609fd..929a81d49723 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -1395,6 +1395,7 @@ approval_policy = "never" r#async: false, status_message: Some("checking".to_string()), additional_context_limit: None, + fail_closed: false, }], }], ..Default::default() diff --git a/sdk/python/src/openai_codex/generated/v2_all.py b/sdk/python/src/openai_codex/generated/v2_all.py index 5bbd86342beb..dc8a0e9da697 100644 --- a/sdk/python/src/openai_codex/generated/v2_all.py +++ b/sdk/python/src/openai_codex/generated/v2_all.py @@ -1001,6 +1001,13 @@ class CommandConfiguredHookHandler(BaseModel): async_: Annotated[bool, Field(alias="async")] command: str command_windows: Annotated[str | None, Field(alias="commandWindows")] = None + fail_closed: Annotated[ + bool | None, + Field( + alias="failClosed", + description="When true, a failure of this hook blocks the tool instead of failing open.", + ), + ] = False status_message: Annotated[str | None, Field(alias="statusMessage")] = None timeout_sec: Annotated[int | None, Field(alias="timeoutSec", ge=0)] = None type: Annotated[Literal["command"], Field(title="CommandConfiguredHookHandlerType")]