From 87f1e73dec4b048752e6e10ed35fbbb1d7efe15d Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Fri, 4 Sep 2026 09:26:16 +0100 Subject: [PATCH] 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 --- crates/maestro-daemon/src/session.rs | 18 +++++----- .../maestro-daemon/tests/activity_report.rs | 35 ++++++++++++++++--- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index b58b020..ccd4afd 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -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 diff --git a/crates/maestro-daemon/tests/activity_report.rs b/crates/maestro-daemon/tests/activity_report.rs index d1af586..71e4f7c 100644 --- a/crates/maestro-daemon/tests/activity_report.rs +++ b/crates/maestro-daemon/tests/activity_report.rs @@ -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]