diff --git a/src/config/mcp_factory.rs b/src/config/mcp_factory.rs index aaa4b14..5153d68 100644 --- a/src/config/mcp_factory.rs +++ b/src/config/mcp_factory.rs @@ -100,7 +100,9 @@ impl McpFactory { // lock section so a racing first connect that inserts between them // cannot be misreported as a reconnect. The spawn await below stays // outside the lock, so two racing first connects may still both - // spawn (no singleflight); the loser then reports a reconnect. + // spawn (no singleflight); the loser then fires a second plain + // `connected` (no RECONNECT flag) and its insert overwrites the + // winner's map entry. let reconnect = { let map = self.active.lock(); if let Some(existing) = map.get(&key).and_then(|weak| weak.upgrade()) { diff --git a/src/config/request_context.rs b/src/config/request_context.rs index ae47738..271fd0d 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -8427,6 +8427,7 @@ mod tests { ctx.update_app_config(|app| { app.hooks = hooks_map_of(&["session.started", "session.resumed"], marker) }); + ctx.role = Some(Role::new("started-role", "Prompt")); let app = ctx.app.config.clone(); let abort = utils::create_abort_signal(); run_async(ctx.use_session(&app, Some("hook-fresh"), abort.clone())).unwrap(); @@ -8441,10 +8442,21 @@ mod tests { .map(String::as_str), Some("hook-fresh") ); + assert_eq!( + captures[0].envs.get("COYOTE_ROLE").map(String::as_str), + Some("started-role"), + "session.started must carry the active role" + ); // Resuming a persisted session fires session.resumed, never - // session.started. - write_paused_todo_session(&ctx, "hook-resume"); + // session.started, and carries the role the session holds. + let session_path = ctx.session_file("hook-resume"); + ensure_parent_exists(&session_path).unwrap(); + write( + &session_path, + "model: test-seeded:test-chat\nrole_name: resumed-role\nmessages: []\n", + ) + .unwrap(); let mut resumed_ctx = create_test_ctx(); resumed_ctx.update_app_config(|app| { app.hooks = hooks_map_of(&["session.started", "session.resumed"], marker) @@ -8466,6 +8478,11 @@ mod tests { resumed[0].envs.get("COYOTE_SESSION_ID").map(String::as_str), Some("hook-resume") ); + assert_eq!( + resumed[0].envs.get("COYOTE_ROLE").map(String::as_str), + Some("resumed-role"), + "session.resumed must carry the session-held role" + ); } #[test] @@ -8482,14 +8499,22 @@ mod tests { ctx.exit_session().unwrap(); assert!(marker_captures(marker).is_empty()); - ctx.session = Some(Session::default()); + let mut session = Session::default(); + session.set_role(Role::new("ended-role", "Prompt")); + ctx.session = Some(session); ctx.exit_session().unwrap(); ctx.exit_session().unwrap(); + let captures = marker_captures(marker); assert_eq!( - marker_captures(marker).len(), + captures.len(), 1, "session.ended fires exactly once, only while a session is attached" ); + assert_eq!( + captures[0].envs.get("COYOTE_ROLE").map(String::as_str), + Some("ended-role"), + "session.ended must fire while the session-held role is still attached" + ); } #[test] diff --git a/src/hooks.rs b/src/hooks.rs index 7c76a0a..eaa5c65 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -714,7 +714,8 @@ pub(crate) fn base_envs_parts( /// only two families that carry the variable. Resolved at fire time, because /// roles change mid-session (`.role`, `.exit role`, temp roles): the role /// held directly on the context wins, then the name of a role a session has -/// absorbed. Derived roles have no name and report nothing. +/// absorbed. Derived roles have no name; resolution then falls through to +/// the session-held role, if any. pub fn role_extras(ctx: &RequestContext) -> Vec<(&'static str, String)> { ctx.role .as_ref() diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 4e46b59..9751a5a 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -2216,6 +2216,59 @@ mod tests { } } + #[tokio::test] + #[serial] + async fn turn_interrupted_carries_the_role_a_session_holds() { + let _sink = test_sink::install(); + let marker = "tb-role-int-f7c"; + let mut ctx = ctx_with_hooks(&TURN_EVENTS, marker); + let mut session = Session::default(); + session.set_role(Role::new("dev-role", "Prompt")); + ctx.session = Some(session); + let abort_signal = create_abort_signal(); + abort_signal.set_ctrlc(); + + let bracket = TurnBracket::enter(&ctx); + let result: Result<()> = Ok(()); + bracket.finish(&ctx, &abort_signal, result).unwrap(); + + let interrupted = test_sink::snapshot() + .into_iter() + .find(|capture| capture.hook_name == format!("{marker}-turn.interrupted")) + .expect("turn.interrupted capture"); + assert_eq!( + interrupted.envs.get("COYOTE_ROLE").map(String::as_str), + Some("dev-role"), + "turn.interrupted must carry the session-held role" + ); + } + + #[tokio::test] + #[serial] + async fn turn_failed_carries_the_role_a_session_holds() { + let _sink = test_sink::install(); + let marker = "tb-role-fail-z2n"; + let mut ctx = ctx_with_hooks(&TURN_EVENTS, marker); + let mut session = Session::default(); + session.set_role(Role::new("dev-role", "Prompt")); + ctx.session = Some(session); + let abort_signal = create_abort_signal(); + + let bracket = TurnBracket::enter(&ctx); + let result: Result<()> = Err(anyhow!("turn exploded")); + bracket.finish(&ctx, &abort_signal, result).unwrap_err(); + + let failed = test_sink::snapshot() + .into_iter() + .find(|capture| capture.hook_name == format!("{marker}-turn.failed")) + .expect("turn.failed capture"); + assert_eq!( + failed.envs.get("COYOTE_ROLE").map(String::as_str), + Some("dev-role"), + "turn.failed must carry the session-held role" + ); + } + #[tokio::test] #[serial] async fn turn_events_omit_the_role_var_when_no_role_is_active() {