Skip to content

Commit 6ecf5e3

Browse files
tcconnallyclaude
andcommitted
fix(temporal): reject one-sided valid_from that inverts a stored valid_to on re-assert (#363)
Review finding (PR #370 verifier, round 3): the mirror image of the one-sided valid_to hole. On a content-UNCHANGED re-assert the entities UPDATE takes the caller's valid_from via COALESCE while KEEPING the stored valid_to (the content-changed stamp that would re-set valid_to never runs), so `remember {identical body, valid_from: >= stored close}` silently stored an inverted [vf, stored_to) period — unmatchable by valid_at at every instant. Symmetric guard in remember_impl: when valid_from is supplied without valid_to on the content-unchanged branch, validate it against the stored valid_to the write will keep; reject vf >= stored_to before any mutation, same error style. The content-changed and insert paths are unaffected — they re-set valid_to to the caller's value (NULL => unbounded), which a one-sided valid_from cannot invert. Tests: new reassert_rejects_one_sided_valid_from_at_or_after_stored_valid_to (after/at the stored close rejected with the period verified untouched; before the close accepted and moves the open; unbounded fact accepts any one-sided valid_from). Tightened the round-2 identical-re-assert case to assert the stored period is unchanged, not just the error string. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2f6b804 commit 6ecf5e3

2 files changed

Lines changed: 128 additions & 1 deletion

File tree

src/db.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,6 +1769,31 @@ impl Database {
17691769
)
17701770
.into());
17711771
}
1772+
} else if let Some(vf) = valid_from {
1773+
// #363 review (round 3): the mirror-image hole. On a
1774+
// content-UNCHANGED re-assert the UPDATE below takes the
1775+
// caller's valid_from (COALESCE ?20) while KEEPING the stored
1776+
// valid_to, so a one-sided valid_from at/after the stored
1777+
// close would store [vf, stored_to) — inverted. Only this
1778+
// branch can inherit a bound: on a content change the stamp
1779+
// UPDATE re-sets valid_to to the caller's value (NULL here,
1780+
// i.e. [vf, ∞)), which cannot invert.
1781+
if !content_changed {
1782+
let stored_to: Option<i64> = conn.query_row(
1783+
"SELECT valid_to_unix_ms FROM entities WHERE id = ?1",
1784+
params![id],
1785+
|r| r.get(0),
1786+
)?;
1787+
if let Some(et) = stored_to {
1788+
if vf >= et {
1789+
return Err(format!(
1790+
"valid_from_unix_ms ({vf}) must be less than the fact's effective \
1791+
valid_to ({et}) — refusing to invert the valid period"
1792+
)
1793+
.into());
1794+
}
1795+
}
1796+
}
17721797
}
17731798

17741799
// M-1: wrap entity UPDATE + FTS UPDATE in a transaction

src/tools.rs

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2864,14 +2864,116 @@ mod tests {
28642864
assert!(body.contains("v1"), "rejected write must not update the entity: {body}");
28652865

28662866
// (c) Existing entity, identical body (COALESCE re-assert path):
2867-
// valid_to is validated against the STORED valid_from.
2867+
// valid_to is validated against the STORED valid_from — and the
2868+
// rejected write must leave the STORED PERIOD untouched.
2869+
let stored_period = |db: &Database| -> (Value, Value) {
2870+
let r = handle_valid_at(
2871+
&db,
2872+
json!({"category": "facts", "key": "one-sided", "valid_at_unix_ms": now_ms()}),
2873+
)
2874+
.expect("valid_at");
2875+
let v: Value = serde_json::from_str(&r).unwrap();
2876+
assert_eq!(v["found"], json!(true), "{r}");
2877+
(v["valid_from_unix_ms"].clone(), v["valid_to_unix_ms"].clone())
2878+
};
2879+
let before = stored_period(&db);
28682880
let err = handle_remember(
28692881
&db,
28702882
json!({"category": "facts", "key": "one-sided", "body_json": "{\"note\":\"v1\"}",
28712883
"valid_to_unix_ms": past}),
28722884
)
28732885
.expect_err("one-sided past valid_to on an identical re-assert must be rejected");
28742886
assert!(err.contains("valid_to_unix_ms"), "got: {err}");
2887+
assert_eq!(
2888+
stored_period(&db),
2889+
before,
2890+
"rejected re-assert must not change the stored valid period"
2891+
);
2892+
2893+
let _ = std::fs::remove_file(&path);
2894+
}
2895+
2896+
#[test]
2897+
fn reassert_rejects_one_sided_valid_from_at_or_after_stored_valid_to() {
2898+
// #363 review (round 3): the mirror-image hole. On an identical-body
2899+
// re-assert the UPDATE takes the caller's valid_from via COALESCE
2900+
// while KEEPING the stored valid_to — so a one-sided valid_from
2901+
// at/after the stored close would store [vf, stored_to): inverted,
2902+
// unanswerable at every instant.
2903+
let (db, path) = temp_db();
2904+
let now = now_ms();
2905+
let vf = now - 100_000;
2906+
let vt = now - 50_000;
2907+
let body = "{\"note\":\"bounded\"}";
2908+
2909+
handle_remember(
2910+
&db,
2911+
json!({"category": "facts", "key": "mirror", "body_json": body,
2912+
"valid_from_unix_ms": vf, "valid_to_unix_ms": vt}),
2913+
)
2914+
.expect("bounded fact");
2915+
2916+
let stored_period = |db: &Database| -> (Value, Value) {
2917+
let r = handle_valid_at(
2918+
&db,
2919+
json!({"category": "facts", "key": "mirror", "valid_at_unix_ms": vt - 1_000}),
2920+
)
2921+
.expect("valid_at");
2922+
let v: Value = serde_json::from_str(&r).unwrap();
2923+
assert_eq!(v["found"], json!(true), "{r}");
2924+
(v["valid_from_unix_ms"].clone(), v["valid_to_unix_ms"].clone())
2925+
};
2926+
assert_eq!(stored_period(&db), (json!(vf), json!(vt)));
2927+
2928+
// (a) valid_from strictly after the stored close: inverted, rejected.
2929+
let err = handle_remember(
2930+
&db,
2931+
json!({"category": "facts", "key": "mirror", "body_json": body,
2932+
"valid_from_unix_ms": vt + 10_000}),
2933+
)
2934+
.expect_err("one-sided valid_from after the stored valid_to must be rejected");
2935+
assert!(err.contains("valid_from_unix_ms"), "got: {err}");
2936+
2937+
// (b) valid_from exactly AT the stored close: empty period, rejected.
2938+
let err = handle_remember(
2939+
&db,
2940+
json!({"category": "facts", "key": "mirror", "body_json": body,
2941+
"valid_from_unix_ms": vt}),
2942+
)
2943+
.expect_err("one-sided valid_from at the stored valid_to must be rejected");
2944+
assert!(err.contains("valid_from_unix_ms"), "got: {err}");
2945+
2946+
// Rejected writes left the stored period untouched.
2947+
assert_eq!(
2948+
stored_period(&db),
2949+
(json!(vf), json!(vt)),
2950+
"rejected re-asserts must not change the stored valid period"
2951+
);
2952+
2953+
// (c) Legitimate one-sided valid_from strictly BEFORE the stored
2954+
// close: accepted, and it moves the open while keeping the close.
2955+
let new_vf = vf - 10_000;
2956+
handle_remember(
2957+
&db,
2958+
json!({"category": "facts", "key": "mirror", "body_json": body,
2959+
"valid_from_unix_ms": new_vf}),
2960+
)
2961+
.expect("one-sided valid_from before the stored valid_to must be accepted");
2962+
assert_eq!(stored_period(&db), (json!(new_vf), json!(vt)));
2963+
2964+
// (d) No stored valid_to (unbounded fact): any one-sided valid_from
2965+
// yields [vf, infinity) — accepted.
2966+
handle_remember(
2967+
&db,
2968+
json!({"category": "facts", "key": "unbounded", "body_json": body}),
2969+
)
2970+
.expect("unbounded fact");
2971+
handle_remember(
2972+
&db,
2973+
json!({"category": "facts", "key": "unbounded", "body_json": body,
2974+
"valid_from_unix_ms": now + 60_000}),
2975+
)
2976+
.expect("one-sided valid_from on an unbounded fact must be accepted");
28752977

28762978
let _ = std::fs::remove_file(&path);
28772979
}

0 commit comments

Comments
 (0)