mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
fix(exec): carry add-dir roots through app server
This commit is contained in:
@@ -3790,6 +3790,9 @@ pub struct ThreadStartParams {
|
||||
pub service_tier: Option<Option<ServiceTier>>,
|
||||
#[ts(optional = nullable)]
|
||||
pub cwd: Option<String>,
|
||||
#[experimental("thread/start.projectRoots")]
|
||||
#[ts(optional = nullable)]
|
||||
pub project_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
#[experimental(nested)]
|
||||
#[ts(optional = nullable)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
@@ -3944,6 +3947,9 @@ pub struct ThreadResumeParams {
|
||||
pub service_tier: Option<Option<ServiceTier>>,
|
||||
#[ts(optional = nullable)]
|
||||
pub cwd: Option<String>,
|
||||
#[experimental("thread/resume.projectRoots")]
|
||||
#[ts(optional = nullable)]
|
||||
pub project_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
#[experimental(nested)]
|
||||
#[ts(optional = nullable)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
|
||||
@@ -728,6 +728,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
project_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -756,6 +757,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
project_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -1107,6 +1109,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider: Option<String>,
|
||||
service_tier: Option<Option<codex_protocol::config_types::ServiceTier>>,
|
||||
cwd: Option<String>,
|
||||
project_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
|
||||
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
|
||||
sandbox: Option<SandboxMode>,
|
||||
@@ -1115,16 +1118,25 @@ impl ThreadRequestProcessor {
|
||||
developer_instructions: Option<String>,
|
||||
personality: Option<Personality>,
|
||||
) -> ConfigOverrides {
|
||||
let cwd = cwd.map(PathBuf::from);
|
||||
let cwd_for_filter = cwd.as_deref();
|
||||
let additional_writable_roots = project_roots
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.map(AbsolutePathBuf::into_path_buf)
|
||||
.filter(|root| Some(root.as_path()) != cwd_for_filter)
|
||||
.collect();
|
||||
let mut overrides = ConfigOverrides {
|
||||
model,
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd: cwd.map(PathBuf::from),
|
||||
cwd,
|
||||
approval_policy: approval_policy
|
||||
.map(codex_app_server_protocol::AskForApproval::to_core),
|
||||
approvals_reviewer: approvals_reviewer
|
||||
.map(codex_app_server_protocol::ApprovalsReviewer::to_core),
|
||||
sandbox_mode: sandbox.map(SandboxMode::to_core),
|
||||
additional_writable_roots,
|
||||
codex_linux_sandbox_exe: self.arg0_paths.codex_linux_sandbox_exe.clone(),
|
||||
main_execve_wrapper_exe: self.arg0_paths.main_execve_wrapper_exe.clone(),
|
||||
base_instructions,
|
||||
@@ -2227,6 +2239,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
project_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -2262,6 +2275,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
project_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -2879,6 +2893,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
/*project_roots*/ None,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
|
||||
@@ -514,6 +514,7 @@ mod thread_processor_behavior_tests {
|
||||
model_provider: None,
|
||||
service_tier: Some(Some(codex_protocol::config_types::ServiceTier::Fast)),
|
||||
cwd: None,
|
||||
project_roots: None,
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
sandbox: None,
|
||||
|
||||
@@ -664,6 +664,7 @@ async fn skills_changed_notification_is_emitted_after_skill_change() -> Result<(
|
||||
model_provider: None,
|
||||
service_tier: None,
|
||||
cwd: None,
|
||||
project_roots: None,
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
sandbox: None,
|
||||
|
||||
@@ -207,6 +207,7 @@ struct ExecRunArgs {
|
||||
oss: bool,
|
||||
output_schema_path: Option<PathBuf>,
|
||||
prompt: Option<String>,
|
||||
project_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
skip_git_repo_check: bool,
|
||||
stderr_with_ansi: bool,
|
||||
}
|
||||
@@ -304,6 +305,14 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
}
|
||||
None => AbsolutePathBuf::current_dir()?,
|
||||
};
|
||||
let project_roots =
|
||||
(!add_dir.is_empty()).then(|| {
|
||||
std::iter::once(config_cwd.clone())
|
||||
.chain(add_dir.iter().cloned().map(|path| {
|
||||
AbsolutePathBuf::resolve_path_against_base(path, config_cwd.as_path())
|
||||
}))
|
||||
.collect()
|
||||
});
|
||||
|
||||
// we load config.toml here to determine project state.
|
||||
#[allow(clippy::print_stderr)]
|
||||
@@ -541,6 +550,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
oss,
|
||||
output_schema_path,
|
||||
prompt,
|
||||
project_roots,
|
||||
skip_git_repo_check,
|
||||
stderr_with_ansi,
|
||||
})
|
||||
@@ -563,9 +573,11 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
oss,
|
||||
output_schema_path,
|
||||
prompt,
|
||||
project_roots,
|
||||
skip_git_repo_check,
|
||||
stderr_with_ansi,
|
||||
} = args;
|
||||
let project_roots = project_roots.as_deref();
|
||||
|
||||
let mut event_processor: Box<dyn EventProcessor> = match json_mode {
|
||||
true => Box::new(EventProcessorWithJsonOutput::new(last_message_file.clone())),
|
||||
@@ -685,7 +697,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
&client,
|
||||
ClientRequest::ThreadResume {
|
||||
request_id: request_ids.next(),
|
||||
params: thread_resume_params_from_config(&config, thread_id),
|
||||
params: thread_resume_params_from_config(&config, thread_id, project_roots),
|
||||
},
|
||||
"thread/resume",
|
||||
)
|
||||
@@ -700,7 +712,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
&client,
|
||||
ClientRequest::ThreadStart {
|
||||
request_id: request_ids.next(),
|
||||
params: thread_start_params_from_config(&config),
|
||||
params: thread_start_params_from_config(&config, project_roots),
|
||||
},
|
||||
"thread/start",
|
||||
)
|
||||
@@ -716,7 +728,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
&client,
|
||||
ClientRequest::ThreadStart {
|
||||
request_id: request_ids.next(),
|
||||
params: thread_start_params_from_config(&config),
|
||||
params: thread_start_params_from_config(&config, project_roots),
|
||||
},
|
||||
"thread/start",
|
||||
)
|
||||
@@ -929,7 +941,10 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn thread_start_params_from_config(config: &Config) -> ThreadStartParams {
|
||||
fn thread_start_params_from_config(
|
||||
config: &Config,
|
||||
project_roots: Option<&[AbsolutePathBuf]>,
|
||||
) -> ThreadStartParams {
|
||||
let permissions = permissions_selection_from_config(config);
|
||||
let sandbox = permissions.is_none().then(|| {
|
||||
sandbox_mode_from_permission_profile(
|
||||
@@ -945,13 +960,18 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams {
|
||||
approvals_reviewer: approvals_reviewer_override_from_config(config),
|
||||
sandbox: sandbox.flatten(),
|
||||
permissions,
|
||||
project_roots: project_roots.map(|roots| roots.to_vec()),
|
||||
config: config_request_overrides_from_config(config),
|
||||
ephemeral: Some(config.ephemeral),
|
||||
..ThreadStartParams::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_resume_params_from_config(config: &Config, thread_id: String) -> ThreadResumeParams {
|
||||
fn thread_resume_params_from_config(
|
||||
config: &Config,
|
||||
thread_id: String,
|
||||
project_roots: Option<&[AbsolutePathBuf]>,
|
||||
) -> ThreadResumeParams {
|
||||
let permissions = permissions_selection_from_config(config);
|
||||
let sandbox = permissions.is_none().then(|| {
|
||||
sandbox_mode_from_permission_profile(
|
||||
@@ -968,6 +988,7 @@ fn thread_resume_params_from_config(config: &Config, thread_id: String) -> Threa
|
||||
approvals_reviewer: approvals_reviewer_override_from_config(config),
|
||||
sandbox: sandbox.flatten(),
|
||||
permissions,
|
||||
project_roots: project_roots.map(|roots| roots.to_vec()),
|
||||
config: config_request_overrides_from_config(config),
|
||||
..ThreadResumeParams::default()
|
||||
}
|
||||
|
||||
@@ -355,7 +355,7 @@ async fn thread_start_params_include_review_policy_when_review_policy_is_manual_
|
||||
.await
|
||||
.expect("build config with manual-only review policy");
|
||||
|
||||
let params = thread_start_params_from_config(&config);
|
||||
let params = thread_start_params_from_config(&config, None);
|
||||
|
||||
assert_eq!(
|
||||
params.approvals_reviewer,
|
||||
@@ -383,7 +383,7 @@ async fn thread_start_params_include_review_policy_when_auto_review_is_enabled()
|
||||
.await
|
||||
.expect("build config with guardian review policy");
|
||||
|
||||
let params = thread_start_params_from_config(&config);
|
||||
let params = thread_start_params_from_config(&config, None);
|
||||
|
||||
assert_eq!(
|
||||
params.approvals_reviewer,
|
||||
@@ -395,31 +395,36 @@ async fn thread_start_params_include_review_policy_when_auto_review_is_enabled()
|
||||
async fn thread_lifecycle_params_include_legacy_sandbox_when_no_active_profile() {
|
||||
let codex_home = tempdir().expect("create temp codex home");
|
||||
let cwd = tempdir().expect("create temp cwd");
|
||||
let extra = AbsolutePathBuf::try_from(cwd.path().join("extra")).expect("absolute path");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
sandbox_mode: Some(SandboxMode::DangerFullAccess),
|
||||
sandbox_mode: Some(SandboxMode::WorkspaceWrite),
|
||||
..Default::default()
|
||||
})
|
||||
.fallback_cwd(Some(cwd.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("build config with legacy sandbox override");
|
||||
let project_roots = vec![config.cwd.clone(), extra];
|
||||
|
||||
let start_params = thread_start_params_from_config(&config);
|
||||
let resume_params = thread_resume_params_from_config(&config, "thread-id".to_string());
|
||||
let start_params = thread_start_params_from_config(&config, Some(&project_roots));
|
||||
let resume_params =
|
||||
thread_resume_params_from_config(&config, "thread-id".to_string(), Some(&project_roots));
|
||||
|
||||
assert_eq!(config.permissions.active_permission_profile(), None);
|
||||
assert_eq!(
|
||||
start_params.sandbox,
|
||||
Some(codex_app_server_protocol::SandboxMode::DangerFullAccess)
|
||||
Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite)
|
||||
);
|
||||
assert_eq!(start_params.permissions, None);
|
||||
assert_eq!(start_params.project_roots, Some(project_roots.clone()));
|
||||
assert_eq!(
|
||||
resume_params.sandbox,
|
||||
Some(codex_app_server_protocol::SandboxMode::DangerFullAccess)
|
||||
Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite)
|
||||
);
|
||||
assert_eq!(resume_params.permissions, None);
|
||||
assert_eq!(resume_params.project_roots, Some(project_roots));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user