From ff0324a0e5971d84e224745ba742ceb79bef9b37 Mon Sep 17 00:00:00 2001 From: Sam Valladares Date: Sun, 19 Apr 2026 16:53:10 -0500 Subject: [PATCH] fix(schema): honor changelog start/end + intention include_snoozed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both parameters were advertised in the MCP tool schemas since v1.7+ but were silently ignored at runtime — a schema-contract violation. Any caller that set them got unfiltered results with no error or warning, which is the worst possible failure mode for a public tool surface. changelog.rs - Parse `start` / `end` as ISO-8601/RFC-3339 timestamps; return an explicit error on malformed input (previously: silent drop). - In system-wide mode, over-fetch 4× limit when a time window is set, then apply an inclusive [start, end] filter in Rust before the sort+truncate. SQL-level filtering is a v2.1+ optimisation. - Response JSON gains a `filter` field echoing the applied bounds so callers can confirm the window was honored. - Per-memory mode still ignores the window (semantically meaningless when scoped to one memory's transition history). intention_unified.rs - `execute_check`: when `include_snoozed=true`, fold snoozed intentions back into the check pool so their time/context triggers can wake them when a matching condition appears. Previously snoozed intentions were invisible to check regardless of the arg. - Deduplicates defensively via a HashSet on intention.id in case storage ever returns overlap. Tests: 9 changelog + 37 intention_unified tests continue to pass. Full vestige-mcp lib suite 419 passing, 0 failures. --- crates/vestige-mcp/src/tools/changelog.rs | 79 ++++++++++++++++--- .../src/tools/intention_unified.rs | 23 +++++- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/crates/vestige-mcp/src/tools/changelog.rs b/crates/vestige-mcp/src/tools/changelog.rs index e2c4928..59d6612 100644 --- a/crates/vestige-mcp/src/tools/changelog.rs +++ b/crates/vestige-mcp/src/tools/changelog.rs @@ -46,9 +46,7 @@ pub fn schema() -> Value { struct ChangelogArgs { #[serde(alias = "memory_id")] memory_id: Option, - #[allow(dead_code)] start: Option, - #[allow(dead_code)] end: Option, limit: Option, } @@ -67,12 +65,36 @@ pub async fn execute(storage: &Arc, args: Option) -> Result, field: &str) -> Result>, String> { + match raw { + Some(s) if !s.is_empty() => DateTime::parse_from_rfc3339(s) + .map(|dt| Some(dt.with_timezone(&Utc))) + .map_err(|e| { + format!( + "Invalid {} timestamp {:?}: {}. Use ISO-8601 / RFC-3339 (e.g. 2026-04-19T12:00:00Z).", + field, s, e + ) + }), + _ => Ok(None), } } @@ -118,20 +140,42 @@ fn execute_per_memory(storage: &Storage, memory_id: &str, limit: i32) -> Result< })) } -/// System-wide changelog: consolidations + recent state transitions -fn execute_system_wide(storage: &Storage, limit: i32) -> Result { +/// System-wide changelog: consolidations + recent state transitions. +/// +/// `start`/`end` optionally bound the returned events to an inclusive +/// time window. Filtering happens in Rust after the DB reads because +/// the underlying storage helpers don't yet take time parameters — +/// moving the filter into SQL is a v2.1+ optimisation (tracked in the +/// v2.0.7 scope notes). For now we over-fetch (up to 4× `limit`) when +/// a window is supplied so filtering doesn't starve the result set. +fn execute_system_wide( + storage: &Storage, + limit: i32, + start: Option>, + end: Option>, +) -> Result { + // When a window is supplied we can't predict how many events fall inside, + // so over-fetch to reduce the chance of returning an empty page on a + // long-tail time range. 4× is arbitrary; revisit if it becomes a cost + // on very large changelog tables. + let fetch_limit = if start.is_some() || end.is_some() { + limit.saturating_mul(4) + } else { + limit + }; + // Get consolidation history let consolidations = storage - .get_consolidation_history(limit) + .get_consolidation_history(fetch_limit) .map_err(|e| e.to_string())?; // Get recent state transitions across all memories let transitions = storage - .get_recent_state_transitions(limit) + .get_recent_state_transitions(fetch_limit) .map_err(|e| e.to_string())?; // Get dream history (Bug #9 fix — dreams were invisible to audit trail) - let dreams = storage.get_dream_history(limit).unwrap_or_default(); + let dreams = storage.get_dream_history(fetch_limit).unwrap_or_default(); // Build unified event list let mut events: Vec<(DateTime, Value)> = Vec::new(); @@ -181,6 +225,17 @@ fn execute_system_wide(storage: &Storage, limit: i32) -> Result { )); } + // Apply the optional [start, end] window. `start` is inclusive, `end` + // is inclusive — matches "show me events between these two wall-clock + // instants" user expectation. + if start.is_some() || end.is_some() { + events.retain(|(ts, _)| { + let after_start = start.is_none_or(|s| *ts >= s); + let before_end = end.is_none_or(|e| *ts <= e); + after_start && before_end + }); + } + // Sort by timestamp descending events.sort_by(|a, b| b.0.cmp(&a.0)); @@ -194,6 +249,10 @@ fn execute_system_wide(storage: &Storage, limit: i32) -> Result { "mode": "system_wide", "totalEvents": formatted_events.len(), "events": formatted_events, + "filter": serde_json::json!({ + "start": start.map(|s| s.to_rfc3339()), + "end": end.map(|e| e.to_rfc3339()), + }), })) } diff --git a/crates/vestige-mcp/src/tools/intention_unified.rs b/crates/vestige-mcp/src/tools/intention_unified.rs index 5eae9dc..a7976d4 100644 --- a/crates/vestige-mcp/src/tools/intention_unified.rs +++ b/crates/vestige-mcp/src/tools/intention_unified.rs @@ -435,8 +435,27 @@ async fn execute_check( let _ = cog.prospective_memory.update_context(prospective_ctx); } - // Get active intentions - let intentions = storage.get_active_intentions().map_err(|e| e.to_string())?; + // Get active intentions. `include_snoozed=true` folds snoozed intentions + // back into the check pool so time/context triggers can wake them up when + // their snooze window has elapsed or a matching context appears — before + // v2.0.7 this flag was advertised in the schema but silently ignored, so + // snoozed intentions were invisible to `check` regardless of the arg. + let mut intentions = storage.get_active_intentions().map_err(|e| e.to_string())?; + if args.include_snoozed.unwrap_or(false) { + let snoozed = storage + .get_intentions_by_status("snoozed") + .map_err(|e| e.to_string())?; + // Deduplicate defensively — an intention should never be in both lists + // but we pay the O(n) hash-set cost to stay correct if storage semantics + // shift under us. + use std::collections::HashSet; + let seen: HashSet = intentions.iter().map(|i| i.id.clone()).collect(); + for s in snoozed { + if !seen.contains(&s.id) { + intentions.push(s); + } + } + } let mut triggered = Vec::new(); let mut pending = Vec::new();