hive-sh4re: one saturating_age for every loose-end producer
`age_seconds` is documented on the LooseEnd enum as saturating to zero on any clock anomaly, but the derivation was in three places: hive-c0re had a named `saturating_age` helper with tests, and the in-agent socket server hand-rolled the same two lines twice, untested. Move the helper to hive-sh4re::inbox, beside the enum whose contract it implements and inside the one crate both producers already depend on. Its three tests move with it (not dropped) and gain two arms: the whole-i64 range, where the saturating_sub is what stops the subtraction overflowing, and a far-past control so those zeros are the clamp firing rather than the function bottoming out on large inputs. The two clamps are not redundant, which is what `to_loose_end`'s doc got wrong: it credited "saturating" for the zero, but saturating_sub bottoms out at i64::MIN, still negative. The try_from is what yields 0. Also cover the two projections themselves, which is the part the shared helper cannot: that a reminder ages from created_at rather than due_at, and a todo from updated_at, with a future timestamp reading 0 through both and a past-timestamp control on each.
This commit is contained in:
parent
576294012b
commit
1e67f56249
3 changed files with 193 additions and 36 deletions
|
|
@ -26,7 +26,7 @@ use std::sync::Arc;
|
||||||
|
|
||||||
use anyhow::{Context, Result};
|
use anyhow::{Context, Result};
|
||||||
use hive_agent_sock::{Request, Response};
|
use hive_agent_sock::{Request, Response};
|
||||||
use hive_sh4re::inbox::LooseEnd;
|
use hive_sh4re::inbox::{LooseEnd, saturating_age};
|
||||||
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
|
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
|
||||||
use tokio::net::{UnixListener, UnixStream};
|
use tokio::net::{UnixListener, UnixStream};
|
||||||
use tokio::sync::Notify;
|
use tokio::sync::Notify;
|
||||||
|
|
@ -430,13 +430,12 @@ fn no_reminders_store() -> Response {
|
||||||
/// it's due).
|
/// it's due).
|
||||||
fn reminder_to_loose_end(r: Reminder) -> LooseEnd {
|
fn reminder_to_loose_end(r: Reminder) -> LooseEnd {
|
||||||
let now = chrono::Utc::now().timestamp();
|
let now = chrono::Utc::now().timestamp();
|
||||||
let age = u64::try_from(now.saturating_sub(r.created_at.timestamp())).unwrap_or(0);
|
|
||||||
LooseEnd::Reminder {
|
LooseEnd::Reminder {
|
||||||
id: r.id,
|
id: r.id,
|
||||||
owner: crate::identity::label(),
|
owner: crate::identity::label(),
|
||||||
message: r.message,
|
message: r.message,
|
||||||
due_at: r.due_at,
|
due_at: r.due_at,
|
||||||
age_seconds: age,
|
age_seconds: saturating_age(now, r.created_at.timestamp()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -448,16 +447,140 @@ fn err(e: &anyhow::Error) -> Response {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Map a stored [`Todo`] to a [`LooseEnd::Todo`], deriving `age_seconds`
|
/// Map a stored [`Todo`] to a [`LooseEnd::Todo`], deriving `age_seconds`
|
||||||
/// from `updated_at` (saturating so a backwards clock step reads 0).
|
/// from `updated_at` via [`saturating_age`].
|
||||||
fn to_loose_end(t: Todo) -> LooseEnd {
|
fn to_loose_end(t: Todo) -> LooseEnd {
|
||||||
let now = chrono::Utc::now().timestamp();
|
let now = chrono::Utc::now().timestamp();
|
||||||
let age = u64::try_from(now.saturating_sub(t.updated_at.timestamp())).unwrap_or(0);
|
|
||||||
LooseEnd::Todo {
|
LooseEnd::Todo {
|
||||||
id: t.id,
|
id: t.id,
|
||||||
subsystem: t.subsystem,
|
subsystem: t.subsystem,
|
||||||
subsystem_key: t.subsystem_key,
|
subsystem_key: t.subsystem_key,
|
||||||
summary: t.summary,
|
summary: t.summary,
|
||||||
source: t.source,
|
source: t.source,
|
||||||
age_seconds: age,
|
age_seconds: saturating_age(now, t.updated_at.timestamp()),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use chrono::{DateTime, Duration, Utc};
|
||||||
|
|
||||||
|
/// `saturating_age`'s own arms are exact (`hive-sh4re::inbox`); what
|
||||||
|
/// is only checkable here is *which* stored timestamp each projection
|
||||||
|
/// feeds it, and both read the clock internally — so these assert a
|
||||||
|
/// range wide enough that a slow runner can't fail them.
|
||||||
|
const SLACK: u64 = 600;
|
||||||
|
|
||||||
|
/// Not asserted anywhere below: `LooseEnd::Reminder.owner`. It comes
|
||||||
|
/// from `identity::label()`, which reads a process-global env var
|
||||||
|
/// that `identity`'s own tests set and restore — in the same test
|
||||||
|
/// binary, in parallel. Any value assertion here would be a race, not
|
||||||
|
/// a check.
|
||||||
|
fn reminder(created_at: DateTime<Utc>, due_at: DateTime<Utc>) -> Reminder {
|
||||||
|
Reminder {
|
||||||
|
id: 7,
|
||||||
|
message: "check the gate".to_owned(),
|
||||||
|
file_path: None,
|
||||||
|
due_at,
|
||||||
|
created_at,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn todo(updated_at: DateTime<Utc>) -> Todo {
|
||||||
|
Todo {
|
||||||
|
id: 11,
|
||||||
|
subsystem: "matrix".to_owned(),
|
||||||
|
subsystem_key: Some("!room:x".to_owned()),
|
||||||
|
summary: "3 unread".to_owned(),
|
||||||
|
source: Some("#ops".to_owned()),
|
||||||
|
updated_at,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_reminder_ages_from_created_at_not_from_when_it_is_due() {
|
||||||
|
// Scheduled 600s ago, due an hour out. Reading the age off
|
||||||
|
// `due_at` would clamp to 0, so the two fields are separable.
|
||||||
|
let r = reminder(
|
||||||
|
Utc::now() - Duration::seconds(600),
|
||||||
|
Utc::now() + Duration::seconds(3600),
|
||||||
|
);
|
||||||
|
let due = r.due_at;
|
||||||
|
match reminder_to_loose_end(r) {
|
||||||
|
LooseEnd::Reminder {
|
||||||
|
id,
|
||||||
|
message,
|
||||||
|
due_at,
|
||||||
|
age_seconds,
|
||||||
|
..
|
||||||
|
} => {
|
||||||
|
assert_eq!(id, 7);
|
||||||
|
assert_eq!(message, "check the gate");
|
||||||
|
assert_eq!(due_at, due, "due_at passes through untouched");
|
||||||
|
assert!(
|
||||||
|
(600..600 + SLACK).contains(&age_seconds),
|
||||||
|
"age {age_seconds} should track created_at, not due_at"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
other => panic!("expected a Reminder loose end, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_todo_ages_from_updated_at_and_carries_its_dedup_key() {
|
||||||
|
let t = todo(Utc::now() - Duration::seconds(600));
|
||||||
|
match to_loose_end(t) {
|
||||||
|
LooseEnd::Todo {
|
||||||
|
id,
|
||||||
|
subsystem,
|
||||||
|
subsystem_key,
|
||||||
|
summary,
|
||||||
|
source,
|
||||||
|
age_seconds,
|
||||||
|
} => {
|
||||||
|
assert_eq!(id, 11);
|
||||||
|
assert_eq!(subsystem, "matrix");
|
||||||
|
// Distinct values: the two `Option<String>` fields are
|
||||||
|
// adjacent and same-typed, so a swap has to be visible.
|
||||||
|
assert_eq!(subsystem_key.as_deref(), Some("!room:x"));
|
||||||
|
assert_eq!(source.as_deref(), Some("#ops"));
|
||||||
|
assert_eq!(summary, "3 unread");
|
||||||
|
assert!((600..600 + SLACK).contains(&age_seconds));
|
||||||
|
}
|
||||||
|
other => panic!("expected a Todo loose end, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_timestamp_in_the_future_reads_as_zero_through_both_projections() {
|
||||||
|
// Not a hypothetical: these stores are written by in-container
|
||||||
|
// producers, and `updated_at` crossing `now` is exactly what a
|
||||||
|
// clock sync during a turn produces.
|
||||||
|
let ahead = Utc::now() + Duration::seconds(3600);
|
||||||
|
let behind = Utc::now() - Duration::seconds(600);
|
||||||
|
|
||||||
|
let (future_r, past_r) = (age_of_reminder(ahead), age_of_reminder(behind));
|
||||||
|
let (future_t, past_t) = (age_of_todo(ahead), age_of_todo(behind));
|
||||||
|
|
||||||
|
assert_eq!(future_r, 0);
|
||||||
|
assert_eq!(future_t, 0);
|
||||||
|
// Controls: same call, past timestamp — so the zeros above are
|
||||||
|
// the clamp firing, not the projections always emitting 0.
|
||||||
|
assert!(past_r >= 600, "reminder control read {past_r}");
|
||||||
|
assert!(past_t >= 600, "todo control read {past_t}");
|
||||||
|
}
|
||||||
|
|
||||||
|
fn age_of_reminder(created_at: DateTime<Utc>) -> u64 {
|
||||||
|
match reminder_to_loose_end(reminder(created_at, Utc::now())) {
|
||||||
|
LooseEnd::Reminder { age_seconds, .. } => age_seconds,
|
||||||
|
other => panic!("expected a Reminder loose end, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn age_of_todo(updated_at: DateTime<Utc>) -> u64 {
|
||||||
|
match to_loose_end(todo(updated_at)) {
|
||||||
|
LooseEnd::Todo { age_seconds, .. } => age_seconds,
|
||||||
|
other => panic!("expected a Todo loose end, got {other:?}"),
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -20,7 +20,7 @@
|
||||||
|
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
use chrono::Utc;
|
use chrono::Utc;
|
||||||
use hive_sh4re::inbox::LooseEnd;
|
use hive_sh4re::inbox::{LooseEnd, saturating_age};
|
||||||
|
|
||||||
use crate::coordinator::Coordinator;
|
use crate::coordinator::Coordinator;
|
||||||
|
|
||||||
|
|
@ -93,32 +93,3 @@ pub fn hive_wide(coord: &Coordinator) -> Result<Vec<LooseEnd>> {
|
||||||
}
|
}
|
||||||
Ok(out)
|
Ok(out)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn saturating_age(now: i64, then: i64) -> u64 {
|
|
||||||
let delta = now.saturating_sub(then);
|
|
||||||
u64::try_from(delta).unwrap_or(0)
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(test)]
|
|
||||||
mod tests {
|
|
||||||
use super::*;
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn saturating_age_handles_clock_back_step() {
|
|
||||||
// `now` < `then`: caller's clock went backwards between rows.
|
|
||||||
// We saturate to 0 rather than returning a negative or
|
|
||||||
// wrapping around to ~u64::MAX (which would render as "27
|
|
||||||
// billion years ago" in the wake prompt).
|
|
||||||
assert_eq!(saturating_age(100, 200), 0);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn saturating_age_normal_case() {
|
|
||||||
assert_eq!(saturating_age(1_000_000, 999_990), 10);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn saturating_age_zero_when_equal() {
|
|
||||||
assert_eq!(saturating_age(42, 42), 0);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
|
||||||
|
|
@ -188,6 +188,23 @@ pub enum LooseEnd {
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Age in seconds between two unix timestamps, clamped to 0 when `then`
|
||||||
|
/// is in the future. This is what backs the `age_seconds` promise on
|
||||||
|
/// every [`LooseEnd`] variant above, so it lives beside the enum rather
|
||||||
|
/// than in either producer: hive-c0re derives approval ages here, and
|
||||||
|
/// hive-agent's in-agent socket server derives reminder + todo ages.
|
||||||
|
///
|
||||||
|
/// Both clamps are load-bearing and neither substitutes for the other:
|
||||||
|
/// `saturating_sub` keeps the subtraction itself from overflowing on
|
||||||
|
/// absurd inputs, and the `try_from` is what turns a negative delta into
|
||||||
|
/// 0 — on its own `saturating_sub` saturates towards `i64::MIN`, which is
|
||||||
|
/// still negative and would wrap to ~`u64::MAX` in an `as` cast.
|
||||||
|
#[must_use]
|
||||||
|
pub fn saturating_age(now: i64, then: i64) -> u64 {
|
||||||
|
let delta = now.saturating_sub(then);
|
||||||
|
u64::try_from(delta).unwrap_or(0)
|
||||||
|
}
|
||||||
|
|
||||||
/// Kind discriminator for `CancelLooseEnd`. Per-kind store +
|
/// Kind discriminator for `CancelLooseEnd`. Per-kind store +
|
||||||
/// authorisation rules live in
|
/// authorisation rules live in
|
||||||
/// `docs/process/conventions.md::Loose-ends wire shape`.
|
/// `docs/process/conventions.md::Loose-ends wire shape`.
|
||||||
|
|
@ -198,3 +215,49 @@ pub enum CancelLooseEndKind {
|
||||||
/// Withdraw a pending approval (manager surface only).
|
/// Withdraw a pending approval (manager surface only).
|
||||||
Approval,
|
Approval,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn saturating_age_normal_case() {
|
||||||
|
assert_eq!(saturating_age(1_000_000, 999_990), 10);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn saturating_age_zero_when_equal() {
|
||||||
|
assert_eq!(saturating_age(42, 42), 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn saturating_age_handles_clock_back_step() {
|
||||||
|
// `now` < `then`: the clock went backwards between rows. 0 beats
|
||||||
|
// both a negative and the ~u64::MAX an `as` cast would produce,
|
||||||
|
// which renders as "27 billion years ago" in the wake prompt.
|
||||||
|
assert_eq!(saturating_age(100, 200), 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_far_future_timestamp_stays_zero_rather_than_wrapping() {
|
||||||
|
// The back-step case above is one second of skew; this is the
|
||||||
|
// whole i64 range, where the subtraction itself saturates. Both
|
||||||
|
// must land on 0, and the pair is what pins the two clamps
|
||||||
|
// together — either one alone passes one of these and not both.
|
||||||
|
assert_eq!(saturating_age(i64::MIN, i64::MAX), 0);
|
||||||
|
assert_eq!(saturating_age(0, i64::MAX), 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_far_past_timestamp_is_a_real_age_not_a_clamp() {
|
||||||
|
// Control for the two arms above: the maximum representable
|
||||||
|
// delta still comes back as itself, so their 0 is the clamp
|
||||||
|
// firing and not this function bottoming out on large inputs.
|
||||||
|
let age = saturating_age(i64::MAX, i64::MIN);
|
||||||
|
assert_eq!(age, u64::try_from(i64::MAX).unwrap());
|
||||||
|
assert_eq!(
|
||||||
|
saturating_age(i64::MAX, 0),
|
||||||
|
u64::try_from(i64::MAX).unwrap()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue