Repository navigation
fix(schedule): omit an untouched agent_id from a serialized CronJobPatch - #144
Conversation
The patch's agent_id is Option<Option<String>> 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 <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper completed its review; deterministic results follow. State: Ready for maintainer review Review snapshot
Completeness: Complete What changed`CronJobPatch::agent_id` in `crates/tinyflows-bus/src/schedule/types.rs` now carries `skip_serializing_if = "Option::is_none"` alongside its existing `default` and `deserialize_with = "deserialize_double_option"` attributes, and gains doc text explaining that an omitted field means "leave the agent alone" while a present `null` means "clear the agent definition". Behaviourally, a patch that does not mention the agent definition no longer serializes `"agent_id": null` across a serialized boundary such as the bus, which previously would have been decoded as an explicit clear and turned every unrelated patch into a clear-the-agent operation. Explicit clears still serialize as `null`. A corresponding round-trip test was added in `crates/tinyflows-bus/src/schedule/wire_tests.rs`. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. How this fits togetherflowchart LR
n0["CronJobPatch<br/>changed"]:::changed
n1["update_job"]:::impacted
n2["add_shell_job"]:::impacted
n3["get_job"]:::impacted
n4["CronJob"]:::impacted
n5["complete_run"]:::impacted
n1 -->|uses| n0
n1 -->|uses| n4
n2 -->|uses| n4
n3 -->|uses| n4
n5 -->|uses| n4
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0013 · 152,364 in / 7,583 out · 31,744 cached (21%) · ladder/vectors-oai3, deepseek-v4.1-flash · 141 embedded
critique: $0.0001 · 47,579 in / 671 out · 17,152 cached (36%) · deepseek-v4.1-flash
security: $0.0000 · 45,306 in / 439 out · 5,504 cached (12%) · deepseek-v4.1-flash
tests: $0.0000 · 31,036 in / 398 out · 4,992 cached (16%) · deepseek-v4.1-flash
description: $0.0010 · 26,347 in / 4,499 out · 4,096 cached (16%) · deepseek-v4.1-flash
Why
CronJobPatch.agent_idis a double option whose custom deserializer reads a presentnullas "clear the agent definition". Serialization wrote"agent_id": nullforNone(leave untouched), so any patch that crossed the bus (OpenHuman #7397's cron cutover onto the Schedule contract) wiped the job's agent definition.Change
skip_serializing_if = "Option::is_none"onagent_id, matching the siblingoriginfield.Test
schedule::wire_tests::an_unrelated_patch_does_not_clear_the_agent_definition_on_a_round_tripfails without the change (verified) and passes with it.cargo test -p tinyflows-bus -p tinyflows-scheduleis green.Summary by CodeRabbit