From ae7ef53b8ed63680402c89e794ac8adaaae45056 Mon Sep 17 00:00:00 2001 From: cephalonaut Date: Wed, 20 May 2026 00:32:58 -0400 Subject: [PATCH 1/7] feat(SessionSourceType): carry optional task_id on User variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decorate `SessionSourceType::User` with an `Option task_id` field so manually-shared local sessions can plumb the conversation's server-side `ai_tasks` row id through to orchestration discovery, just as cloud-spawned `AmbientAgent` sessions do today. Wire compatibility is preserved in both directions: * The custom `Deserialize` accepts all three shapes: bare `"User"` (legacy), `{"User":{"task_id":"..."}}` (new), and `{"AmbientAgent":{"task_id":"..."}}` — mirroring the existing `AmbientAgent` handling. * A new manual `Serialize` impl emits the bare legacy form when `task_id.is_none()` and the struct form otherwise, so older readers that only understand the unit-variant shape stay forward-compatible until they pick up the new deserializer. * `From<&SessionSourceType> for LegacySessionSourceType` now matches `User { .. }` instead of the unit variant, so the legacy `JoinedSuccessfully.source_type` field continues to round-trip. Adds a `SessionSourceType::orchestrator_task_id()` helper that returns the `task_id` regardless of variant, so downstream orchestration sites can key off task-id presence rather than the variant discriminant. Includes unit tests covering legacy + new wire shapes in both directions, default value, the helper, and the `From` mapping for both variants. Co-Authored-By: Oz --- src/sharer.rs | 322 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 304 insertions(+), 18 deletions(-) diff --git a/src/sharer.rs b/src/sharer.rs index 513e4a5..5b73e51 100644 --- a/src/sharer.rs +++ b/src/sharer.rs @@ -24,7 +24,8 @@ use crate::common::{ use super::common::Scrollback; use byte_unit::Byte; -use serde::{Deserialize, Deserializer, Serialize}; +use serde::ser::SerializeStructVariant; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use uuid::Uuid; /// Possible reasons why the server might gracefully terminate @@ -142,16 +143,37 @@ pub use crate::common::{ UpdatePendingUserRoleResponse, }; -#[derive(Clone, Debug, Serialize, Default)] +#[derive(Clone, Debug)] pub enum SessionSourceType { /// The session was started by a user directly. - #[default] - User, + /// + /// Carries the conversation's server-side `ai_tasks` row id when known. This + /// is independent of whether the conversation acts as an orchestrator — + /// orchestration discovery downstream decides whether to render a pill bar + /// based on whether the task has children. + User { task_id: Option }, /// The session was started in the course of spinning up an ambient agent. - AmbientAgent { - #[serde(default)] - task_id: Option, - }, + AmbientAgent { task_id: Option }, +} + +// `#[derive(Default)]` with `#[default]` on a struct variant is not yet +// supported by stable Rust, so we implement `Default` manually. The default +// matches the pre-`task_id` behavior: a `User`-sourced share with no +// associated task id. +impl Default for SessionSourceType { + fn default() -> Self { + SessionSourceType::User { task_id: None } + } +} + +impl SessionSourceType { + /// Returns the orchestrator `task_id` carried by this source type, + /// regardless of variant. Used to drive orchestration discovery. + pub fn orchestrator_task_id(&self) -> Option<&str> { + match self { + Self::User { task_id } | Self::AmbientAgent { task_id } => task_id.as_deref(), + } + } } /// Internal helper that mirrors all wire representations of SessionSourceType @@ -160,18 +182,24 @@ pub enum SessionSourceType { #[derive(Deserialize)] #[serde(untagged)] enum SessionSourceTypeWire { - /// Legacy representation: "User" or "AmbientAgent". + /// Legacy representation: bare `"User"` or `"AmbientAgent"`. Legacy(LegacySessionSourceType), - /// New representation: externally tagged AmbientAgent with fields, e.g. - /// { "AmbientAgent": { "task_id": "..." } }. - New { + /// New representation: externally tagged `User` with fields, e.g. + /// `{ "User": { "task_id": "..." } }`. + NewUser { + #[serde(rename = "User")] + user: TaskIdFields, + }, + /// New representation: externally tagged `AmbientAgent` with fields, e.g. + /// `{ "AmbientAgent": { "task_id": "..." } }`. + NewAmbientAgent { #[serde(rename = "AmbientAgent")] - ambient_agent: AmbientAgentFields, + ambient_agent: TaskIdFields, }, } #[derive(Deserialize)] -struct AmbientAgentFields { +struct TaskIdFields { #[serde(default)] task_id: Option, } @@ -179,12 +207,17 @@ struct AmbientAgentFields { impl From for SessionSourceType { fn from(value: SessionSourceTypeWire) -> Self { match value { - SessionSourceTypeWire::Legacy(LegacySessionSourceType::User) => SessionSourceType::User, + SessionSourceTypeWire::Legacy(LegacySessionSourceType::User) => { + SessionSourceType::User { task_id: None } + } SessionSourceTypeWire::Legacy(LegacySessionSourceType::AmbientAgent) => { SessionSourceType::AmbientAgent { task_id: None } } - SessionSourceTypeWire::New { - ambient_agent: AmbientAgentFields { task_id }, + SessionSourceTypeWire::NewUser { + user: TaskIdFields { task_id }, + } => SessionSourceType::User { task_id }, + SessionSourceTypeWire::NewAmbientAgent { + ambient_agent: TaskIdFields { task_id }, } => SessionSourceType::AmbientAgent { task_id }, } } @@ -200,6 +233,48 @@ impl<'de> Deserialize<'de> for SessionSourceType { } } +/// Manual `Serialize` impl that emits the bare legacy form (`"User"` or +/// `"AmbientAgent"`) when `task_id` is `None`, and the externally-tagged +/// struct form (`{ "User": { "task_id": "..." } }`) when `task_id` is +/// `Some(_)`. The bare form keeps older readers — which only know the +/// legacy unit-variant shape — working until they pick up the new +/// deserializer. +impl Serialize for SessionSourceType { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + match self { + SessionSourceType::User { task_id: None } => { + serializer.serialize_unit_variant("SessionSourceType", 0, "User") + } + SessionSourceType::User { + task_id: Some(task_id), + } => { + let mut sv = + serializer.serialize_struct_variant("SessionSourceType", 0, "User", 1)?; + sv.serialize_field("task_id", task_id)?; + sv.end() + } + SessionSourceType::AmbientAgent { task_id: None } => { + serializer.serialize_unit_variant("SessionSourceType", 1, "AmbientAgent") + } + SessionSourceType::AmbientAgent { + task_id: Some(task_id), + } => { + let mut sv = serializer.serialize_struct_variant( + "SessionSourceType", + 1, + "AmbientAgent", + 1, + )?; + sv.serialize_field("task_id", task_id)?; + sv.end() + } + } + } +} + #[derive(Clone, Debug, Serialize, Deserialize, Default)] pub enum LegacySessionSourceType { #[default] @@ -210,7 +285,7 @@ pub enum LegacySessionSourceType { impl From<&SessionSourceType> for LegacySessionSourceType { fn from(value: &SessionSourceType) -> Self { match value { - SessionSourceType::User => LegacySessionSourceType::User, + SessionSourceType::User { .. } => LegacySessionSourceType::User, SessionSourceType::AmbientAgent { .. } => LegacySessionSourceType::AmbientAgent, } } @@ -601,3 +676,214 @@ impl UpstreamMessage { } } } + +#[cfg(test)] +mod session_source_type_tests { + //! Wire-compatibility tests for `SessionSourceType`. + //! + //! These guarantee both directions of the migration stay + //! backward-compatible: legacy clients writing the bare + //! string form continue to be readable, and new clients + //! writing the bare form when `task_id` is `None` stay + //! readable by older clients that only understand the + //! legacy unit-variant shape. + use super::*; + + // --- Deserialization --- + + #[test] + fn deserialize_legacy_user_bare() { + let v: SessionSourceType = serde_json::from_str("\"User\"").unwrap(); + assert!(matches!(v, SessionSourceType::User { task_id: None })); + } + + #[test] + fn deserialize_legacy_ambient_agent_bare() { + let v: SessionSourceType = serde_json::from_str("\"AmbientAgent\"").unwrap(); + assert!(matches!( + v, + SessionSourceType::AmbientAgent { task_id: None } + )); + } + + #[test] + fn deserialize_new_user_with_task_id() { + let v: SessionSourceType = serde_json::from_str(r#"{"User":{"task_id":"abc"}}"#).unwrap(); + match v { + SessionSourceType::User { + task_id: Some(ref s), + } if s == "abc" => {} + other => panic!("expected User {{ task_id: Some(\"abc\") }}, got {other:?}"), + } + } + + #[test] + fn deserialize_new_user_with_null_task_id() { + let v: SessionSourceType = serde_json::from_str(r#"{"User":{"task_id":null}}"#).unwrap(); + assert!(matches!(v, SessionSourceType::User { task_id: None })); + } + + #[test] + fn deserialize_new_user_without_task_id_field() { + let v: SessionSourceType = serde_json::from_str(r#"{"User":{}}"#).unwrap(); + assert!(matches!(v, SessionSourceType::User { task_id: None })); + } + + #[test] + fn deserialize_new_ambient_agent_with_task_id() { + let v: SessionSourceType = + serde_json::from_str(r#"{"AmbientAgent":{"task_id":"xyz"}}"#).unwrap(); + match v { + SessionSourceType::AmbientAgent { + task_id: Some(ref s), + } if s == "xyz" => {} + other => panic!("expected AmbientAgent {{ task_id: Some(\"xyz\") }}, got {other:?}"), + } + } + + #[test] + fn deserialize_new_ambient_agent_without_task_id_field() { + let v: SessionSourceType = serde_json::from_str(r#"{"AmbientAgent":{}}"#).unwrap(); + assert!(matches!( + v, + SessionSourceType::AmbientAgent { task_id: None } + )); + } + + // --- Serialization --- + + #[test] + fn serialize_user_without_task_id_emits_bare_form() { + let v = SessionSourceType::User { task_id: None }; + let json = serde_json::to_string(&v).unwrap(); + // Legacy bare form so older readers can still parse it. + assert_eq!(json, "\"User\""); + } + + #[test] + fn serialize_user_with_task_id_emits_struct_form() { + let v = SessionSourceType::User { + task_id: Some("abc".to_string()), + }; + let json = serde_json::to_string(&v).unwrap(); + assert_eq!(json, r#"{"User":{"task_id":"abc"}}"#); + } + + #[test] + fn serialize_ambient_agent_without_task_id_emits_bare_form() { + let v = SessionSourceType::AmbientAgent { task_id: None }; + let json = serde_json::to_string(&v).unwrap(); + assert_eq!(json, "\"AmbientAgent\""); + } + + #[test] + fn serialize_ambient_agent_with_task_id_emits_struct_form() { + let v = SessionSourceType::AmbientAgent { + task_id: Some("xyz".to_string()), + }; + let json = serde_json::to_string(&v).unwrap(); + assert_eq!(json, r#"{"AmbientAgent":{"task_id":"xyz"}}"#); + } + + // --- Roundtrip --- + + #[test] + fn roundtrip_user_with_task_id() { + let v = SessionSourceType::User { + task_id: Some("abc".to_string()), + }; + let json = serde_json::to_string(&v).unwrap(); + let parsed: SessionSourceType = serde_json::from_str(&json).unwrap(); + match parsed { + SessionSourceType::User { + task_id: Some(ref s), + } if s == "abc" => {} + other => panic!("roundtrip altered value: {other:?}"), + } + } + + #[test] + fn roundtrip_ambient_agent_with_task_id() { + let v = SessionSourceType::AmbientAgent { + task_id: Some("xyz".to_string()), + }; + let json = serde_json::to_string(&v).unwrap(); + let parsed: SessionSourceType = serde_json::from_str(&json).unwrap(); + match parsed { + SessionSourceType::AmbientAgent { + task_id: Some(ref s), + } if s == "xyz" => {} + other => panic!("roundtrip altered value: {other:?}"), + } + } + + // --- Helpers --- + + #[test] + fn orchestrator_task_id_returns_user_task_id() { + let v = SessionSourceType::User { + task_id: Some("abc".to_string()), + }; + assert_eq!(v.orchestrator_task_id(), Some("abc")); + } + + #[test] + fn orchestrator_task_id_returns_ambient_agent_task_id() { + let v = SessionSourceType::AmbientAgent { + task_id: Some("xyz".to_string()), + }; + assert_eq!(v.orchestrator_task_id(), Some("xyz")); + } + + #[test] + fn orchestrator_task_id_none_when_missing() { + assert_eq!( + SessionSourceType::User { task_id: None }.orchestrator_task_id(), + None + ); + assert_eq!( + SessionSourceType::AmbientAgent { task_id: None }.orchestrator_task_id(), + None + ); + } + + #[test] + fn from_user_maps_to_legacy_user_regardless_of_task_id() { + let no_task = SessionSourceType::User { task_id: None }; + assert!(matches!( + LegacySessionSourceType::from(&no_task), + LegacySessionSourceType::User + )); + + let with_task = SessionSourceType::User { + task_id: Some("abc".to_string()), + }; + assert!(matches!( + LegacySessionSourceType::from(&with_task), + LegacySessionSourceType::User + )); + } + + #[test] + fn from_ambient_agent_maps_to_legacy_ambient_agent_regardless_of_task_id() { + let no_task = SessionSourceType::AmbientAgent { task_id: None }; + assert!(matches!( + LegacySessionSourceType::from(&no_task), + LegacySessionSourceType::AmbientAgent + )); + + let with_task = SessionSourceType::AmbientAgent { + task_id: Some("xyz".to_string()), + }; + assert!(matches!( + LegacySessionSourceType::from(&with_task), + LegacySessionSourceType::AmbientAgent + )); + } + + #[test] + fn default_is_user_without_task_id() { + let v = SessionSourceType::default(); + assert!(matches!(v, SessionSourceType::User { task_id: None })); + } +} From b19b3b3b1d95787d0bb98b39a14fddec2df2c862 Mon Sep 17 00:00:00 2001 From: cephalonaut Date: Wed, 20 May 2026 22:50:21 -0400 Subject: [PATCH 2/7] Add symmetric null-task_id deserialize test for AmbientAgent Mirror the existing deserialize_new_user_with_null_task_id test for the AmbientAgent variant so we keep wire compatibility with any already-persisted Redis SessionManifest rows that were written before the manual Serialize impl collapsed the None case to the bare unit-variant form. Co-Authored-By: Oz --- src/sharer.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/sharer.rs b/src/sharer.rs index 5b73e51..1e8ab68 100644 --- a/src/sharer.rs +++ b/src/sharer.rs @@ -741,6 +741,19 @@ mod session_source_type_tests { } } + #[test] + fn deserialize_new_ambient_agent_with_null_task_id() { + // Guards already-persisted Redis SessionManifest rows that were + // written before the manual `Serialize` impl collapsed the + // None case down to the bare unit-variant form. + let v: SessionSourceType = + serde_json::from_str(r#"{"AmbientAgent":{"task_id":null}}"#).unwrap(); + assert!(matches!( + v, + SessionSourceType::AmbientAgent { task_id: None } + )); + } + #[test] fn deserialize_new_ambient_agent_without_task_id_field() { let v: SessionSourceType = serde_json::from_str(r#"{"AmbientAgent":{}}"#).unwrap(); From 2141fb3dae361bde7695e6c63a3a1a5582a0a57f Mon Sep 17 00:00:00 2001 From: cephalonaut Date: Wed, 20 May 2026 23:38:15 -0400 Subject: [PATCH 3/7] Tighten QUALITY-726 comments further MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trim the multi-line doc comments left by the previous cleanup: - SessionSourceType::User: drop the orchestrator-discovery paragraph and fold the `task_id` description into a single sentence. - Default impl: collapse the 4-line "default matches previous behavior" note to a 2-line stable-Rust workaround note. - orchestrator_task_id: drop the "Used to drive orchestration discovery" postscript — the function name carries it. - Serialize impl doc: collapse the 6-line variant explanation to 3 lines. - session_source_type_tests module doc: drop the multi-paragraph migration narrative and keep the one-line summary. - deserialize_new_ambient_agent_with_null_task_id: shorten the inline rationale to one line. Co-Authored-By: Oz --- src/sharer.rs | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/src/sharer.rs b/src/sharer.rs index 1e8ab68..6d458e0 100644 --- a/src/sharer.rs +++ b/src/sharer.rs @@ -145,21 +145,15 @@ pub use crate::common::{ #[derive(Clone, Debug)] pub enum SessionSourceType { - /// The session was started by a user directly. - /// - /// Carries the conversation's server-side `ai_tasks` row id when known. This - /// is independent of whether the conversation acts as an orchestrator — - /// orchestration discovery downstream decides whether to render a pill bar - /// based on whether the task has children. + /// The session was started by a user directly. `task_id` is the + /// server-side `ai_tasks` row id when known. User { task_id: Option }, /// The session was started in the course of spinning up an ambient agent. AmbientAgent { task_id: Option }, } // `#[derive(Default)]` with `#[default]` on a struct variant is not yet -// supported by stable Rust, so we implement `Default` manually. The default -// matches the pre-`task_id` behavior: a `User`-sourced share with no -// associated task id. +// supported on stable Rust, so we implement `Default` manually. impl Default for SessionSourceType { fn default() -> Self { SessionSourceType::User { task_id: None } @@ -167,8 +161,7 @@ impl Default for SessionSourceType { } impl SessionSourceType { - /// Returns the orchestrator `task_id` carried by this source type, - /// regardless of variant. Used to drive orchestration discovery. + /// Returns the `task_id` carried by this source type, regardless of variant. pub fn orchestrator_task_id(&self) -> Option<&str> { match self { Self::User { task_id } | Self::AmbientAgent { task_id } => task_id.as_deref(), @@ -233,12 +226,9 @@ impl<'de> Deserialize<'de> for SessionSourceType { } } -/// Manual `Serialize` impl that emits the bare legacy form (`"User"` or -/// `"AmbientAgent"`) when `task_id` is `None`, and the externally-tagged -/// struct form (`{ "User": { "task_id": "..." } }`) when `task_id` is -/// `Some(_)`. The bare form keeps older readers — which only know the -/// legacy unit-variant shape — working until they pick up the new -/// deserializer. +/// Emits the bare legacy form when `task_id` is `None` and the struct form +/// otherwise, so older readers that only understand the unit-variant shape +/// stay forward-compatible until they pick up the new deserializer. impl Serialize for SessionSourceType { fn serialize(&self, serializer: S) -> Result where @@ -680,13 +670,6 @@ impl UpstreamMessage { #[cfg(test)] mod session_source_type_tests { //! Wire-compatibility tests for `SessionSourceType`. - //! - //! These guarantee both directions of the migration stay - //! backward-compatible: legacy clients writing the bare - //! string form continue to be readable, and new clients - //! writing the bare form when `task_id` is `None` stay - //! readable by older clients that only understand the - //! legacy unit-variant shape. use super::*; // --- Deserialization --- @@ -743,9 +726,7 @@ mod session_source_type_tests { #[test] fn deserialize_new_ambient_agent_with_null_task_id() { - // Guards already-persisted Redis SessionManifest rows that were - // written before the manual `Serialize` impl collapsed the - // None case down to the bare unit-variant form. + // Guards Redis rows written before Serialize emitted bare unit-variants. let v: SessionSourceType = serde_json::from_str(r#"{"AmbientAgent":{"task_id":null}}"#).unwrap(); assert!(matches!( From 30bb41553092879e59e8d4ad7cc72a303948868f Mon Sep 17 00:00:00 2001 From: cephalonaut Date: Thu, 21 May 2026 09:25:32 -0400 Subject: [PATCH 4/7] Move User task_id off the variant to a source_task_id sidecar Reverts SessionSourceType::User back to a strict unit variant and adds a new optional source_task_id sidecar to InitPayload (sharer) and JoinedSuccessfully (viewer). Restoring the unit form keeps the wire shape compatible with pre-QUALITY-726 viewers that only understood the bare 'User' string; new code threads orchestrator task ids through the sidecar instead. AmbientAgent is left unchanged from main (struct variant with task_id) so existing AmbientAgent producers and viewers continue to roundtrip. The User-variant deserialization, custom Serialize impl, and the NewUser wire branch are all removed; tests are pared down to the reverted SessionSourceType surface and a new init_payload_tests module that exercises the sidecar field for back-compat and roundtrip. Co-Authored-By: Oz --- src/sharer.rs | 274 ++++++++++++++++++-------------------------------- src/viewer.rs | 7 ++ 2 files changed, 105 insertions(+), 176 deletions(-) diff --git a/src/sharer.rs b/src/sharer.rs index 6d458e0..4776fbe 100644 --- a/src/sharer.rs +++ b/src/sharer.rs @@ -24,8 +24,7 @@ use crate::common::{ use super::common::Scrollback; use byte_unit::Byte; -use serde::ser::SerializeStructVariant; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Deserializer, Serialize}; use uuid::Uuid; /// Possible reasons why the server might gracefully terminate @@ -143,56 +142,35 @@ pub use crate::common::{ UpdatePendingUserRoleResponse, }; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Serialize, Default)] pub enum SessionSourceType { - /// The session was started by a user directly. `task_id` is the - /// server-side `ai_tasks` row id when known. - User { task_id: Option }, + /// The session was started by a user directly. + #[default] + User, /// The session was started in the course of spinning up an ambient agent. - AmbientAgent { task_id: Option }, -} - -// `#[derive(Default)]` with `#[default]` on a struct variant is not yet -// supported on stable Rust, so we implement `Default` manually. -impl Default for SessionSourceType { - fn default() -> Self { - SessionSourceType::User { task_id: None } - } -} - -impl SessionSourceType { - /// Returns the `task_id` carried by this source type, regardless of variant. - pub fn orchestrator_task_id(&self) -> Option<&str> { - match self { - Self::User { task_id } | Self::AmbientAgent { task_id } => task_id.as_deref(), - } - } + AmbientAgent { + #[serde(default)] + task_id: Option, + }, } -/// Internal helper that mirrors all wire representations of SessionSourceType -/// (both legacy and new) so we don't recursively call SessionSourceType's -/// custom Deserialize impl. +/// Mirrors the legacy unit-variant form and the new struct-variant form so +/// the custom `Deserialize` impl can accept both shapes. #[derive(Deserialize)] #[serde(untagged)] enum SessionSourceTypeWire { /// Legacy representation: bare `"User"` or `"AmbientAgent"`. Legacy(LegacySessionSourceType), - /// New representation: externally tagged `User` with fields, e.g. - /// `{ "User": { "task_id": "..." } }`. - NewUser { - #[serde(rename = "User")] - user: TaskIdFields, - }, /// New representation: externally tagged `AmbientAgent` with fields, e.g. /// `{ "AmbientAgent": { "task_id": "..." } }`. - NewAmbientAgent { + New { #[serde(rename = "AmbientAgent")] - ambient_agent: TaskIdFields, + ambient_agent: AmbientAgentFields, }, } #[derive(Deserialize)] -struct TaskIdFields { +struct AmbientAgentFields { #[serde(default)] task_id: Option, } @@ -200,17 +178,12 @@ struct TaskIdFields { impl From for SessionSourceType { fn from(value: SessionSourceTypeWire) -> Self { match value { - SessionSourceTypeWire::Legacy(LegacySessionSourceType::User) => { - SessionSourceType::User { task_id: None } - } + SessionSourceTypeWire::Legacy(LegacySessionSourceType::User) => SessionSourceType::User, SessionSourceTypeWire::Legacy(LegacySessionSourceType::AmbientAgent) => { SessionSourceType::AmbientAgent { task_id: None } } - SessionSourceTypeWire::NewUser { - user: TaskIdFields { task_id }, - } => SessionSourceType::User { task_id }, - SessionSourceTypeWire::NewAmbientAgent { - ambient_agent: TaskIdFields { task_id }, + SessionSourceTypeWire::New { + ambient_agent: AmbientAgentFields { task_id }, } => SessionSourceType::AmbientAgent { task_id }, } } @@ -226,45 +199,6 @@ impl<'de> Deserialize<'de> for SessionSourceType { } } -/// Emits the bare legacy form when `task_id` is `None` and the struct form -/// otherwise, so older readers that only understand the unit-variant shape -/// stay forward-compatible until they pick up the new deserializer. -impl Serialize for SessionSourceType { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - match self { - SessionSourceType::User { task_id: None } => { - serializer.serialize_unit_variant("SessionSourceType", 0, "User") - } - SessionSourceType::User { - task_id: Some(task_id), - } => { - let mut sv = - serializer.serialize_struct_variant("SessionSourceType", 0, "User", 1)?; - sv.serialize_field("task_id", task_id)?; - sv.end() - } - SessionSourceType::AmbientAgent { task_id: None } => { - serializer.serialize_unit_variant("SessionSourceType", 1, "AmbientAgent") - } - SessionSourceType::AmbientAgent { - task_id: Some(task_id), - } => { - let mut sv = serializer.serialize_struct_variant( - "SessionSourceType", - 1, - "AmbientAgent", - 1, - )?; - sv.serialize_field("task_id", task_id)?; - sv.end() - } - } - } -} - #[derive(Clone, Debug, Serialize, Deserialize, Default)] pub enum LegacySessionSourceType { #[default] @@ -275,7 +209,7 @@ pub enum LegacySessionSourceType { impl From<&SessionSourceType> for LegacySessionSourceType { fn from(value: &SessionSourceType) -> Self { match value { - SessionSourceType::User { .. } => LegacySessionSourceType::User, + SessionSourceType::User => LegacySessionSourceType::User, SessionSourceType::AmbientAgent { .. } => LegacySessionSourceType::AmbientAgent, } } @@ -328,6 +262,13 @@ pub struct InitPayload { #[serde(default)] pub source_type: SessionSourceType, + /// Optional orchestrator `task_id` carried alongside `source_type`. + /// Set when the sharer wants downstream orchestration discovery to find + /// this share's children regardless of variant kind. Sidecar so the + /// `User` variant can stay a unit and old viewers ignore it. + #[serde(default)] + pub source_task_id: Option, + /// Client feature support declaration. #[serde(default)] pub feature_support: FeatureSupport, @@ -669,7 +610,11 @@ impl UpstreamMessage { #[cfg(test)] mod session_source_type_tests { - //! Wire-compatibility tests for `SessionSourceType`. + //! Wire-compatibility tests for `SessionSourceType` after the + //! QUALITY-726 sidecar redesign reverted `User` to a strict unit + //! variant. `AmbientAgent` keeps the struct shape it already had + //! on `main`; new orchestrator `task_id`s for `User` shares ride + //! on the `InitPayload::source_task_id` sidecar instead. use super::*; // --- Deserialization --- @@ -677,7 +622,7 @@ mod session_source_type_tests { #[test] fn deserialize_legacy_user_bare() { let v: SessionSourceType = serde_json::from_str("\"User\"").unwrap(); - assert!(matches!(v, SessionSourceType::User { task_id: None })); + assert!(matches!(v, SessionSourceType::User)); } #[test] @@ -689,29 +634,6 @@ mod session_source_type_tests { )); } - #[test] - fn deserialize_new_user_with_task_id() { - let v: SessionSourceType = serde_json::from_str(r#"{"User":{"task_id":"abc"}}"#).unwrap(); - match v { - SessionSourceType::User { - task_id: Some(ref s), - } if s == "abc" => {} - other => panic!("expected User {{ task_id: Some(\"abc\") }}, got {other:?}"), - } - } - - #[test] - fn deserialize_new_user_with_null_task_id() { - let v: SessionSourceType = serde_json::from_str(r#"{"User":{"task_id":null}}"#).unwrap(); - assert!(matches!(v, SessionSourceType::User { task_id: None })); - } - - #[test] - fn deserialize_new_user_without_task_id_field() { - let v: SessionSourceType = serde_json::from_str(r#"{"User":{}}"#).unwrap(); - assert!(matches!(v, SessionSourceType::User { task_id: None })); - } - #[test] fn deserialize_new_ambient_agent_with_task_id() { let v: SessionSourceType = @@ -726,7 +648,7 @@ mod session_source_type_tests { #[test] fn deserialize_new_ambient_agent_with_null_task_id() { - // Guards Redis rows written before Serialize emitted bare unit-variants. + // Guards Redis rows written before Serialize collapsed the None case. let v: SessionSourceType = serde_json::from_str(r#"{"AmbientAgent":{"task_id":null}}"#).unwrap(); assert!(matches!( @@ -747,27 +669,20 @@ mod session_source_type_tests { // --- Serialization --- #[test] - fn serialize_user_without_task_id_emits_bare_form() { - let v = SessionSourceType::User { task_id: None }; + fn serialize_user_emits_bare_form() { + let v = SessionSourceType::User; let json = serde_json::to_string(&v).unwrap(); - // Legacy bare form so older readers can still parse it. assert_eq!(json, "\"User\""); } #[test] - fn serialize_user_with_task_id_emits_struct_form() { - let v = SessionSourceType::User { - task_id: Some("abc".to_string()), - }; - let json = serde_json::to_string(&v).unwrap(); - assert_eq!(json, r#"{"User":{"task_id":"abc"}}"#); - } - - #[test] - fn serialize_ambient_agent_without_task_id_emits_bare_form() { + fn serialize_ambient_agent_without_task_id_emits_struct_form() { + // `AmbientAgent` has been a struct variant since before QUALITY-726, + // so the derived Serialize emits the externally tagged form with a + // `null` task_id rather than the bare legacy form. let v = SessionSourceType::AmbientAgent { task_id: None }; let json = serde_json::to_string(&v).unwrap(); - assert_eq!(json, "\"AmbientAgent\""); + assert_eq!(json, r#"{"AmbientAgent":{"task_id":null}}"#); } #[test] @@ -782,18 +697,10 @@ mod session_source_type_tests { // --- Roundtrip --- #[test] - fn roundtrip_user_with_task_id() { - let v = SessionSourceType::User { - task_id: Some("abc".to_string()), - }; - let json = serde_json::to_string(&v).unwrap(); + fn roundtrip_user() { + let json = serde_json::to_string(&SessionSourceType::User).unwrap(); let parsed: SessionSourceType = serde_json::from_str(&json).unwrap(); - match parsed { - SessionSourceType::User { - task_id: Some(ref s), - } if s == "abc" => {} - other => panic!("roundtrip altered value: {other:?}"), - } + assert!(matches!(parsed, SessionSourceType::User)); } #[test] @@ -814,46 +721,9 @@ mod session_source_type_tests { // --- Helpers --- #[test] - fn orchestrator_task_id_returns_user_task_id() { - let v = SessionSourceType::User { - task_id: Some("abc".to_string()), - }; - assert_eq!(v.orchestrator_task_id(), Some("abc")); - } - - #[test] - fn orchestrator_task_id_returns_ambient_agent_task_id() { - let v = SessionSourceType::AmbientAgent { - task_id: Some("xyz".to_string()), - }; - assert_eq!(v.orchestrator_task_id(), Some("xyz")); - } - - #[test] - fn orchestrator_task_id_none_when_missing() { - assert_eq!( - SessionSourceType::User { task_id: None }.orchestrator_task_id(), - None - ); - assert_eq!( - SessionSourceType::AmbientAgent { task_id: None }.orchestrator_task_id(), - None - ); - } - - #[test] - fn from_user_maps_to_legacy_user_regardless_of_task_id() { - let no_task = SessionSourceType::User { task_id: None }; - assert!(matches!( - LegacySessionSourceType::from(&no_task), - LegacySessionSourceType::User - )); - - let with_task = SessionSourceType::User { - task_id: Some("abc".to_string()), - }; + fn from_user_maps_to_legacy_user() { assert!(matches!( - LegacySessionSourceType::from(&with_task), + LegacySessionSourceType::from(&SessionSourceType::User), LegacySessionSourceType::User )); } @@ -876,8 +746,60 @@ mod session_source_type_tests { } #[test] - fn default_is_user_without_task_id() { + fn default_is_user() { let v = SessionSourceType::default(); - assert!(matches!(v, SessionSourceType::User { task_id: None })); + assert!(matches!(v, SessionSourceType::User)); + } +} + +#[cfg(test)] +mod init_payload_tests { + //! Wire-compatibility tests for the `source_task_id` sidecar on + //! `InitPayload`. The sidecar is the canonical way to carry an + //! orchestrator `task_id` for `SessionSourceType::User` shares, + //! since the `User` variant is unit-shaped. + use super::*; + use crate::common::{ActivePrompt, BlockId, InputReplicaId, Selection, UserID, WindowSize}; + + fn make_payload(source_task_id: Option) -> InitPayload { + InitPayload { + scrollback: Scrollback { + blocks: Vec::new(), + is_alt_screen_active: false, + }, + active_prompt: ActivePrompt::PS1, + window_size: WindowSize { + num_rows: 24, + num_cols: 80, + }, + user_id: UserID::default(), + selection: Selection::None, + init_block_id: BlockId::default(), + input_replica_id: InputReplicaId::default(), + telemetry_context: None, + lifetime: Lifetime::default(), + universal_developer_input_context: None, + source_type: SessionSourceType::User, + source_task_id, + feature_support: FeatureSupport::default(), + } + } + + #[test] + fn source_task_id_defaults_to_none_when_field_missing() { + // Older clients pre-sidecar omit the field entirely; the server + // must still accept that payload shape and treat the task id as + // absent. + let mut value = serde_json::to_value(make_payload(None)).unwrap(); + value.as_object_mut().unwrap().remove("source_task_id"); + let parsed: InitPayload = serde_json::from_value(value).unwrap(); + assert!(parsed.source_task_id.is_none()); + } + + #[test] + fn source_task_id_roundtrips_when_present() { + let json = serde_json::to_string(&make_payload(Some("abc".to_string()))).unwrap(); + let parsed: InitPayload = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed.source_task_id.as_deref(), Some("abc")); } } diff --git a/src/viewer.rs b/src/viewer.rs index 16fbfd2..5295378 100644 --- a/src/viewer.rs +++ b/src/viewer.rs @@ -142,6 +142,13 @@ pub enum DownstreamMessage { /// The detailed source type for this shared session. #[serde(default)] detailed_source_type: SessionSourceType, + + /// Optional orchestrator `task_id` carried alongside the source + /// type, mirroring `sharer::InitPayload::source_task_id`. Lets + /// viewers find this share's orchestrator task without keying + /// off the source-type variant kind. + #[serde(default)] + source_task_id: Option, }, /// The server sends this message when the session was successfully rejoined. From a6645cd5ce629dd19773e4edade371073171a146 Mon Sep 17 00:00:00 2001 From: cephalonaut Date: Thu, 21 May 2026 09:43:56 -0400 Subject: [PATCH 5/7] Suppress large_enum_variant on UpstreamMessage The new InitPayload::source_task_id sidecar pushed InitPayload across the default clippy threshold for variant size. Boxing the variant would be wire-compatible but ripples through every call site; suppress the lint instead, matching the same scoped allow the vendored protocol copy in session-sharing-server uses. Co-Authored-By: Oz --- src/sharer.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sharer.rs b/src/sharer.rs index 4776fbe..c9f622c 100644 --- a/src/sharer.rs +++ b/src/sharer.rs @@ -480,6 +480,10 @@ impl DownstreamMessage { } /// The possible messages sent from client (sharer) to server. +// `Initialize(InitPayload)` is much larger than the other variants because +// `InitPayload` carries scrollback and feature-support data. Boxing it would +// be wire-compatible but churn every call site; suppress the lint instead. +#[allow(clippy::large_enum_variant)] #[derive(Debug, Deserialize, Serialize)] pub enum UpstreamMessage { /// The client sends this message to start a shared session. From 8984d8d035e1a5c1178a4a74d7f8efaadedfae30 Mon Sep 17 00:00:00 2001 From: cephalonaut Date: Thu, 21 May 2026 16:24:17 -0400 Subject: [PATCH 6/7] Trim QUALITY-726 tests to the wire-compat essentials Address review feedback on PR #15 ("tests seem unnecessary for a protocol"): keep only the cases that exercise hand-written logic. The 5 SessionSourceType deserialize tests pin the custom Deserialize impl that bridges the legacy bare-string form and the new struct-variant form, and the InitPayload sidecar test pins the #[serde(default)] contract that lets old clients omit source_task_id. Drop the derive snapshots, roundtrips, trivial From/Default tests, and the Some-case sidecar roundtrip. Co-Authored-By: Oz --- src/sharer.rs | 126 +++++--------------------------------------------- 1 file changed, 11 insertions(+), 115 deletions(-) diff --git a/src/sharer.rs b/src/sharer.rs index c9f622c..3ae74bd 100644 --- a/src/sharer.rs +++ b/src/sharer.rs @@ -614,15 +614,11 @@ impl UpstreamMessage { #[cfg(test)] mod session_source_type_tests { - //! Wire-compatibility tests for `SessionSourceType` after the - //! QUALITY-726 sidecar redesign reverted `User` to a strict unit - //! variant. `AmbientAgent` keeps the struct shape it already had - //! on `main`; new orchestrator `task_id`s for `User` shares ride - //! on the `InitPayload::source_task_id` sidecar instead. + //! Wire-compatibility tests for the custom `SessionSourceType` + //! `Deserialize` impl, which bridges the legacy bare-string form + //! and the new struct-variant form so old clients keep parsing. use super::*; - // --- Deserialization --- - #[test] fn deserialize_legacy_user_bare() { let v: SessionSourceType = serde_json::from_str("\"User\"").unwrap(); @@ -669,104 +665,18 @@ mod session_source_type_tests { SessionSourceType::AmbientAgent { task_id: None } )); } - - // --- Serialization --- - - #[test] - fn serialize_user_emits_bare_form() { - let v = SessionSourceType::User; - let json = serde_json::to_string(&v).unwrap(); - assert_eq!(json, "\"User\""); - } - - #[test] - fn serialize_ambient_agent_without_task_id_emits_struct_form() { - // `AmbientAgent` has been a struct variant since before QUALITY-726, - // so the derived Serialize emits the externally tagged form with a - // `null` task_id rather than the bare legacy form. - let v = SessionSourceType::AmbientAgent { task_id: None }; - let json = serde_json::to_string(&v).unwrap(); - assert_eq!(json, r#"{"AmbientAgent":{"task_id":null}}"#); - } - - #[test] - fn serialize_ambient_agent_with_task_id_emits_struct_form() { - let v = SessionSourceType::AmbientAgent { - task_id: Some("xyz".to_string()), - }; - let json = serde_json::to_string(&v).unwrap(); - assert_eq!(json, r#"{"AmbientAgent":{"task_id":"xyz"}}"#); - } - - // --- Roundtrip --- - - #[test] - fn roundtrip_user() { - let json = serde_json::to_string(&SessionSourceType::User).unwrap(); - let parsed: SessionSourceType = serde_json::from_str(&json).unwrap(); - assert!(matches!(parsed, SessionSourceType::User)); - } - - #[test] - fn roundtrip_ambient_agent_with_task_id() { - let v = SessionSourceType::AmbientAgent { - task_id: Some("xyz".to_string()), - }; - let json = serde_json::to_string(&v).unwrap(); - let parsed: SessionSourceType = serde_json::from_str(&json).unwrap(); - match parsed { - SessionSourceType::AmbientAgent { - task_id: Some(ref s), - } if s == "xyz" => {} - other => panic!("roundtrip altered value: {other:?}"), - } - } - - // --- Helpers --- - - #[test] - fn from_user_maps_to_legacy_user() { - assert!(matches!( - LegacySessionSourceType::from(&SessionSourceType::User), - LegacySessionSourceType::User - )); - } - - #[test] - fn from_ambient_agent_maps_to_legacy_ambient_agent_regardless_of_task_id() { - let no_task = SessionSourceType::AmbientAgent { task_id: None }; - assert!(matches!( - LegacySessionSourceType::from(&no_task), - LegacySessionSourceType::AmbientAgent - )); - - let with_task = SessionSourceType::AmbientAgent { - task_id: Some("xyz".to_string()), - }; - assert!(matches!( - LegacySessionSourceType::from(&with_task), - LegacySessionSourceType::AmbientAgent - )); - } - - #[test] - fn default_is_user() { - let v = SessionSourceType::default(); - assert!(matches!(v, SessionSourceType::User)); - } } #[cfg(test)] mod init_payload_tests { - //! Wire-compatibility tests for the `source_task_id` sidecar on - //! `InitPayload`. The sidecar is the canonical way to carry an - //! orchestrator `task_id` for `SessionSourceType::User` shares, - //! since the `User` variant is unit-shaped. + //! Wire-compatibility test for `InitPayload::source_task_id`: + //! older clients pre-sidecar omit the field and must still parse. use super::*; use crate::common::{ActivePrompt, BlockId, InputReplicaId, Selection, UserID, WindowSize}; - fn make_payload(source_task_id: Option) -> InitPayload { - InitPayload { + #[test] + fn source_task_id_defaults_to_none_when_field_missing() { + let payload = InitPayload { scrollback: Scrollback { blocks: Vec::new(), is_alt_screen_active: false, @@ -784,26 +694,12 @@ mod init_payload_tests { lifetime: Lifetime::default(), universal_developer_input_context: None, source_type: SessionSourceType::User, - source_task_id, + source_task_id: None, feature_support: FeatureSupport::default(), - } - } - - #[test] - fn source_task_id_defaults_to_none_when_field_missing() { - // Older clients pre-sidecar omit the field entirely; the server - // must still accept that payload shape and treat the task id as - // absent. - let mut value = serde_json::to_value(make_payload(None)).unwrap(); + }; + let mut value = serde_json::to_value(payload).unwrap(); value.as_object_mut().unwrap().remove("source_task_id"); let parsed: InitPayload = serde_json::from_value(value).unwrap(); assert!(parsed.source_task_id.is_none()); } - - #[test] - fn source_task_id_roundtrips_when_present() { - let json = serde_json::to_string(&make_payload(Some("abc".to_string()))).unwrap(); - let parsed: InitPayload = serde_json::from_str(&json).unwrap(); - assert_eq!(parsed.source_task_id.as_deref(), Some("abc")); - } } From d1c3d7b5763d27df3b9a8b272eafecc5a0c67bc5 Mon Sep 17 00:00:00 2001 From: cephalonaut Date: Thu, 21 May 2026 16:35:31 -0400 Subject: [PATCH 7/7] Drop QUALITY-726 protocol tests Per review on PR #15: the 5 SessionSourceType deserialize tests covered code that already exists on main (the custom Deserialize bridge predates this PR), and the InitPayload source_task_id default test exercised standard #[serde(default)] behavior that is documented by the attribute itself. Co-Authored-By: Oz --- src/sharer.rs | 92 --------------------------------------------------- 1 file changed, 92 deletions(-) diff --git a/src/sharer.rs b/src/sharer.rs index 3ae74bd..01ecd9d 100644 --- a/src/sharer.rs +++ b/src/sharer.rs @@ -611,95 +611,3 @@ impl UpstreamMessage { } } } - -#[cfg(test)] -mod session_source_type_tests { - //! Wire-compatibility tests for the custom `SessionSourceType` - //! `Deserialize` impl, which bridges the legacy bare-string form - //! and the new struct-variant form so old clients keep parsing. - use super::*; - - #[test] - fn deserialize_legacy_user_bare() { - let v: SessionSourceType = serde_json::from_str("\"User\"").unwrap(); - assert!(matches!(v, SessionSourceType::User)); - } - - #[test] - fn deserialize_legacy_ambient_agent_bare() { - let v: SessionSourceType = serde_json::from_str("\"AmbientAgent\"").unwrap(); - assert!(matches!( - v, - SessionSourceType::AmbientAgent { task_id: None } - )); - } - - #[test] - fn deserialize_new_ambient_agent_with_task_id() { - let v: SessionSourceType = - serde_json::from_str(r#"{"AmbientAgent":{"task_id":"xyz"}}"#).unwrap(); - match v { - SessionSourceType::AmbientAgent { - task_id: Some(ref s), - } if s == "xyz" => {} - other => panic!("expected AmbientAgent {{ task_id: Some(\"xyz\") }}, got {other:?}"), - } - } - - #[test] - fn deserialize_new_ambient_agent_with_null_task_id() { - // Guards Redis rows written before Serialize collapsed the None case. - let v: SessionSourceType = - serde_json::from_str(r#"{"AmbientAgent":{"task_id":null}}"#).unwrap(); - assert!(matches!( - v, - SessionSourceType::AmbientAgent { task_id: None } - )); - } - - #[test] - fn deserialize_new_ambient_agent_without_task_id_field() { - let v: SessionSourceType = serde_json::from_str(r#"{"AmbientAgent":{}}"#).unwrap(); - assert!(matches!( - v, - SessionSourceType::AmbientAgent { task_id: None } - )); - } -} - -#[cfg(test)] -mod init_payload_tests { - //! Wire-compatibility test for `InitPayload::source_task_id`: - //! older clients pre-sidecar omit the field and must still parse. - use super::*; - use crate::common::{ActivePrompt, BlockId, InputReplicaId, Selection, UserID, WindowSize}; - - #[test] - fn source_task_id_defaults_to_none_when_field_missing() { - let payload = InitPayload { - scrollback: Scrollback { - blocks: Vec::new(), - is_alt_screen_active: false, - }, - active_prompt: ActivePrompt::PS1, - window_size: WindowSize { - num_rows: 24, - num_cols: 80, - }, - user_id: UserID::default(), - selection: Selection::None, - init_block_id: BlockId::default(), - input_replica_id: InputReplicaId::default(), - telemetry_context: None, - lifetime: Lifetime::default(), - universal_developer_input_context: None, - source_type: SessionSourceType::User, - source_task_id: None, - feature_support: FeatureSupport::default(), - }; - let mut value = serde_json::to_value(payload).unwrap(); - value.as_object_mut().unwrap().remove("source_task_id"); - let parsed: InitPayload = serde_json::from_value(value).unwrap(); - assert!(parsed.source_task_id.is_none()); - } -}