From 9f8fee02169eac9a0ef48e07cd430753205eae32 Mon Sep 17 00:00:00 2001 From: Zach G Date: Sun, 5 Apr 2026 12:06:30 -0400 Subject: [PATCH 1/2] fix(ingestion): wire memory persistence contract to ingestion hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ingestion process_chunk function creates a MemoryPersistenceContractState and passes it to the tool server, but never connects it to the SpacebotHook. This means the hook's on_tool_call_result handler can never call set_terminal_outcome() because memory_persistence_contract is None. As a result, has_terminal_outcome() always returns false after prompt_once() completes, causing every chunk to fail with "completed without memory_persistence_complete signal" — even when the model correctly calls the tool and receives a success response. The fix adds .with_memory_persistence_contract(contract_state.clone()) to the hook construction, matching how branch.rs wires the same contract state (line 71). Fixes #538 --- src/agent/ingestion.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/agent/ingestion.rs b/src/agent/ingestion.rs index e2c9e69b1..f665d8ea7 100644 --- a/src/agent/ingestion.rs +++ b/src/agent/ingestion.rs @@ -520,7 +520,8 @@ async fn process_chunk( ProcessType::Branch, None, deps.event_tx.clone(), - ); + ) + .with_memory_persistence_contract(contract_state.clone()); let user_prompt = prompt_engine.render_system_ingestion_chunk(filename, chunk_number, total_chunks, chunk)?; From d2950f20da4fc673ddd8b7b935f91b851b4c7e0c Mon Sep 17 00:00:00 2001 From: Zach G Date: Sun, 5 Apr 2026 12:21:38 -0400 Subject: [PATCH 2/2] test: add regression tests for ingestion persistence contract wiring (#538) Two tests that verify the root cause and fix: 1. ingestion_hook_without_contract_does_not_record_terminal_outcome Confirms the bug: a hook created without with_memory_persistence_contract() does not record the terminal outcome even when memory_persistence_complete tool call succeeds. 2. ingestion_hook_with_contract_records_terminal_outcome Confirms the fix: a hook created with with_memory_persistence_contract() correctly records the terminal outcome after a successful tool call. --- src/hooks/spacebot.rs | 60 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/hooks/spacebot.rs b/src/hooks/spacebot.rs index 9fbe084d3..fcbe9c7f1 100644 --- a/src/hooks/spacebot.rs +++ b/src/hooks/spacebot.rs @@ -2368,4 +2368,64 @@ mod tests { assert!(matches!(action, HookAction::Continue)); } + + /// Regression test for #538: ingestion hook must wire the memory persistence + /// contract so that `has_terminal_outcome()` returns true after a successful + /// `memory_persistence_complete` tool call. + #[tokio::test] + async fn ingestion_hook_without_contract_does_not_record_terminal_outcome() { + // Simulate the OLD ingestion code path: hook created without + // with_memory_persistence_contract — terminal outcome is never recorded. + let (event_tx, _event_rx) = tokio::sync::broadcast::channel(8); + let contract_state = Arc::new(MemoryPersistenceContractState::default()); + let hook_without_contract = SpacebotHook::new( + std::sync::Arc::::from("agent"), + ProcessId::Branch(uuid::Uuid::new_v4()), + ProcessType::Branch, + None, + event_tx, + ); + // Do NOT call .with_memory_persistence_contract() + + let _ = >::on_tool_result( + &hook_without_contract, + "memory_persistence_complete", + None, + "internal_1", + "{}", + "{\"success\":true,\"outcome\":\"no_memories\",\"saved_memory_ids\":[],\"reason\":\"No durable facts\"}", + ) + .await; + + // Without contract wiring, has_terminal_outcome stays false — this was the #538 bug. + assert!( + !contract_state.has_terminal_outcome(), + "contract state should NOT be set when hook lacks with_memory_persistence_contract" + ); + } + + /// Regression test for #538: ingestion hook WITH the contract wired correctly + /// records the terminal outcome after memory_persistence_complete succeeds. + #[tokio::test] + async fn ingestion_hook_with_contract_records_terminal_outcome() { + // Simulate the FIXED ingestion code path: hook created with + // with_memory_persistence_contract — terminal outcome is recorded. + let (hook, contract_state) = make_memory_persistence_hook(); + + let _ = >::on_tool_result( + &hook, + "memory_persistence_complete", + None, + "internal_1", + "{}", + "{\"success\":true,\"outcome\":\"no_memories\",\"saved_memory_ids\":[],\"reason\":\"No durable facts\"}", + ) + .await; + + // With contract wiring, has_terminal_outcome returns true — the fix for #538. + assert!( + contract_state.has_terminal_outcome(), + "contract state MUST be set when hook has with_memory_persistence_contract" + ); + } }