feat(TASK-061): assert COYOTE_ROLE on all session/turn events; fix two stale comments
Cover the remaining role-carrying events with test_sink assertions: session.started (ctx-held role), session.resumed and session.ended (session-held role), turn.interrupted and turn.failed. Correct the mcp_factory race comment (the loser fires a second plain connected and overwrites the winner's map entry, not a reconnect) and the role_extras doc (derived roles fall through to the session-held role).
This commit is contained in:
4 files changed
+87
-6
No files matched your search
@@ -100,7 +100,9 @@ impl McpFactory {
|
|||||||
// lock section so a racing first connect that inserts between them
|
// lock section so a racing first connect that inserts between them
|
||||||
// cannot be misreported as a reconnect. The spawn await below stays
|
// cannot be misreported as a reconnect. The spawn await below stays
|
||||||
// outside the lock, so two racing first connects may still both
|
// 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 reconnect = {
|
||||||
let map = self.active.lock();
|
let map = self.active.lock();
|
||||||
if let Some(existing) = map.get(&key).and_then(|weak| weak.upgrade()) {
|
if let Some(existing) = map.get(&key).and_then(|weak| weak.upgrade()) {
|
||||||
|
|||||||
@@ -8427,6 +8427,7 @@ mod tests {
|
|||||||
ctx.update_app_config(|app| {
|
ctx.update_app_config(|app| {
|
||||||
app.hooks = hooks_map_of(&["session.started", "session.resumed"], marker)
|
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 app = ctx.app.config.clone();
|
||||||
let abort = utils::create_abort_signal();
|
let abort = utils::create_abort_signal();
|
||||||
run_async(ctx.use_session(&app, Some("hook-fresh"), abort.clone())).unwrap();
|
run_async(ctx.use_session(&app, Some("hook-fresh"), abort.clone())).unwrap();
|
||||||
@@ -8441,10 +8442,21 @@ mod tests {
|
|||||||
.map(String::as_str),
|
.map(String::as_str),
|
||||||
Some("hook-fresh")
|
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
|
// Resuming a persisted session fires session.resumed, never
|
||||||
// session.started.
|
// session.started, and carries the role the session holds.
|
||||||
write_paused_todo_session(&ctx, "hook-resume");
|
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();
|
let mut resumed_ctx = create_test_ctx();
|
||||||
resumed_ctx.update_app_config(|app| {
|
resumed_ctx.update_app_config(|app| {
|
||||||
app.hooks = hooks_map_of(&["session.started", "session.resumed"], marker)
|
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),
|
resumed[0].envs.get("COYOTE_SESSION_ID").map(String::as_str),
|
||||||
Some("hook-resume")
|
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]
|
#[test]
|
||||||
@@ -8482,14 +8499,22 @@ mod tests {
|
|||||||
ctx.exit_session().unwrap();
|
ctx.exit_session().unwrap();
|
||||||
assert!(marker_captures(marker).is_empty());
|
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();
|
||||||
ctx.exit_session().unwrap();
|
ctx.exit_session().unwrap();
|
||||||
|
let captures = marker_captures(marker);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
marker_captures(marker).len(),
|
captures.len(),
|
||||||
1,
|
1,
|
||||||
"session.ended fires exactly once, only while a session is attached"
|
"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]
|
#[test]
|
||||||
|
|||||||
+2
-1
@@ -714,7 +714,8 @@ pub(crate) fn base_envs_parts(
|
|||||||
/// only two families that carry the variable. Resolved at fire time, because
|
/// only two families that carry the variable. Resolved at fire time, because
|
||||||
/// roles change mid-session (`.role`, `.exit role`, temp roles): the role
|
/// 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
|
/// 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)> {
|
pub fn role_extras(ctx: &RequestContext) -> Vec<(&'static str, String)> {
|
||||||
ctx.role
|
ctx.role
|
||||||
.as_ref()
|
.as_ref()
|
||||||
|
|||||||
@@ -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]
|
#[tokio::test]
|
||||||
#[serial]
|
#[serial]
|
||||||
async fn turn_events_omit_the_role_var_when_no_role_is_active() {
|
async fn turn_events_omit_the_role_var_when_no_role_is_active() {
|
||||||
|
|||||||
Reference in new issue
Block a user