fix(codex): preserve project root on structured resume
Recalcule la policy structured Codex lors des reprises custom/headless afin que le project root et le managed tmp soient transmis en writable_roots. Fail-closed si une session Codex structured est lancée sans policy explicite.
This commit is contained in:
@ -625,6 +625,7 @@ struct FakeStructuredFactory {
|
|||||||
envs: Arc<Mutex<Vec<Vec<(String, String)>>>>,
|
envs: Arc<Mutex<Vec<Vec<(String, String)>>>>,
|
||||||
sandboxes: Arc<Mutex<Vec<Option<domain::sandbox::SandboxPlan>>>>,
|
sandboxes: Arc<Mutex<Vec<Option<domain::sandbox::SandboxPlan>>>>,
|
||||||
policies: Arc<Mutex<Vec<Option<domain::ports::StructuredProviderLaunchPolicy>>>>,
|
policies: Arc<Mutex<Vec<Option<domain::ports::StructuredProviderLaunchPolicy>>>>,
|
||||||
|
plans: Arc<Mutex<Vec<SessionPlan>>>,
|
||||||
shutdowns: Arc<Mutex<Vec<SessionId>>>,
|
shutdowns: Arc<Mutex<Vec<SessionId>>>,
|
||||||
next_session: SessionId,
|
next_session: SessionId,
|
||||||
}
|
}
|
||||||
@ -638,6 +639,7 @@ impl FakeStructuredFactory {
|
|||||||
envs: Arc::new(Mutex::new(Vec::new())),
|
envs: Arc::new(Mutex::new(Vec::new())),
|
||||||
sandboxes: Arc::new(Mutex::new(Vec::new())),
|
sandboxes: Arc::new(Mutex::new(Vec::new())),
|
||||||
policies: Arc::new(Mutex::new(Vec::new())),
|
policies: Arc::new(Mutex::new(Vec::new())),
|
||||||
|
plans: Arc::new(Mutex::new(Vec::new())),
|
||||||
shutdowns: Arc::new(Mutex::new(Vec::new())),
|
shutdowns: Arc::new(Mutex::new(Vec::new())),
|
||||||
next_session,
|
next_session,
|
||||||
}
|
}
|
||||||
@ -662,6 +664,10 @@ impl FakeStructuredFactory {
|
|||||||
fn policies(&self) -> Vec<Option<domain::ports::StructuredProviderLaunchPolicy>> {
|
fn policies(&self) -> Vec<Option<domain::ports::StructuredProviderLaunchPolicy>> {
|
||||||
self.policies.lock().unwrap().clone()
|
self.policies.lock().unwrap().clone()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn plans(&self) -> Vec<SessionPlan> {
|
||||||
|
self.plans.lock().unwrap().clone()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[async_trait]
|
#[async_trait]
|
||||||
@ -675,7 +681,7 @@ impl AgentSessionFactory for FakeStructuredFactory {
|
|||||||
profile: &AgentProfile,
|
profile: &AgentProfile,
|
||||||
_ctx: &PreparedContext,
|
_ctx: &PreparedContext,
|
||||||
_cwd: &ProjectPath,
|
_cwd: &ProjectPath,
|
||||||
_session: &SessionPlan,
|
session: &SessionPlan,
|
||||||
_requester: Option<&str>,
|
_requester: Option<&str>,
|
||||||
_env: &[(String, String)],
|
_env: &[(String, String)],
|
||||||
_sandbox: Option<&domain::sandbox::SandboxPlan>,
|
_sandbox: Option<&domain::sandbox::SandboxPlan>,
|
||||||
@ -696,6 +702,7 @@ impl AgentSessionFactory for FakeStructuredFactory {
|
|||||||
.lock()
|
.lock()
|
||||||
.unwrap()
|
.unwrap()
|
||||||
.push(structured_policy.cloned());
|
.push(structured_policy.cloned());
|
||||||
|
self.plans.lock().unwrap().push(session.clone());
|
||||||
Ok(Arc::new(FakeSession {
|
Ok(Arc::new(FakeSession {
|
||||||
id: self.next_session,
|
id: self.next_session,
|
||||||
conversation_id: Some("engine-session-1".to_owned()),
|
conversation_id: Some("engine-session-1".to_owned()),
|
||||||
@ -4987,7 +4994,7 @@ async fn launch_structured_non_codex_uses_managed_tmp_for_env_without_os_sandbox
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn launch_structured_codex_uses_managed_tmp_for_env_and_writable_roots_without_os_sandbox() {
|
async fn custom_chat_codex_resume_recomputes_project_and_managed_tmp_writable_roots() {
|
||||||
let root = fresh_temp_dir("launch-tmp-structured");
|
let root = fresh_temp_dir("launch-tmp-structured");
|
||||||
let tmp = root.join("managed-tmp-main").to_string_lossy().into_owned();
|
let tmp = root.join("managed-tmp-main").to_string_lossy().into_owned();
|
||||||
let store = Arc::new(FakeManagedTempStore::default());
|
let store = Arc::new(FakeManagedTempStore::default());
|
||||||
@ -5011,10 +5018,10 @@ async fn launch_structured_codex_uses_managed_tmp_for_env_and_writable_roots_wit
|
|||||||
.with_structured_routing_mode(StructuredRoutingMode::RequireStructured)
|
.with_structured_routing_mode(StructuredRoutingMode::RequireStructured)
|
||||||
.with_structured(Arc::new(factory.clone()), Arc::clone(&structured));
|
.with_structured(Arc::new(factory.clone()), Arc::clone(&structured));
|
||||||
|
|
||||||
launch
|
let mut input = launch_input_with_project_root(agent.id, &root);
|
||||||
.execute(launch_input_with_project_root(agent.id, &root))
|
input.require_structured = true;
|
||||||
.await
|
input.conversation_id = Some("pair-conversation-306".to_owned());
|
||||||
.expect("structured launch");
|
launch.execute(input).await.expect("structured launch");
|
||||||
|
|
||||||
assert!(pty.spawns().is_empty());
|
assert!(pty.spawns().is_empty());
|
||||||
let env = factory.envs().remove(0);
|
let env = factory.envs().remove(0);
|
||||||
@ -5028,7 +5035,15 @@ async fn launch_structured_codex_uses_managed_tmp_for_env_and_writable_roots_wit
|
|||||||
else {
|
else {
|
||||||
panic!("expected Codex policy");
|
panic!("expected Codex policy");
|
||||||
};
|
};
|
||||||
assert!(writable_roots.contains(&tmp));
|
assert_eq!(
|
||||||
|
writable_roots,
|
||||||
|
vec![root.to_string_lossy().into_owned(), tmp.clone()],
|
||||||
|
"resume/custom chat policy must be rebuilt from the current project root and managed tmp"
|
||||||
|
);
|
||||||
|
assert!(matches!(
|
||||||
|
factory.plans().as_slice(),
|
||||||
|
[SessionPlan::Resume { conversation_id }] if conversation_id == "pair-conversation-306"
|
||||||
|
));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
factory.sandboxes(),
|
factory.sandboxes(),
|
||||||
vec![None],
|
vec![None],
|
||||||
|
|||||||
@ -1100,6 +1100,28 @@ fn self_restart_launch_input(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn automatic_resume_launch_input(
|
||||||
|
context: ResumeContext,
|
||||||
|
agent_id: AgentId,
|
||||||
|
node_id: domain::NodeId,
|
||||||
|
conversation_id: Option<String>,
|
||||||
|
mcp_runtime: Option<McpRuntime>,
|
||||||
|
) -> LaunchAgentInput {
|
||||||
|
LaunchAgentInput {
|
||||||
|
project: context.project,
|
||||||
|
agent_id,
|
||||||
|
rows: context.rows,
|
||||||
|
cols: context.cols,
|
||||||
|
node_id: Some(node_id),
|
||||||
|
conversation_id,
|
||||||
|
mcp_runtime,
|
||||||
|
allow_structured_alongside_pty: false,
|
||||||
|
require_structured: context.require_structured,
|
||||||
|
fresh_start: false,
|
||||||
|
managed_temp_exposure: application::ManagedTempExposureRequest::default(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod self_restart_contract_tests {
|
mod self_restart_contract_tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@ -1137,6 +1159,42 @@ mod self_restart_contract_tests {
|
|||||||
assert!(!input.fresh_start);
|
assert!(!input.fresh_start);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn automatic_resume_preserves_custom_chat_structured_routing() {
|
||||||
|
let project = Project::new(
|
||||||
|
ProjectId::from_uuid(Uuid::from_u128(306)),
|
||||||
|
"resume",
|
||||||
|
domain::project::ProjectPath::new("/external/project-root").unwrap(),
|
||||||
|
domain::remote::RemoteRef::local(),
|
||||||
|
0,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
let agent = AgentId::from_uuid(Uuid::from_u128(1));
|
||||||
|
let node = domain::NodeId::from_uuid(Uuid::from_u128(2));
|
||||||
|
let input = automatic_resume_launch_input(
|
||||||
|
ResumeContext {
|
||||||
|
project,
|
||||||
|
rows: 24,
|
||||||
|
cols: 80,
|
||||||
|
node_id: Some(node),
|
||||||
|
conversation_id: Some("old-conversation".to_owned()),
|
||||||
|
require_structured: true,
|
||||||
|
},
|
||||||
|
agent,
|
||||||
|
node,
|
||||||
|
Some("current-conversation".to_owned()),
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(input.require_structured);
|
||||||
|
assert_eq!(input.node_id, Some(node));
|
||||||
|
assert_eq!(
|
||||||
|
input.conversation_id.as_deref(),
|
||||||
|
Some("current-conversation")
|
||||||
|
);
|
||||||
|
assert!(!input.fresh_start);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn human_turn_ignores_old_terminal_and_accepts_replacement() {
|
async fn human_turn_ignores_old_terminal_and_accepts_replacement() {
|
||||||
let mailbox = Arc::new(InMemoryMailbox::new());
|
let mailbox = Arc::new(InMemoryMailbox::new());
|
||||||
@ -1682,19 +1740,13 @@ impl AgentResumer for AppAgentResumer {
|
|||||||
|
|
||||||
let project_id = ctx.project.id;
|
let project_id = ctx.project.id;
|
||||||
self.launch_agent
|
self.launch_agent
|
||||||
.execute(LaunchAgentInput {
|
.execute(automatic_resume_launch_input(
|
||||||
project: ctx.project,
|
ctx,
|
||||||
agent_id,
|
agent_id,
|
||||||
rows: ctx.rows,
|
node_id,
|
||||||
cols: ctx.cols,
|
|
||||||
node_id: Some(node_id),
|
|
||||||
conversation_id,
|
conversation_id,
|
||||||
mcp_runtime,
|
mcp_runtime,
|
||||||
allow_structured_alongside_pty: false,
|
))
|
||||||
require_structured: false,
|
|
||||||
fresh_start: false,
|
|
||||||
managed_temp_exposure: application::ManagedTempExposureRequest::default(),
|
|
||||||
})
|
|
||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
// Premier tour de reprise par le **portail d'entrée** (pas de write brut, §20) :
|
// Premier tour de reprise par le **portail d'entrée** (pas de write brut, §20) :
|
||||||
|
|||||||
@ -189,7 +189,11 @@ impl AgentSessionFactory for StructuredSessionFactory {
|
|||||||
writable_roots.clone(),
|
writable_roots.clone(),
|
||||||
Some(*network_access),
|
Some(*network_access),
|
||||||
),
|
),
|
||||||
None => ("workspace-write".to_owned(), Vec::new(), None),
|
None => {
|
||||||
|
return Err(AgentSessionError::Start(
|
||||||
|
"structured Codex launch requires an explicit launch policy".to_owned(),
|
||||||
|
));
|
||||||
|
}
|
||||||
};
|
};
|
||||||
Arc::new(CodexExecSession::new_with_policy_and_overrides(
|
Arc::new(CodexExecSession::new_with_policy_and_overrides(
|
||||||
id,
|
id,
|
||||||
|
|||||||
@ -51,7 +51,7 @@ mod tests {
|
|||||||
use domain::ports::{
|
use domain::ports::{
|
||||||
AgentSession, AgentSessionError, AgentSessionFactory, ContextInjectionPlan,
|
AgentSession, AgentSessionError, AgentSessionFactory, ContextInjectionPlan,
|
||||||
PreparedContext, ReplyEvent, ReplyProgressKind, ReplyProgressSource, ReplyProgressStage,
|
PreparedContext, ReplyEvent, ReplyProgressKind, ReplyProgressSource, ReplyProgressStage,
|
||||||
SessionPlan,
|
SessionPlan, StructuredProviderLaunchPolicy,
|
||||||
};
|
};
|
||||||
use domain::profile::{AgentProfile, ContextInjection, HttpChatConfig, StructuredAdapter};
|
use domain::profile::{AgentProfile, ContextInjection, HttpChatConfig, StructuredAdapter};
|
||||||
use domain::project::ProjectPath;
|
use domain::project::ProjectPath;
|
||||||
@ -589,6 +589,11 @@ mod tests {
|
|||||||
// Codex : routé vers l'adapter Codex (id de session distinct, démarrage ok).
|
// Codex : routé vers l'adapter Codex (id de session distinct, démarrage ok).
|
||||||
let fake_cx = FakeCli::printing(&codex_script());
|
let fake_cx = FakeCli::printing(&codex_script());
|
||||||
let codex = structured_profile(StructuredAdapter::Codex, &fake_cx.command());
|
let codex = structured_profile(StructuredAdapter::Codex, &fake_cx.command());
|
||||||
|
let codex_policy = StructuredProviderLaunchPolicy::Codex {
|
||||||
|
sandbox_mode: "read-only".to_owned(),
|
||||||
|
writable_roots: Vec::new(),
|
||||||
|
network_access: false,
|
||||||
|
};
|
||||||
let session_cx = factory
|
let session_cx = factory
|
||||||
.start(
|
.start(
|
||||||
&codex,
|
&codex,
|
||||||
@ -598,7 +603,7 @@ mod tests {
|
|||||||
None,
|
None,
|
||||||
&[],
|
&[],
|
||||||
None,
|
None,
|
||||||
None,
|
Some(&codex_policy),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.expect("start Codex ok");
|
.expect("start Codex ok");
|
||||||
@ -647,7 +652,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn factory_does_not_pass_project_root_to_codex_add_dir_without_policy() {
|
async fn factory_rejects_codex_launch_without_explicit_policy() {
|
||||||
let factory = StructuredSessionFactory::new();
|
let factory = StructuredSessionFactory::new();
|
||||||
let (cmd, argv) = make_recording_fake(&[
|
let (cmd, argv) = make_recording_fake(&[
|
||||||
r#"{"type":"thread.started","thread_id":"cx-new"}"#,
|
r#"{"type":"thread.started","thread_id":"cx-new"}"#,
|
||||||
@ -660,7 +665,7 @@ mod tests {
|
|||||||
project_root: "/project/root".to_owned(),
|
project_root: "/project/root".to_owned(),
|
||||||
};
|
};
|
||||||
|
|
||||||
let session = factory
|
let err = match factory
|
||||||
.start(
|
.start(
|
||||||
&codex,
|
&codex,
|
||||||
&ctx,
|
&ctx,
|
||||||
@ -672,23 +677,18 @@ mod tests {
|
|||||||
None,
|
None,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.expect("start Codex ok");
|
{
|
||||||
let content = drain_final(session.as_ref()).await;
|
Ok(_) => panic!("Codex launch without a policy must fail closed"),
|
||||||
assert_eq!(content, "ok");
|
Err(err) => err,
|
||||||
|
};
|
||||||
let recorded = std::fs::read_to_string(&argv).expect("argv");
|
|
||||||
let args: Vec<&str> = recorded.lines().collect();
|
|
||||||
assert!(
|
assert!(
|
||||||
!args.windows(2).any(|w| w == ["--add-dir", "/project/root"]),
|
err.to_string()
|
||||||
"factory must not invent --add-dir project root without an explicit policy, got: {args:?}"
|
.contains("requires an explicit launch policy"),
|
||||||
|
"unexpected error: {err}"
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
!args.contains(&"--add-dir"),
|
!argv.exists(),
|
||||||
"default Codex adapter launch must not carry any implicit --add-dir, got: {args:?}"
|
"factory must reject the launch before spawning Codex"
|
||||||
);
|
|
||||||
assert!(
|
|
||||||
!args.contains(&"--ask-for-approval"),
|
|
||||||
"codex exec must not receive unsupported approval flags, got: {args:?}"
|
|
||||||
);
|
);
|
||||||
|
|
||||||
let _ = std::fs::remove_file(&cmd);
|
let _ = std::fs::remove_file(&cmd);
|
||||||
@ -710,6 +710,11 @@ mod tests {
|
|||||||
relative_path: "AGENTS.md".to_owned(),
|
relative_path: "AGENTS.md".to_owned(),
|
||||||
project_root: "/project/root".to_owned(),
|
project_root: "/project/root".to_owned(),
|
||||||
};
|
};
|
||||||
|
let policy = StructuredProviderLaunchPolicy::Codex {
|
||||||
|
sandbox_mode: "workspace-write".to_owned(),
|
||||||
|
writable_roots: vec!["/project/root".to_owned()],
|
||||||
|
network_access: false,
|
||||||
|
};
|
||||||
|
|
||||||
let session = factory
|
let session = factory
|
||||||
.start(
|
.start(
|
||||||
@ -720,7 +725,7 @@ mod tests {
|
|||||||
None,
|
None,
|
||||||
&[],
|
&[],
|
||||||
None,
|
None,
|
||||||
None,
|
Some(&policy),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.expect("start Codex ok");
|
.expect("start Codex ok");
|
||||||
@ -1803,6 +1808,11 @@ mod tests {
|
|||||||
r#"{"type":"item.completed","item":{"id":"i0","type":"agent_message","text":"ok"}}"#,
|
r#"{"type":"item.completed","item":{"id":"i0","type":"agent_message","text":"ok"}}"#,
|
||||||
]);
|
]);
|
||||||
let codex = structured_profile(StructuredAdapter::Codex, &fake.command());
|
let codex = structured_profile(StructuredAdapter::Codex, &fake.command());
|
||||||
|
let policy = StructuredProviderLaunchPolicy::Codex {
|
||||||
|
sandbox_mode: "read-only".to_owned(),
|
||||||
|
writable_roots: Vec::new(),
|
||||||
|
network_access: false,
|
||||||
|
};
|
||||||
let session = factory
|
let session = factory
|
||||||
.start(
|
.start(
|
||||||
&codex,
|
&codex,
|
||||||
@ -1814,7 +1824,7 @@ mod tests {
|
|||||||
None,
|
None,
|
||||||
&[],
|
&[],
|
||||||
None,
|
None,
|
||||||
None,
|
Some(&policy),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.expect("start resume codex");
|
.expect("start resume codex");
|
||||||
@ -2102,7 +2112,7 @@ mod tests {
|
|||||||
cmd.clone(),
|
cmd.clone(),
|
||||||
"/",
|
"/",
|
||||||
Some("cx-id".to_owned()),
|
Some("cx-id".to_owned()),
|
||||||
vec!["/project/root".to_owned()],
|
vec!["/project/root".to_owned(), "/managed/tmp".to_owned()],
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
None,
|
None,
|
||||||
None,
|
None,
|
||||||
@ -2121,6 +2131,8 @@ mod tests {
|
|||||||
"workspace-write",
|
"workspace-write",
|
||||||
"--add-dir",
|
"--add-dir",
|
||||||
"/project/root",
|
"/project/root",
|
||||||
|
"--add-dir",
|
||||||
|
"/managed/tmp",
|
||||||
"resume",
|
"resume",
|
||||||
"cx-id",
|
"cx-id",
|
||||||
"vas-y",
|
"vas-y",
|
||||||
|
|||||||
Reference in New Issue
Block a user