From f1a1fb262183923b80451928ebc447500ed93efe Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Fri, 16 Oct 2026 15:59:41 +0300 Subject: [PATCH] fix(schedule): omit an untouched agent_id from a serialized CronJobPatch The patch's agent_id is Option> with a deserializer that reads a present null as a clear. Without skip_serializing_if a patch that left the agent alone serialized agent_id: null, so any CronJobPatch sent over the bus cleared the job's agent definition. Co-authored-by: Medulla --- crates/tinyflows-bus/src/schedule/types.rs | 11 ++++++++- .../tinyflows-bus/src/schedule/wire_tests.rs | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/tinyflows-bus/src/schedule/types.rs b/crates/tinyflows-bus/src/schedule/types.rs index 251ffc81..8c9f93a5 100644 --- a/crates/tinyflows-bus/src/schedule/types.rs +++ b/crates/tinyflows-bus/src/schedule/types.rs @@ -441,7 +441,16 @@ pub struct CronJobPatch { /// "clear the agent definition" (`Some(None)`). See /// [`deserialize_double_option`] for why the custom deserializer is required /// to honor a wire `null` as a clear rather than a silent no-op. - #[serde(default, deserialize_with = "deserialize_double_option")] + /// + /// Omitted when serialized for `None`: the custom deserializer reads a + /// present `null` as a clear, so a patch that leaves the agent alone must + /// not write `"agent_id": null` or crossing a serialized boundary (the bus) + /// would turn every unrelated patch into "clear the agent definition". + #[serde( + default, + deserialize_with = "deserialize_double_option", + skip_serializing_if = "Option::is_none" + )] pub agent_id: Option>, /// Set (`Some(Some(_))`) or clear (`Some(None)`) the job's origin /// conversation; `None` leaves it unchanged (and is omitted when diff --git a/crates/tinyflows-bus/src/schedule/wire_tests.rs b/crates/tinyflows-bus/src/schedule/wire_tests.rs index 1d60bfe8..0d5593e0 100644 --- a/crates/tinyflows-bus/src/schedule/wire_tests.rs +++ b/crates/tinyflows-bus/src/schedule/wire_tests.rs @@ -137,3 +137,26 @@ fn job_with_origin_and_run_with_delivery_status_wire_bytes() { let run: CronRun = serde_json::from_str(run_json).unwrap(); assert_eq!(serde_json::to_string(&run).unwrap(), run_json); } + +/// A patch that does not mention the agent definition must still not mention it +/// after a trip over the wire; only an explicit clear writes `null`. +#[test] +fn an_unrelated_patch_does_not_clear_the_agent_definition_on_a_round_trip() { + let untouched = CronJobPatch { + enabled: Some(false), + ..CronJobPatch::default() + }; + let wire = serde_json::to_value(&untouched).unwrap(); + assert!(wire.get("agent_id").is_none(), "{wire}"); + let back: CronJobPatch = serde_json::from_value(wire).unwrap(); + assert_eq!(back.agent_id, None); + + let cleared = CronJobPatch { + agent_id: Some(None), + ..CronJobPatch::default() + }; + let wire = serde_json::to_value(&cleared).unwrap(); + assert_eq!(wire["agent_id"], serde_json::Value::Null); + let back: CronJobPatch = serde_json::from_value(wire).unwrap(); + assert_eq!(back.agent_id, Some(None)); +}