mirrors/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-27 18:01:04 +03:00
Code Issues Packages Projects Releases Wiki Activity

Make environment filesystem explicit

Browse Source 
Require project-doc and skill injection loading to take an explicit filesystem, create the session environment before initial user-instruction loading, and keep unified exec routing behind the existing manager/factory surface.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
starr-openai
2026-03-19 13:38:31 -07:00
parent bba7b95d5e
commit 9c191254cd
7 changed files with 129 additions and 161 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
codex-rs/core/src/codex.rs
View File

@@ -42,7 +42,7 @@ use crate::realtime_conversation::handle_close as handle_realtime_conversation_c
use crate::realtime_conversation::handle_start as handle_realtime_conversation_start;
use crate::realtime_conversation::handle_text as handle_realtime_conversation_text;
use crate::rollout::session_index;
use crate::skills::build_skill_injections_with_filesystem;
use crate::skills::build_skill_injections;
use crate::skills::render_skills_section;
use crate::stream_events_utils::HandleOutputCtx;
use crate::stream_events_utils::handle_non_tool_response_item;
@@ -483,7 +483,12 @@ impl Codex {
config.startup_warnings.push(message);
}
let user_instructions = get_user_instructions(&config).await;
let environment = Arc::new(
Environment::create(config.experimental_exec_server_url.clone())
.await
.map_err(|err| CodexErr::Fatal(format!("failed to create environment: {err}")))?,
);
let user_instructions = get_user_instructions(&config, environment.filesystem()).await;
let exec_policy = if crate::guardian::is_guardian_reviewer_source(&session_source) {
// Guardian review should rely on the built-in shell safety checks,
@@ -1774,11 +1779,6 @@ impl Session {
});
}
let environment = Arc::new(
Environment::create(config.experimental_exec_server_url.clone())
.await
.map_err(|err| CodexErr::Fatal(format!("failed to create environment: {err}")))?,
);
let unified_exec_session_factory =
unified_exec_session_factory_for_environment(environment.as_ref());
@@ -5475,7 +5475,7 @@ pub(crate) async fn run_turn(
let SkillInjections {
items: skill_items,
warnings: skill_warnings,
} = build_skill_injections_with_filesystem(
} = build_skill_injections(
&mentioned_skills,
Some(&session_telemetry),
&sess.services.analytics_events_client,

39
codex-rs/core/src/project_doc.rs
View File

@@ -22,11 +22,9 @@ use crate::config_loader::merge_toml_values;
use crate::config_loader::project_root_markers_from_config;
use crate::features::Feature;
use codex_app_server_protocol::ConfigLayerSource;
use codex_exec_server::Environment;
use codex_exec_server::ExecutorFileSystem;
use codex_utils_absolute_path::AbsolutePathBuf;
use dunce::canonicalize as normalize_path;
use std::io;
use std::path::PathBuf;
use std::sync::Arc;
use toml::Value as TomlValue;
@@ -80,21 +78,11 @@ fn render_js_repl_instructions(config: &Config) -> Option<String> {
/// Combines `Config::instructions` and `AGENTS.md` (if present) into a single
/// string of instructions.
pub(crate) async fn get_user_instructions(config: &Config) -> Option<String> {
let file_system = match filesystem_for_config(config).await {
Ok(file_system) => Some(file_system),
Err(err) => {
error!("error creating environment filesystem for project docs: {err}");
None
}
};
let project_docs = if let Some(file_system) = file_system {
read_project_docs_with_filesystem(config, &file_system).await
} else {
Ok(None)
};
pub(crate) async fn get_user_instructions(
config: &Config,
filesystem: Arc<dyn ExecutorFileSystem>,
) -> Option<String> {
let project_docs = read_project_docs(config, filesystem).await;
let mut output = String::new();
if let Some(instructions) = config.user_instructions.clone() {
@@ -141,8 +129,10 @@ pub(crate) async fn get_user_instructions(config: &Config) -> Option<String> {
/// concatenation of all discovered docs. If no documentation file is found the
/// function returns `Ok(None)`. Unexpected I/O failures bubble up as `Err` so
/// callers can decide how to handle them.
pub async fn read_project_docs(config: &Config) -> std::io::Result<Option<String>> {
let filesystem = filesystem_for_config(config).await?;
pub async fn read_project_docs(
config: &Config,
filesystem: Arc<dyn ExecutorFileSystem>,
) -> std::io::Result<Option<String>> {
read_project_docs_with_filesystem(config, &filesystem).await
}
@@ -212,17 +202,6 @@ async fn read_project_docs_with_filesystem(
}
}
async fn filesystem_for_config(config: &Config) -> io::Result<Arc<dyn ExecutorFileSystem>> {
if let Some(url) = &config.experimental_exec_server_url {
let environment = Environment::create(Some(url.to_string()))
.await
.map_err(|err| io::Error::other(err.to_string()))?;
Ok(environment.get_filesystem())
} else {
Ok(Environment::local().get_filesystem())
}
}
/// Discover the list of AGENTS.md files using the same search rules as
/// `read_project_docs`, but return the file paths instead of concatenated
/// contents. The list is ordered from project root to the current working

60
codex-rs/core/src/project_doc_tests.rs
View File

@@ -1,10 +1,17 @@
use super::*;
use crate::config::ConfigBuilder;
use crate::features::Feature;
use codex_exec_server::Environment;
use codex_exec_server::ExecutorFileSystem;
use std::fs;
use std::path::PathBuf;
use std::sync::Arc;
use tempfile::TempDir;
fn test_filesystem() -> Arc<dyn ExecutorFileSystem> {
Environment::default().filesystem()
}
/// Helper that returns a `Config` pointing at `root` and using `limit` as
/// the maximum number of bytes to embed from AGENTS.md. The caller can
/// optionally specify a custom `instructions` string – when `None` the
@@ -73,7 +80,7 @@ async fn make_config_with_project_root_markers(
async fn no_doc_file_returns_none() {
let tmp = tempfile::tempdir().expect("tempdir");
let res = get_user_instructions(&make_config(&tmp, 4096, None).await).await;
let res = get_user_instructions(&make_config(&tmp, 4096, None).await, test_filesystem()).await;
assert!(
res.is_none(),
"Expected None when AGENTS.md is absent and no system instructions provided"
@@ -87,7 +94,7 @@ async fn doc_smaller_than_limit_is_returned() {
let tmp = tempfile::tempdir().expect("tempdir");
fs::write(tmp.path().join("AGENTS.md"), "hello world").unwrap();
let res = get_user_instructions(&make_config(&tmp, 4096, None).await)
let res = get_user_instructions(&make_config(&tmp, 4096, None).await, test_filesystem())
.await
.expect("doc expected");
@@ -106,7 +113,7 @@ async fn doc_larger_than_limit_is_truncated() {
let huge = "A".repeat(LIMIT * 2); // 2 KiB
fs::write(tmp.path().join("AGENTS.md"), &huge).unwrap();
let res = get_user_instructions(&make_config(&tmp, LIMIT, None).await)
let res = get_user_instructions(&make_config(&tmp, LIMIT, None).await, test_filesystem())
.await
.expect("doc expected");
@@ -138,7 +145,9 @@ async fn finds_doc_in_repo_root() {
let mut cfg = make_config(&repo, 4096, None).await;
cfg.cwd = nested;
let res = get_user_instructions(&cfg).await.expect("doc expected");
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("doc expected");
assert_eq!(res, "root level doc");
}
@@ -148,7 +157,7 @@ async fn zero_byte_limit_disables_docs() {
let tmp = tempfile::tempdir().expect("tempdir");
fs::write(tmp.path().join("AGENTS.md"), "something").unwrap();
let res = get_user_instructions(&make_config(&tmp, 0, None).await).await;
let res = get_user_instructions(&make_config(&tmp, 0, None).await, test_filesystem()).await;
assert!(
res.is_none(),
"With limit 0 the function should return None"
@@ -163,7 +172,7 @@ async fn js_repl_instructions_are_appended_when_enabled() {
.enable(Feature::JsRepl)
.expect("test config should allow js_repl");
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("js_repl instructions expected");
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.cwd`, `codex.homeDir`, `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n- Top-level bindings persist across cells. If a cell throws, prior bindings remain available and bindings that finished initializing before the throw often remain usable in later cells. For code you plan to reuse across cells, prefer declaring or assigning it in direct top-level statements before operations that might throw. If you hit `SyntaxError: Identifier 'x' has already been declared`, first reuse the existing binding, reassign a previously declared `let`, or pick a new descriptive name. Use `{ ... }` only for a short temporary block when you specifically need local scratch names; do not wrap an entire cell in block scope if you want those names reusable later. Reset the kernel with `js_repl_reset` only when you need a clean state.\n- Top-level static import declarations (for example `import x from \"./file.js\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")`, `await import(\"./file.js\")`, or `await import(\"/abs/path/file.mjs\")` instead. Imported local files must be ESM `.js`/`.mjs` files and run in the same REPL VM context. Bare package imports always resolve from REPL-global search roots (`CODEX_JS_REPL_NODE_MODULE_DIRS`, then cwd), not relative to the imported file location. Local files may statically import only other local relative/absolute/`file://` `.js`/`.mjs` files; package and builtin imports from local files must stay dynamic. `import.meta.resolve()` returns importable strings such as `file://...`, bare package names, and `node:...` specifiers. Local file modules reload between execs, while top-level bindings persist until `js_repl_reset`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`.";
@@ -182,7 +191,7 @@ async fn js_repl_tools_only_instructions_are_feature_gated() {
.set(features)
.expect("test config should allow js_repl tool restrictions");
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("js_repl instructions expected");
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.cwd`, `codex.homeDir`, `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n- Top-level bindings persist across cells. If a cell throws, prior bindings remain available and bindings that finished initializing before the throw often remain usable in later cells. For code you plan to reuse across cells, prefer declaring or assigning it in direct top-level statements before operations that might throw. If you hit `SyntaxError: Identifier 'x' has already been declared`, first reuse the existing binding, reassign a previously declared `let`, or pick a new descriptive name. Use `{ ... }` only for a short temporary block when you specifically need local scratch names; do not wrap an entire cell in block scope if you want those names reusable later. Reset the kernel with `js_repl_reset` only when you need a clean state.\n- Top-level static import declarations (for example `import x from \"./file.js\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")`, `await import(\"./file.js\")`, or `await import(\"/abs/path/file.mjs\")` instead. Imported local files must be ESM `.js`/`.mjs` files and run in the same REPL VM context. Bare package imports always resolve from REPL-global search roots (`CODEX_JS_REPL_NODE_MODULE_DIRS`, then cwd), not relative to the imported file location. Local files may statically import only other local relative/absolute/`file://` `.js`/`.mjs` files; package and builtin imports from local files must stay dynamic. `import.meta.resolve()` returns importable strings such as `file://...`, bare package names, and `node:...` specifiers. Local file modules reload between execs, while top-level bindings persist until `js_repl_reset`.\n- Do not call tools directly; use `js_repl` + `codex.tool(...)` for all tool calls, including shell commands.\n- MCP tools (if any) can also be called by name via `codex.tool(...)`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`.";
@@ -201,7 +210,7 @@ async fn js_repl_image_detail_original_does_not_change_instructions() {
.set(features)
.expect("test config should allow js_repl image detail settings");
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("js_repl instructions expected");
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.cwd`, `codex.homeDir`, `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n- Top-level bindings persist across cells. If a cell throws, prior bindings remain available and bindings that finished initializing before the throw often remain usable in later cells. For code you plan to reuse across cells, prefer declaring or assigning it in direct top-level statements before operations that might throw. If you hit `SyntaxError: Identifier 'x' has already been declared`, first reuse the existing binding, reassign a previously declared `let`, or pick a new descriptive name. Use `{ ... }` only for a short temporary block when you specifically need local scratch names; do not wrap an entire cell in block scope if you want those names reusable later. Reset the kernel with `js_repl_reset` only when you need a clean state.\n- Top-level static import declarations (for example `import x from \"./file.js\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")`, `await import(\"./file.js\")`, or `await import(\"/abs/path/file.mjs\")` instead. Imported local files must be ESM `.js`/`.mjs` files and run in the same REPL VM context. Bare package imports always resolve from REPL-global search roots (`CODEX_JS_REPL_NODE_MODULE_DIRS`, then cwd), not relative to the imported file location. Local files may statically import only other local relative/absolute/`file://` `.js`/`.mjs` files; package and builtin imports from local files must stay dynamic. `import.meta.resolve()` returns importable strings such as `file://...`, bare package names, and `node:...` specifiers. Local file modules reload between execs, while top-level bindings persist until `js_repl_reset`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`.";
@@ -217,9 +226,12 @@ async fn merges_existing_instructions_with_project_doc() {
const INSTRUCTIONS: &str = "base instructions";
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)).await)
.await
.expect("should produce a combined instruction string");
let res = get_user_instructions(
&make_config(&tmp, 4096, Some(INSTRUCTIONS)).await,
test_filesystem(),
)
.await
.expect("should produce a combined instruction string");
let expected = format!("{INSTRUCTIONS}{PROJECT_DOC_SEPARATOR}{}", "proj doc");
@@ -234,7 +246,11 @@ async fn keeps_existing_instructions_when_doc_missing() {
const INSTRUCTIONS: &str = "some instructions";
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)).await).await;
let res = get_user_instructions(
&make_config(&tmp, 4096, Some(INSTRUCTIONS)).await,
test_filesystem(),
)
.await;
assert_eq!(res, Some(INSTRUCTIONS.to_string()));
}
@@ -263,7 +279,9 @@ async fn concatenates_root_and_cwd_docs() {
let mut cfg = make_config(&repo, 4096, None).await;
cfg.cwd = nested;
let res = get_user_instructions(&cfg).await.expect("doc expected");
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("doc expected");
assert_eq!(res, "root doc\n\ncrate doc");
}
@@ -289,7 +307,9 @@ async fn project_root_markers_are_honored_for_agents_discovery() {
assert_eq!(discovery[0], expected_parent);
assert_eq!(discovery[1], expected_child);
let res = get_user_instructions(&cfg).await.expect("doc expected");
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("doc expected");
assert_eq!(res, "parent doc\n\nchild doc");
}
@@ -302,7 +322,7 @@ async fn agents_local_md_preferred() {
let cfg = make_config(&tmp, 4096, None).await;
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("local doc expected");
@@ -324,7 +344,7 @@ async fn uses_configured_fallback_when_agents_missing() {
let cfg = make_config_with_fallback(&tmp, 4096, None, &["EXAMPLE.md"]).await;
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("fallback doc expected");
@@ -340,7 +360,7 @@ async fn agents_md_preferred_over_fallbacks() {
let cfg = make_config_with_fallback(&tmp, 4096, None, &["EXAMPLE.md", ".example.md"]).await;
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("AGENTS.md should win");
@@ -369,7 +389,7 @@ async fn skills_are_not_appended_to_project_doc() {
"extract from pdfs",
);
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("instructions expected");
assert_eq!(res, "base doc");
@@ -383,7 +403,7 @@ async fn apps_feature_does_not_emit_user_instructions_by_itself() {
.enable(Feature::Apps)
.expect("test config should allow apps");
let res = get_user_instructions(&cfg).await;
let res = get_user_instructions(&cfg, test_filesystem()).await;
assert_eq!(res, None);
}
@@ -397,7 +417,7 @@ async fn apps_feature_does_not_append_to_project_doc_user_instructions() {
.enable(Feature::Apps)
.expect("test config should allow apps");
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, test_filesystem())
.await
.expect("instructions expected");
assert_eq!(res, "base doc");

17
codex-rs/core/src/skills/injection.rs
View File

@@ -14,7 +14,6 @@ use crate::mention_syntax::TOOL_MENTION_SIGIL;
use crate::mentions::build_skill_name_counts;
use crate::skills::SkillMetadata;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::LocalFileSystem;
use codex_otel::SessionTelemetry;
use codex_protocol::models::ResponseItem;
use codex_protocol::user_input::UserInput;
@@ -54,22 +53,6 @@ pub(crate) async fn build_skill_injections(
otel: Option<&SessionTelemetry>,
analytics_client: &AnalyticsEventsClient,
tracking: TrackEventsContext,
) -> SkillInjections {
build_skill_injections_with_filesystem(
mentioned_skills,
otel,
analytics_client,
tracking,
Arc::new(LocalFileSystem),
)
.await
}
pub(crate) async fn build_skill_injections_with_filesystem(
mentioned_skills: &[SkillMetadata],
otel: Option<&SessionTelemetry>,
analytics_client: &AnalyticsEventsClient,
tracking: TrackEventsContext,
filesystem: Arc<dyn ExecutorFileSystem>,
) -> SkillInjections {
if mentioned_skills.is_empty() {

1
codex-rs/core/src/skills/mod.rs
View File

@@ -12,7 +12,6 @@ pub(crate) use env_var_dependencies::collect_env_var_dependencies;
pub(crate) use env_var_dependencies::resolve_skill_dependencies_for_turn;
pub(crate) use injection::SkillInjections;
pub(crate) use injection::build_skill_injections;
pub(crate) use injection::build_skill_injections_with_filesystem;
pub(crate) use injection::collect_explicit_skill_mentions;
pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
pub(crate) use invocation_utils::maybe_emit_implicit_skill_invocation;

67
codex-rs/core/src/unified_exec/backend.rs
View File

@@ -2,10 +2,7 @@ use std::sync::Arc;
use async_trait::async_trait;
use codex_exec_server::Environment;
use codex_exec_server::ExecSpawnRequest;
use codex_exec_server::InheritedFd;
use codex_exec_server::SandboxKind;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_exec_server::ExecServerClient;
use tracing::debug;
use crate::exec::SandboxType;
@@ -47,31 +44,31 @@ impl UnifiedExecSessionFactory for LocalUnifiedExecSessionFactory {
}
}
pub(crate) struct EnvironmentUnifiedExecSessionFactory {
environment: Arc<Environment>,
pub(crate) struct ExecServerUnifiedExecSessionFactory {
client: ExecServerClient,
}
impl std::fmt::Debug for EnvironmentUnifiedExecSessionFactory {
impl std::fmt::Debug for ExecServerUnifiedExecSessionFactory {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("EnvironmentUnifiedExecSessionFactory")
f.debug_struct("ExecServerUnifiedExecSessionFactory")
.finish_non_exhaustive()
}
}
impl EnvironmentUnifiedExecSessionFactory {
pub(crate) fn new(environment: Arc<Environment>) -> UnifiedExecSessionFactoryHandle {
Arc::new(Self { environment })
impl ExecServerUnifiedExecSessionFactory {
pub(crate) fn from_client(client: ExecServerClient) -> UnifiedExecSessionFactoryHandle {
Arc::new(Self { client })
}
}
#[async_trait]
impl UnifiedExecSessionFactory for EnvironmentUnifiedExecSessionFactory {
impl UnifiedExecSessionFactory for ExecServerUnifiedExecSessionFactory {
async fn open_session(
&self,
process_id: i32,
env: &ExecRequest,
tty: bool,
mut spawn_lifecycle: SpawnLifecycleHandle,
spawn_lifecycle: SpawnLifecycleHandle,
) -> Result<UnifiedExecProcess, UnifiedExecError> {
let inherited_fds = spawn_lifecycle.inherited_fds();
if !inherited_fds.is_empty() {
@@ -91,34 +88,25 @@ impl UnifiedExecSessionFactory for EnvironmentUnifiedExecSessionFactory {
return open_local_session(env, tty, spawn_lifecycle).await;
}
let session = self
.environment
.executor()
.spawn(ExecSpawnRequest {
process_id: process_id.to_string(),
argv: env.command.clone(),
cwd: AbsolutePathBuf::try_from(env.cwd.clone())
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?,
env: env.env.clone(),
arg0: env.arg0.clone(),
tty,
sandbox: sandbox_kind(env.sandbox),
inherited_fds: inherited_fds
.into_iter()
.map(|target_fd| InheritedFd { target_fd })
.collect(),
})
.await
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
spawn_lifecycle.after_spawn();
UnifiedExecProcess::from_exec_session(session, env.sandbox, tty, spawn_lifecycle).await
UnifiedExecProcess::from_exec_server(
self.client.clone(),
process_id,
env,
tty,
spawn_lifecycle,
)
.await
}
}
pub(crate) fn unified_exec_session_factory_for_environment(
environment: &Environment,
) -> UnifiedExecSessionFactoryHandle {
EnvironmentUnifiedExecSessionFactory::new(Arc::new(environment.clone()))
if let Some(client) = environment.exec_server_client() {
ExecServerUnifiedExecSessionFactory::from_client(client)
} else {
local_unified_exec_session_factory()
}
}
async fn open_local_session(
@@ -158,12 +146,3 @@ async fn open_local_session(
spawn_lifecycle.after_spawn();
UnifiedExecProcess::from_spawned(spawned, env.sandbox, spawn_lifecycle).await
}
fn sandbox_kind(sandbox: SandboxType) -> SandboxKind {
match sandbox {
SandboxType::None => SandboxKind::None,
SandboxType::MacosSeatbelt => SandboxKind::MacosSeatbelt,
SandboxType::LinuxSeccomp => SandboxKind::LinuxSeccomp,
SandboxType::WindowsRestrictedToken => SandboxKind::WindowsRestrictedToken,
}
}

90
codex-rs/core/src/unified_exec/process.rs
View File

@@ -17,10 +17,12 @@ use crate::exec::ExecToolCallOutput;
use crate::exec::SandboxType;
use crate::exec::StreamOutput;
use crate::exec::is_likely_sandbox_denied;
use crate::sandboxing::ExecRequest;
use crate::truncate::TruncationPolicy;
use crate::truncate::formatted_truncate_text;
use codex_exec_server::ExecOutputEvent;
use codex_exec_server::ExecSession;
use codex_exec_server::ExecParams;
use codex_exec_server::ExecServerClient;
use codex_exec_server::ExecServerEvent;
use codex_utils_pty::ExecCommandSession;
use codex_utils_pty::SpawnedPty;
@@ -89,7 +91,7 @@ impl std::fmt::Debug for ProcessBackend {
#[derive(Clone)]
struct RemoteExecSession {
process_key: String,
session: Arc<dyn ExecSession>,
client: ExecServerClient,
writer_tx: mpsc::Sender<Vec<u8>>,
exited: Arc<AtomicBool>,
exit_code: Arc<StdMutex<Option<i32>>>,
@@ -214,10 +216,11 @@ impl UnifiedExecProcess {
match &self.process_handle {
ProcessBackend::Local(process_handle) => process_handle.terminate(),
ProcessBackend::Remote(process_handle) => {
let session = Arc::clone(&process_handle.session);
let client = process_handle.client.clone();
let process_key = process_handle.process_key.clone();
if let Ok(handle) = tokio::runtime::Handle::try_current() {
handle.spawn(async move {
let _ = session.terminate().await;
let _ = client.terminate(&process_key).await;
});
}
}
@@ -329,15 +332,27 @@ impl UnifiedExecProcess {
Ok(managed)
}
pub(super) async fn from_exec_session(
session: Box<dyn ExecSession>,
sandbox_type: SandboxType,
_tty: bool,
pub(super) async fn from_exec_server(
client: ExecServerClient,
process_id: i32,
env: &ExecRequest,
tty: bool,
spawn_lifecycle: SpawnLifecycleHandle,
) -> Result<Self, UnifiedExecError> {
let process_key = session.process_id().to_string();
let session: Arc<dyn ExecSession> = Arc::from(session);
let mut session_output_rx = session.subscribe_output();
let process_key = process_id.to_string();
let mut events_rx = client.event_receiver();
client
.exec(ExecParams {
process_id: process_key.clone(),
argv: env.command.clone(),
cwd: env.cwd.clone(),
env: env.env.clone(),
tty,
arg0: env.arg0.clone(),
})
.await
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
let (output_tx, output_rx) = broadcast::channel(256);
let (writer_tx, mut writer_rx) = mpsc::channel::<Vec<u8>>(256);
let exited = Arc::new(AtomicBool::new(false));
@@ -346,21 +361,21 @@ impl UnifiedExecProcess {
let managed = Self::new(
ProcessBackend::Remote(RemoteExecSession {
process_key: process_key.clone(),
session: Arc::clone(&session),
client: client.clone(),
writer_tx,
exited: Arc::clone(&exited),
exit_code: Arc::clone(&exit_code),
}),
output_rx,
sandbox_type,
env.sandbox,
spawn_lifecycle,
);
{
let session = Arc::clone(&session);
let client = client.clone();
tokio::spawn(async move {
while let Some(chunk) = writer_rx.recv().await {
if session.write(chunk).await.is_err() {
if client.write(&process_key, chunk).await.is_err() {
break;
}
}
@@ -368,41 +383,34 @@ impl UnifiedExecProcess {
}
{
let process_key = process_id.to_string();
let exited = Arc::clone(&exited);
let exit_code = Arc::clone(&exit_code);
let cancellation_token = managed.cancellation_token();
let session = Arc::clone(&session);
tokio::spawn(async move {
loop {
match session_output_rx.recv().await {
Ok(ExecOutputEvent::Stdout(chunk) | ExecOutputEvent::Stderr(chunk)) => {
let _ = output_tx.send(chunk);
while let Ok(event) = events_rx.recv().await {
match event {
ExecServerEvent::OutputDelta(notification)
if notification.process_id == process_key =>
{
let _ = output_tx.send(notification.chunk.into_inner());
}
Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => continue,
Err(tokio::sync::broadcast::error::RecvError::Closed) => break,
ExecServerEvent::Exited(notification)
if notification.process_id == process_key =>
{
exited.store(true, Ordering::SeqCst);
if let Ok(mut guard) = exit_code.lock() {
*guard = Some(notification.exit_code);
}
cancellation_token.cancel();
break;
}
ExecServerEvent::OutputDelta(_) | ExecServerEvent::Exited(_) => {}
}
}
});
tokio::spawn(async move {
let exit = session.wait().await;
if let Ok(exit) = exit {
exited.store(true, Ordering::SeqCst);
if let Ok(mut guard) = exit_code.lock() {
*guard = Some(exit.exit_code);
}
}
cancellation_token.cancel();
});
}
if let Some(exit) = session.try_exit_status() {
exited.store(true, Ordering::SeqCst);
if let Ok(mut guard) = exit_code.lock() {
*guard = Some(exit.exit_code);
}
managed.signal_exit();
}
Ok(managed)
}