fix(maestro): replace agent_session_id when a report names a new one
A resume issues a new conversation id, so write-once left the stored id pointing at the pre-resume conversation and every later resume reattached it. Rewrite the sidecar when the reported id differs from the stored one; a repeated report of the same id still writes nothing. Closes #176
This commit is contained in:
@@ -1305,10 +1305,12 @@ impl SessionManager {
|
||||
/// and stays silent on the event stream at steady state.
|
||||
///
|
||||
/// `agent_session_id` (#161) is the agent's own conversation id, parsed
|
||||
/// by the caller from the hook payload's `session_id` field. It is
|
||||
/// written once — the first report that carries one wins — and the
|
||||
/// sidecar is only rewritten on that transition, never on every report,
|
||||
/// since an agent's session id never changes mid-session.
|
||||
/// by the caller from the hook payload's `session_id` field. The
|
||||
/// sidecar is rewritten only when the reported id differs from the
|
||||
/// stored one — a repeated report of the same id is pure churn from the
|
||||
/// hook's `PreToolUse` cadence and writes nothing, but a resume (#176)
|
||||
/// issues a new conversation id and must replace the stale one, or
|
||||
/// every later resume reattaches the pre-resume conversation.
|
||||
pub async fn report_activity(
|
||||
&self,
|
||||
id: Uuid,
|
||||
@@ -1330,16 +1332,16 @@ impl SessionManager {
|
||||
state,
|
||||
self.inner.report_ttl,
|
||||
));
|
||||
let newly_identified =
|
||||
rec.session.agent_session_id.is_none() && agent_session_id.is_some();
|
||||
if newly_identified {
|
||||
let id_changed =
|
||||
agent_session_id.is_some() && rec.session.agent_session_id != agent_session_id;
|
||||
if id_changed {
|
||||
rec.session.agent_session_id = agent_session_id;
|
||||
self.inner
|
||||
.sidecar
|
||||
.write(&rec.session)
|
||||
.map_err(SessionError::Sidecar)?;
|
||||
}
|
||||
(changed || newly_identified).then(|| rec.session.clone())
|
||||
(changed || id_changed).then(|| rec.session.clone())
|
||||
};
|
||||
if let Some(session) = updated {
|
||||
let _ = self
|
||||
|
||||
@@ -187,10 +187,30 @@ async fn a_reported_session_id_is_recorded_on_the_session() {
|
||||
assert_eq!(s.agent_session_id.as_deref(), Some("conv-abc123"));
|
||||
}
|
||||
|
||||
/// #161: a session id does not change mid-session, so once set it is never
|
||||
/// overwritten by a later report — even one that names a different id.
|
||||
/// #161: repeated reports of the same id are pure churn from the hook's
|
||||
/// `PreToolUse` cadence — the sidecar must not be rewritten for them.
|
||||
#[tokio::test]
|
||||
async fn a_reported_session_id_is_written_once_and_never_replaced() {
|
||||
async fn a_repeated_report_of_the_same_session_id_writes_nothing() {
|
||||
let fx = fixture(Duration::from_secs(300)).await;
|
||||
let id = spawn_sleeping(&fx.manager).await;
|
||||
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Active, None, Some("conv-first".into()))
|
||||
.await
|
||||
.unwrap();
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Active, None, Some("conv-first".into()))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let s = observed(&fx.manager, id).await;
|
||||
assert_eq!(s.agent_session_id.as_deref(), Some("conv-first"));
|
||||
}
|
||||
|
||||
/// #176: a resume issues a new conversation id. The stored id must move to
|
||||
/// it, or every later resume reattaches the stale pre-resume conversation.
|
||||
#[tokio::test]
|
||||
async fn a_reported_session_id_is_replaced_when_it_differs() {
|
||||
let fx = fixture(Duration::from_secs(300)).await;
|
||||
let id = spawn_sleeping(&fx.manager).await;
|
||||
|
||||
@@ -204,7 +224,14 @@ async fn a_reported_session_id_is_written_once_and_never_replaced() {
|
||||
.unwrap();
|
||||
|
||||
let s = observed(&fx.manager, id).await;
|
||||
assert_eq!(s.agent_session_id.as_deref(), Some("conv-first"));
|
||||
assert_eq!(s.agent_session_id.as_deref(), Some("conv-second"));
|
||||
|
||||
let stored = fx
|
||||
.sidecar
|
||||
.read(id)
|
||||
.expect("sidecar read")
|
||||
.expect("sidecar present");
|
||||
assert_eq!(stored.agent_session_id.as_deref(), Some("conv-second"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user