From b7a021417b703b807b8dc447ce441793e1a0764d Mon Sep 17 00:00:00 2001 From: Sam Valladares <143034159+samvallad33@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:28:13 -0500 Subject: [PATCH] fix(backend): wire the 3 deferred dead-code issues (#117, #124, #137) (#140) - #117 Agent Black Box: the trace recorder had zero production callers, so agent_traces/receipts/memory_prs never populated. handle_tools_call now records the opening mcp.call event (record_call) before dispatch and the downstream memory events (record_result) after a successful call, under a shared run_id. Also fixed the args capture so tracing works in pure stdio mode (was gated on event_tx, which is None without a dashboard socket). - #124 Recurring intentions now re-arm: mark_triggered advances a recurring trigger's next_occurrence and returns the intention to Active (was firing once and staying Triggered forever). Added Trigger::re_arm (recurses into Compound). - #137 Co-access prediction channel is now live: search_unified feeds the retrieved set to speculative_retriever.record_usage, so co-access patterns populate (were permanently empty). The temporal channel was already fixed (#136); the file-context channel stays dormant by design (search has no file context to supply). Tests: Black Box trace population, recurring re-arm. Full workspace green (1558 tests, clippy -D warnings clean). Co-authored-by: Claude Opus 4.8 (1M context) --- .../src/neuroscience/prospective_memory.rs | 69 ++++++++++++++++++- crates/vestige-mcp/src/server.rs | 65 +++++++++++++++-- .../vestige-mcp/src/tools/search_unified.rs | 10 +++ 3 files changed, 136 insertions(+), 8 deletions(-) diff --git a/crates/vestige-core/src/neuroscience/prospective_memory.rs b/crates/vestige-core/src/neuroscience/prospective_memory.rs index cf310ce..6793780 100644 --- a/crates/vestige-core/src/neuroscience/prospective_memory.rs +++ b/crates/vestige-core/src/neuroscience/prospective_memory.rs @@ -412,6 +412,35 @@ impl IntentionTrigger { } } + /// Advance a recurring trigger to its next occurrence after it fires, so it + /// re-arms instead of staying perpetually triggered. Returns true if the + /// trigger is recurring and was advanced (the intention should return to + /// Active), false for one-shot triggers (which stay fulfilled/triggered). + /// Recurs into Compound so a recurring sub-trigger inside a compound re-arms. + pub fn re_arm(&mut self, now: DateTime) -> bool { + match self { + Self::Recurring { + recurrence, + next_occurrence, + .. + } => { + // Advance from the later of "now" and the slot that just fired, + // so we never re-arm into an already-past occurrence. + let from = next_occurrence.map(|t| t.max(now)).unwrap_or(now); + *next_occurrence = Some(recurrence.next_occurrence(from)); + true + } + Self::Compound { all_of, any_of } => { + let mut any = false; + for t in all_of.iter_mut().chain(any_of.iter_mut()) { + any |= t.re_arm(now); + } + any + } + _ => false, + } + } + /// Get a human-readable description of the trigger pub fn description(&self) -> String { match self { @@ -708,9 +737,18 @@ impl Intention { /// Mark as triggered pub fn mark_triggered(&mut self) { - self.status = IntentionStatus::Triggered; + let now = Utc::now(); self.reminder_count += 1; - self.last_reminded_at = Some(Utc::now()); + self.last_reminded_at = Some(now); + // A recurring intention re-arms to its next occurrence and returns to + // Active so it fires again; a one-shot intention stays Triggered. + // Previously recurring intentions never advanced next_occurrence, so they + // fired once and never again (or stayed perpetually triggered). + if self.trigger.re_arm(now) { + self.status = IntentionStatus::Active; + } else { + self.status = IntentionStatus::Triggered; + } } /// Mark as fulfilled @@ -1696,6 +1734,33 @@ mod tests { assert!((next - now) == Duration::hours(2)); } + #[test] + fn test_recurring_intention_re_arms_after_firing() { + // Regression (#124): a recurring intention must re-arm (advance + // next_occurrence + return to Active) each time it fires, not fire once + // and stay Triggered forever. + let now = Utc::now(); + let trigger = IntentionTrigger::Recurring { + base: Box::new(IntentionTrigger::at_time(now - Duration::minutes(1))), + recurrence: RecurrencePattern::EveryHours(2), + next_occurrence: Some(now - Duration::minutes(1)), // already due + }; + let mut intention = Intention::new("recurring task", trigger); + + // Due now, so it triggers. + assert!(intention.trigger.is_triggered(&Context::default(), &[])); + intention.mark_triggered(); + + // After firing it must be Active again (re-armed), not stuck Triggered. + assert_eq!(intention.status, IntentionStatus::Active); + // And next_occurrence must have advanced into the future so it is no + // longer immediately due. + assert!( + !intention.trigger.is_triggered(&Context::default(), &[]), + "recurring trigger must not stay perpetually due after re-arming" + ); + } + #[test] fn test_stats() { let pm = ProspectiveMemory::new(); diff --git a/crates/vestige-mcp/src/server.rs b/crates/vestige-mcp/src/server.rs index 571e2c6..b47d561 100644 --- a/crates/vestige-mcp/src/server.rs +++ b/crates/vestige-mcp/src/server.rs @@ -464,12 +464,25 @@ impl McpServer { cog.consolidation_scheduler.record_activity(); } - // Save args for event emission (tool dispatch consumes request.arguments) - let saved_args = if self.event_tx.is_some() { - request.arguments.clone() - } else { - None - }; + // Capture the args for tracing/event emission BEFORE tool dispatch + // consumes request.arguments. The Black Box must populate even in pure + // stdio mode (no dashboard socket), so this is NOT gated on event_tx — + // only the WebSocket broadcast inside record()/emit is. + let saved_args = request.arguments.clone(); + + // Agent Black Box: record the opening mcp.call event for this tool + // invocation. run_id groups the events of one agent turn; it is derived + // from the args (an explicit runId, or a fresh id) so record_result below + // can attach the downstream memory events (retrieve/suppress/veto) to the + // same run. + let trace_run_id = crate::trace_recorder::run_id_for(&saved_args); + crate::trace_recorder::record_call( + &self.storage, + self.event_tx.as_ref(), + &trace_run_id, + &request.name, + &saved_args, + ); let result = match request.name.as_str() { // ================================================================ @@ -1129,6 +1142,17 @@ impl McpServer { // ================================================================ if let Ok(ref content) = result { self.emit_tool_event(&request.name, &saved_args, content); + // Agent Black Box: inspect the successful result and record the + // downstream memory events (retrieve/suppress/veto/dream) under the + // same run_id as the opening mcp.call, so /api/traces, /api/receipts + // and the trace:// resource are actually populated. + crate::trace_recorder::record_result( + &self.storage, + self.event_tx.as_ref(), + &trace_run_id, + &request.name, + content, + ); } let response = match result { @@ -2449,6 +2473,35 @@ mod tests { assert_eq!(response.error.unwrap().code, -32602); // InvalidParams } + #[tokio::test] + async fn test_tool_call_populates_agent_black_box_trace() { + // Regression (#117): the trace recorder was dead code — no production + // caller — so agent_traces/receipts/memory_prs never populated. A tool + // call must now record at least the opening mcp.call event under the + // supplied runId. + let (mut server, _dir) = test_server().await; + server + .handle_request(make_request("initialize", Some(init_params()))) + .await; + + let run_id = "test-run-blackbox"; + let request = make_request( + "tools/call", + Some(serde_json::json!({ + "name": "search", + "arguments": { "query": "anything", "runId": run_id } + })), + ); + let response = server.handle_request(request).await.unwrap(); + assert!(response.error.is_none(), "tool call should succeed"); + + let events = server.storage.get_trace(run_id).expect("read trace"); + assert!( + !events.is_empty(), + "the Black Box must record at least the mcp.call event for this run" + ); + } + #[tokio::test] async fn test_tools_call_invalid_params_returns_error() { let (mut server, _dir) = test_server().await; diff --git a/crates/vestige-mcp/src/tools/search_unified.rs b/crates/vestige-mcp/src/tools/search_unified.rs index 9c20def..8c87add 100644 --- a/crates/vestige-mcp/src/tools/search_unified.rs +++ b/crates/vestige-mcp/src/tools/search_unified.rs @@ -781,6 +781,16 @@ pub async fn execute( }; cog.reconsolidation.mark_labile(&result.node.id, snapshot); } + + // 7D. Feed the co-access channel: the memories returned together by one + // query WERE accessed together, so record_usage learns their co-access + // patterns (trigger -> predicted). Without this call the speculative + // retriever's co-access prediction channel stays permanently empty. + if filtered_results.len() >= 2 { + let accessed: Vec = + filtered_results.iter().map(|r| r.node.id.clone()).collect(); + cog.speculative_retriever.record_usage(&[], &accessed); + } } // ====================================================================