Skip to content

Commit d0d4979

Browse files
authored
Merge pull request #199 from cbusillo/fix/session-event-scope-audit
Scope background activity to the owning session
2 parents 7782ec5 + dc4e2d7 commit d0d4979

3 files changed

Lines changed: 184 additions & 23 deletions

File tree

code-rs/core/src/agent_tool.rs

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,12 @@ impl AgentManager {
11261126
.or_else(|| self.archived_terminal_agents.get(agent_id).cloned())
11271127
}
11281128

1129+
pub fn get_agent_for_session(&self, agent_id: &str, owner_session_id: Uuid) -> Option<Agent> {
1130+
self
1131+
.get_agent(agent_id)
1132+
.filter(|agent| agent_belongs_to_session(agent, Some(owner_session_id)))
1133+
}
1134+
11291135
pub fn get_all_agents(&self) -> impl Iterator<Item = &Agent> {
11301136
self.agents.values()
11311137
}
@@ -1176,6 +1182,20 @@ impl AgentManager {
11761182
out
11771183
}
11781184

1185+
pub fn list_agents_for_session(
1186+
&self,
1187+
status_filter: Option<AgentStatus>,
1188+
batch_id: Option<String>,
1189+
recent_only: bool,
1190+
owner_session_id: Uuid,
1191+
) -> Vec<Agent> {
1192+
self
1193+
.list_agents(status_filter, batch_id, recent_only)
1194+
.into_iter()
1195+
.filter(|agent| agent_belongs_to_session(agent, Some(owner_session_id)))
1196+
.collect()
1197+
}
1198+
11791199
pub fn has_active_agents(&self) -> bool {
11801200
self.agents
11811201
.values()
@@ -1196,6 +1216,21 @@ impl AgentManager {
11961216
}
11971217
}
11981218

1219+
pub async fn cancel_agent_for_session(
1220+
&mut self,
1221+
agent_id: &str,
1222+
owner_session_id: Uuid,
1223+
) -> bool {
1224+
if self
1225+
.get_agent(agent_id)
1226+
.is_some_and(|agent| agent_belongs_to_session(&agent, Some(owner_session_id)))
1227+
{
1228+
self.cancel_agent(agent_id).await
1229+
} else {
1230+
false
1231+
}
1232+
}
1233+
11991234
pub async fn cancel_batch(&mut self, batch_id: &str) -> usize {
12001235
let agent_ids: Vec<String> = self
12011236
.agents
@@ -1213,6 +1248,28 @@ impl AgentManager {
12131248
count
12141249
}
12151250

1251+
pub async fn cancel_batch_for_session(
1252+
&mut self,
1253+
batch_id: &str,
1254+
owner_session_id: Uuid,
1255+
) -> usize {
1256+
let agent_ids: Vec<String> = self
1257+
.agents
1258+
.values()
1259+
.filter(|agent| agent.batch_id.as_ref() == Some(&batch_id.to_string()))
1260+
.filter(|agent| agent_belongs_to_session(agent, Some(owner_session_id)))
1261+
.map(|agent| agent.id.clone())
1262+
.collect();
1263+
1264+
let mut count = 0;
1265+
for agent_id in agent_ids {
1266+
if self.cancel_agent(&agent_id).await {
1267+
count += 1;
1268+
}
1269+
}
1270+
count
1271+
}
1272+
12161273
pub async fn update_agent_status(&mut self, agent_id: &str, status: AgentStatus) {
12171274
let mut terminal = false;
12181275
if let Some(agent) = self.agents.get_mut(agent_id) {
@@ -3026,6 +3083,42 @@ mod tests {
30263083
}
30273084
}
30283085

3086+
fn test_agent(
3087+
id: &str,
3088+
owner_session_id: Uuid,
3089+
batch_id: &str,
3090+
status: AgentStatus,
3091+
) -> Agent {
3092+
let now = chrono::Utc::now();
3093+
Agent {
3094+
id: id.to_string(),
3095+
owner_session_id: Some(owner_session_id),
3096+
batch_id: Some(batch_id.to_string()),
3097+
model: "code-gpt-5.5".to_string(),
3098+
name: Some(id.to_string()),
3099+
prompt: "prompt".to_string(),
3100+
context: None,
3101+
output_goal: None,
3102+
files: Vec::new(),
3103+
read_only: true,
3104+
status,
3105+
result: None,
3106+
error: None,
3107+
created_at: now,
3108+
started_at: Some(now),
3109+
completed_at: None,
3110+
progress: Vec::new(),
3111+
worktree_path: None,
3112+
branch_name: None,
3113+
worktree_base: None,
3114+
source_kind: None,
3115+
log_tag: None,
3116+
config: None,
3117+
reasoning_effort: ReasoningEffort::Low,
3118+
last_activity: now,
3119+
}
3120+
}
3121+
30293122
#[test]
30303123
fn code_family_falls_back_when_command_missing() {
30313124
let cfg = agent_with_command("definitely-not-present-429");
@@ -3134,6 +3227,52 @@ mod tests {
31343227
assert!(!payload_b.agents.iter().any(|agent| agent.id == agent_a));
31353228
}
31363229

3230+
#[tokio::test]
3231+
async fn model_facing_agent_queries_are_session_scoped() {
3232+
let mut manager = AgentManager::new();
3233+
let session_a = Uuid::new_v4();
3234+
let session_b = Uuid::new_v4();
3235+
let shared_batch = "shared-batch";
3236+
3237+
manager.agents.insert(
3238+
"agent-a".to_string(),
3239+
test_agent("agent-a", session_a, shared_batch, AgentStatus::Running),
3240+
);
3241+
manager.agents.insert(
3242+
"agent-b".to_string(),
3243+
test_agent("agent-b", session_b, shared_batch, AgentStatus::Running),
3244+
);
3245+
manager.handles.insert("agent-a".to_string(), tokio::spawn(async {}));
3246+
manager.handles.insert("agent-b".to_string(), tokio::spawn(async {}));
3247+
manager.archived_terminal_agents.insert(
3248+
"archived-b".to_string(),
3249+
test_agent("archived-b", session_b, shared_batch, AgentStatus::Completed),
3250+
);
3251+
3252+
let visible_to_a = manager.list_agents_for_session(
3253+
None,
3254+
Some(shared_batch.to_string()),
3255+
false,
3256+
session_a,
3257+
);
3258+
assert_eq!(visible_to_a.len(), 1);
3259+
assert_eq!(visible_to_a[0].id, "agent-a");
3260+
assert!(manager.get_agent_for_session("agent-b", session_a).is_none());
3261+
assert!(manager.get_agent_for_session("archived-b", session_a).is_none());
3262+
assert!(!manager.cancel_agent_for_session("agent-b", session_a).await);
3263+
3264+
assert!(manager.agents.contains_key("agent-b"));
3265+
assert!(manager.cancel_batch_for_session(shared_batch, session_a).await == 1);
3266+
assert_eq!(
3267+
manager.agents.get("agent-a").map(|agent| &agent.status),
3268+
Some(&AgentStatus::Cancelled),
3269+
);
3270+
assert_eq!(
3271+
manager.agents.get("agent-b").map(|agent| &agent.status),
3272+
Some(&AgentStatus::Running),
3273+
);
3274+
}
3275+
31373276
#[tokio::test]
31383277
async fn agent_status_updates_prune_closed_session_senders() {
31393278
let mut manager = AgentManager::new();

code-rs/core/src/codex.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,10 +1171,6 @@ pub struct Codex {
11711171
rx_event: Receiver<Event>,
11721172
}
11731173

1174-
// Allow internal components (like background exec completions) to trigger a new
1175-
// turn without fabricating a visible user message. We enqueue an empty
1176-
// UserInput; the model will only see queued developer/system items.
1177-
static TX_SUB_GLOBAL: OnceLock<Sender<Submission>> = OnceLock::new();
11781174
static ANY_BG_NOTIFY: OnceLock<std::sync::Arc<Notify>> = OnceLock::new();
11791175

11801176
/// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`],
@@ -1247,8 +1243,6 @@ impl Codex {
12471243
tx_sub,
12481244
rx_event,
12491245
};
1250-
// Make a clone of tx_sub available for internal auto-turn triggers.
1251-
let _ = TX_SUB_GLOBAL.set(codex.tx_sub.clone());
12521246
let _ = ANY_BG_NOTIFY.set(std::sync::Arc::new(Notify::new()));
12531247
let init_id = codex.submit(configure_session).await?;
12541248

code-rs/core/src/codex/streaming.rs

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,9 @@ pub(super) async fn submission_loop(
277277
if !seen_batches.insert(trimmed.to_string()) {
278278
continue;
279279
}
280-
cancelled += manager.cancel_batch(trimmed).await;
280+
cancelled += manager
281+
.cancel_batch_for_session(trimmed, sess_arc.session_uuid())
282+
.await;
281283
}
282284

283285
for agent_id in agent_ids {
@@ -288,7 +290,10 @@ pub(super) async fn submission_loop(
288290
if !seen_agents.insert(trimmed.to_string()) {
289291
continue;
290292
}
291-
if manager.cancel_agent(trimmed).await {
293+
if manager
294+
.cancel_agent_for_session(trimmed, sess_arc.session_uuid())
295+
.await
296+
{
292297
cancelled += 1;
293298
}
294299
}
@@ -8911,7 +8916,7 @@ async fn handle_check_agent_status(
89118916
Ok(params) => {
89128917
let manager = AGENT_MANAGER.read().await;
89138918

8914-
if let Some(agent) = manager.get_agent(&params.agent_id) {
8919+
if let Some(agent) = manager.get_agent_for_session(&params.agent_id, sess.session_uuid()) {
89158920
match agent.batch_id.as_deref() {
89168921
Some(batch) if batch == params.batch_id => {}
89178922
_ => {
@@ -8958,7 +8963,7 @@ async fn handle_check_agent_status(
89588963
};
89598964
// Re-acquire manager to get fresh progress after potential delay
89608965
let manager = AGENT_MANAGER.read().await;
8961-
if let Some(agent) = manager.get_agent(&params.agent_id) {
8966+
if let Some(agent) = manager.get_agent_for_session(&params.agent_id, sess.session_uuid()) {
89628967
let joined = agent.progress.join("\n");
89638968
match write_agent_file(&dir, "progress.log", &joined) {
89648969
Ok(p) => progress_file = Some(p.display().to_string()),
@@ -9037,7 +9042,7 @@ async fn handle_get_agent_result(
90379042
Ok(params) => {
90389043
let manager = AGENT_MANAGER.read().await;
90399044

9040-
if let Some(agent) = manager.get_agent(&params.agent_id) {
9045+
if let Some(agent) = manager.get_agent_for_session(&params.agent_id, sess.session_uuid()) {
90419046
match agent.batch_id.as_deref() {
90429047
Some(batch) if batch == params.batch_id => {}
90439048
_ => {
@@ -9171,7 +9176,7 @@ async fn handle_cancel_agent(
91719176
};
91729177
}
91739178
};
9174-
if let Some(agent) = manager.get_agent(&agent_id) {
9179+
if let Some(agent) = manager.get_agent_for_session(&agent_id, sess.session_uuid()) {
91759180
if agent.batch_id.as_deref() != Some(batch_id.as_str()) {
91769181
return ResponseInputItem::FunctionCallOutput {
91779182
call_id: call_id_clone,
@@ -9184,7 +9189,7 @@ async fn handle_cancel_agent(
91849189
};
91859190
}
91869191
}
9187-
if manager.cancel_agent(&agent_id).await {
9192+
if manager.cancel_agent_for_session(&agent_id, sess.session_uuid()).await {
91889193
ResponseInputItem::FunctionCallOutput {
91899194
call_id: call_id_clone,
91909195
output: FunctionCallOutputPayload {
@@ -9200,7 +9205,7 @@ async fn handle_cancel_agent(
92009205
}
92019206
}
92029207
} else if let Some(batch_id) = params.batch_id {
9203-
let count = manager.cancel_batch(&batch_id).await;
9208+
let count = manager.cancel_batch_for_session(&batch_id, sess.session_uuid()).await;
92049209
ResponseInputItem::FunctionCallOutput {
92059210
call_id: call_id_clone,
92069211
output: FunctionCallOutputPayload {
@@ -9273,7 +9278,7 @@ async fn handle_wait_for_agent(
92739278
let manager = AGENT_MANAGER.read().await;
92749279

92759280
if let Some(agent_id) = &params.agent_id {
9276-
if let Some(agent) = manager.get_agent(agent_id) {
9281+
if let Some(agent) = manager.get_agent_for_session(agent_id, sess.session_uuid()) {
92779282
match agent.batch_id.as_deref() {
92789283
Some(batch) if batch == batch_id => {}
92799284
_ => {
@@ -9362,7 +9367,7 @@ async fn handle_wait_for_agent(
93629367
}
93639368
}
93649369
} else {
9365-
let agents = manager.list_agents(None, Some(batch_id.clone()), false);
9370+
let agents = manager.list_agents_for_session(None, Some(batch_id.clone()), false, sess.session_uuid());
93669371

93679372
// Separate terminal vs non-terminal agents
93689373
let completed_agents: Vec<_> = agents
@@ -9678,10 +9683,11 @@ async fn handle_list_agents(
96789683
_ => None,
96799684
});
96809685

9681-
let agents = manager.list_agents(
9686+
let agents = manager.list_agents_for_session(
96829687
status_filter,
96839688
Some(batch_id.clone()),
96849689
params.recent_only.unwrap_or(false),
9690+
sess.session_uuid(),
96859691
);
96869692

96879693
// Count running agents for status update
@@ -10719,6 +10725,7 @@ async fn handle_container_exec_with_params(
1071910725
let suppress_event_flag_task = suppress_event_flag.clone();
1072010726
let display_label_task = display_label.clone();
1072110727
let tool_output_max_bytes = sess.tool_output_max_bytes;
10728+
let sess_for_background_completion = sess.self_handle.upgrade();
1072210729
let task_handle = tokio::spawn(async move {
1072310730
// Build stdout stream with tail capture. We cannot stamp via `Session` here,
1072410731
// but deltas will be delivered with neutral ordering which the UI tolerates.
@@ -10809,10 +10816,10 @@ async fn handle_container_exec_with_params(
1080910816
format!("{label} completed in background")
1081010817
};
1081110818
let bg_event = EventMsg::BackgroundEvent(BackgroundEventEvent { message });
10812-
let ev = Event { id: sub_id_for_events.clone(), event_seq: 0, msg: bg_event, order: None };
10813-
let _ = tx_event.send(ev).await;
10819+
if let Some(sess_arc) = sess_for_background_completion.as_ref() {
10820+
let ev = sess_arc.make_event(&sub_id_for_events, bg_event);
10821+
sess_arc.send_event(ev).await;
1081410822

10815-
if let Some(tx) = TX_SUB_GLOBAL.get() {
1081610823
let header_label = if label.is_empty() {
1081710824
format!("call_id={}", call_id_for_events)
1081810825
} else {
@@ -10828,9 +10835,30 @@ async fn handle_container_exec_with_params(
1082810835
tool_output_max_bytes,
1082910836
);
1083010837
let dev_text = format!("{}\n\n{}", header, body);
10831-
let _ = tx
10832-
.send(Submission { id: uuid::Uuid::new_v4().to_string(), op: Op::AddPendingInputDeveloper { text: dev_text } })
10833-
.await;
10838+
let dev_msg = ResponseInputItem::Message {
10839+
role: "developer".to_string(),
10840+
content: vec![ContentItem::InputText { text: dev_text }],
10841+
};
10842+
if sess_arc.enqueue_out_of_turn_item(dev_msg) {
10843+
sess_arc.cleanup_old_status_items().await;
10844+
let turn_context = sess_arc.make_turn_context();
10845+
let sub_id = sess_arc.next_internal_sub_id();
10846+
let sentinel_input = vec![InputItem::Text {
10847+
text: PENDING_ONLY_SENTINEL.to_string(),
10848+
}];
10849+
let agent = AgentTask::spawn(
10850+
Arc::clone(sess_arc),
10851+
turn_context,
10852+
sub_id,
10853+
sentinel_input,
10854+
TaskOriginKind::OutOfTurnDeveloper,
10855+
false,
10856+
);
10857+
sess_arc.set_task(agent);
10858+
}
10859+
} else {
10860+
let ev = Event { id: sub_id_for_events.clone(), event_seq: 0, msg: bg_event, order: None };
10861+
let _ = tx_event.send(ev).await;
1083410862
}
1083510863
}
1083610864
if let Some(n) = ANY_BG_NOTIFY.get() { n.notify_waiters(); }

0 commit comments

Comments
 (0)