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)>>>>,
|
||||
sandboxes: Arc<Mutex<Vec<Option<domain::sandbox::SandboxPlan>>>>,
|
||||
policies: Arc<Mutex<Vec<Option<domain::ports::StructuredProviderLaunchPolicy>>>>,
|
||||
plans: Arc<Mutex<Vec<SessionPlan>>>,
|
||||
shutdowns: Arc<Mutex<Vec<SessionId>>>,
|
||||
next_session: SessionId,
|
||||
}
|
||||
@ -638,6 +639,7 @@ impl FakeStructuredFactory {
|
||||
envs: Arc::new(Mutex::new(Vec::new())),
|
||||
sandboxes: 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())),
|
||||
next_session,
|
||||
}
|
||||
@ -662,6 +664,10 @@ impl FakeStructuredFactory {
|
||||
fn policies(&self) -> Vec<Option<domain::ports::StructuredProviderLaunchPolicy>> {
|
||||
self.policies.lock().unwrap().clone()
|
||||
}
|
||||
|
||||
fn plans(&self) -> Vec<SessionPlan> {
|
||||
self.plans.lock().unwrap().clone()
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
@ -675,7 +681,7 @@ impl AgentSessionFactory for FakeStructuredFactory {
|
||||
profile: &AgentProfile,
|
||||
_ctx: &PreparedContext,
|
||||
_cwd: &ProjectPath,
|
||||
_session: &SessionPlan,
|
||||
session: &SessionPlan,
|
||||
_requester: Option<&str>,
|
||||
_env: &[(String, String)],
|
||||
_sandbox: Option<&domain::sandbox::SandboxPlan>,
|
||||
@ -696,6 +702,7 @@ impl AgentSessionFactory for FakeStructuredFactory {
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push(structured_policy.cloned());
|
||||
self.plans.lock().unwrap().push(session.clone());
|
||||
Ok(Arc::new(FakeSession {
|
||||
id: self.next_session,
|
||||
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]
|
||||
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 tmp = root.join("managed-tmp-main").to_string_lossy().into_owned();
|
||||
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(Arc::new(factory.clone()), Arc::clone(&structured));
|
||||
|
||||
launch
|
||||
.execute(launch_input_with_project_root(agent.id, &root))
|
||||
.await
|
||||
.expect("structured launch");
|
||||
let mut input = launch_input_with_project_root(agent.id, &root);
|
||||
input.require_structured = true;
|
||||
input.conversation_id = Some("pair-conversation-306".to_owned());
|
||||
launch.execute(input).await.expect("structured launch");
|
||||
|
||||
assert!(pty.spawns().is_empty());
|
||||
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 {
|
||||
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!(
|
||||
factory.sandboxes(),
|
||||
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)]
|
||||
mod self_restart_contract_tests {
|
||||
use super::*;
|
||||
@ -1137,6 +1159,42 @@ mod self_restart_contract_tests {
|
||||
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]
|
||||
async fn human_turn_ignores_old_terminal_and_accepts_replacement() {
|
||||
let mailbox = Arc::new(InMemoryMailbox::new());
|
||||
@ -1682,19 +1740,13 @@ impl AgentResumer for AppAgentResumer {
|
||||
|
||||
let project_id = ctx.project.id;
|
||||
self.launch_agent
|
||||
.execute(LaunchAgentInput {
|
||||
project: ctx.project,
|
||||
.execute(automatic_resume_launch_input(
|
||||
ctx,
|
||||
agent_id,
|
||||
rows: ctx.rows,
|
||||
cols: ctx.cols,
|
||||
node_id: Some(node_id),
|
||||
node_id,
|
||||
conversation_id,
|
||||
mcp_runtime,
|
||||
allow_structured_alongside_pty: false,
|
||||
require_structured: false,
|
||||
fresh_start: false,
|
||||
managed_temp_exposure: application::ManagedTempExposureRequest::default(),
|
||||
})
|
||||
))
|
||||
.await?;
|
||||
|
||||
// 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(),
|
||||
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(
|
||||
id,
|
||||
|
||||
@ -51,7 +51,7 @@ mod tests {
|
||||
use domain::ports::{
|
||||
AgentSession, AgentSessionError, AgentSessionFactory, ContextInjectionPlan,
|
||||
PreparedContext, ReplyEvent, ReplyProgressKind, ReplyProgressSource, ReplyProgressStage,
|
||||
SessionPlan,
|
||||
SessionPlan, StructuredProviderLaunchPolicy,
|
||||
};
|
||||
use domain::profile::{AgentProfile, ContextInjection, HttpChatConfig, StructuredAdapter};
|
||||
use domain::project::ProjectPath;
|
||||
@ -589,6 +589,11 @@ mod tests {
|
||||
// Codex : routé vers l'adapter Codex (id de session distinct, démarrage ok).
|
||||
let fake_cx = FakeCli::printing(&codex_script());
|
||||
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
|
||||
.start(
|
||||
&codex,
|
||||
@ -598,7 +603,7 @@ mod tests {
|
||||
None,
|
||||
&[],
|
||||
None,
|
||||
None,
|
||||
Some(&codex_policy),
|
||||
)
|
||||
.await
|
||||
.expect("start Codex ok");
|
||||
@ -647,7 +652,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[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 (cmd, argv) = make_recording_fake(&[
|
||||
r#"{"type":"thread.started","thread_id":"cx-new"}"#,
|
||||
@ -660,7 +665,7 @@ mod tests {
|
||||
project_root: "/project/root".to_owned(),
|
||||
};
|
||||
|
||||
let session = factory
|
||||
let err = match factory
|
||||
.start(
|
||||
&codex,
|
||||
&ctx,
|
||||
@ -672,23 +677,18 @@ mod tests {
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("start Codex ok");
|
||||
let content = drain_final(session.as_ref()).await;
|
||||
assert_eq!(content, "ok");
|
||||
|
||||
let recorded = std::fs::read_to_string(&argv).expect("argv");
|
||||
let args: Vec<&str> = recorded.lines().collect();
|
||||
{
|
||||
Ok(_) => panic!("Codex launch without a policy must fail closed"),
|
||||
Err(err) => err,
|
||||
};
|
||||
assert!(
|
||||
!args.windows(2).any(|w| w == ["--add-dir", "/project/root"]),
|
||||
"factory must not invent --add-dir project root without an explicit policy, got: {args:?}"
|
||||
err.to_string()
|
||||
.contains("requires an explicit launch policy"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
assert!(
|
||||
!args.contains(&"--add-dir"),
|
||||
"default Codex adapter launch must not carry any implicit --add-dir, got: {args:?}"
|
||||
);
|
||||
assert!(
|
||||
!args.contains(&"--ask-for-approval"),
|
||||
"codex exec must not receive unsupported approval flags, got: {args:?}"
|
||||
!argv.exists(),
|
||||
"factory must reject the launch before spawning Codex"
|
||||
);
|
||||
|
||||
let _ = std::fs::remove_file(&cmd);
|
||||
@ -710,6 +710,11 @@ mod tests {
|
||||
relative_path: "AGENTS.md".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
|
||||
.start(
|
||||
@ -720,7 +725,7 @@ mod tests {
|
||||
None,
|
||||
&[],
|
||||
None,
|
||||
None,
|
||||
Some(&policy),
|
||||
)
|
||||
.await
|
||||
.expect("start Codex ok");
|
||||
@ -1803,6 +1808,11 @@ mod tests {
|
||||
r#"{"type":"item.completed","item":{"id":"i0","type":"agent_message","text":"ok"}}"#,
|
||||
]);
|
||||
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
|
||||
.start(
|
||||
&codex,
|
||||
@ -1814,7 +1824,7 @@ mod tests {
|
||||
None,
|
||||
&[],
|
||||
None,
|
||||
None,
|
||||
Some(&policy),
|
||||
)
|
||||
.await
|
||||
.expect("start resume codex");
|
||||
@ -2102,7 +2112,7 @@ mod tests {
|
||||
cmd.clone(),
|
||||
"/",
|
||||
Some("cx-id".to_owned()),
|
||||
vec!["/project/root".to_owned()],
|
||||
vec!["/project/root".to_owned(), "/managed/tmp".to_owned()],
|
||||
Vec::new(),
|
||||
None,
|
||||
None,
|
||||
@ -2121,6 +2131,8 @@ mod tests {
|
||||
"workspace-write",
|
||||
"--add-dir",
|
||||
"/project/root",
|
||||
"--add-dir",
|
||||
"/managed/tmp",
|
||||
"resume",
|
||||
"cx-id",
|
||||
"vas-y",
|
||||
|
||||
Reference in New Issue
Block a user