Skip to content
32 changes: 24 additions & 8 deletions src/openhuman/agent/git_attribution/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,29 @@ pub fn hook_env() -> HashMap<String, String> {
let Some(dir) = HOOK_DIR.get_or_init(|| build_hook_dir().ok()).as_ref() else {
return HashMap::new();
};
build_hook_env(dir, std::env::var("GIT_CONFIG_PARAMETERS").ok().as_deref())
Comment thread
senamakel marked this conversation as resolved.
Outdated
}

#[cfg(unix)]
fn build_hook_env(
dir: &std::path::Path,
inherited_parameters: Option<&str>,
) -> HashMap<String, String> {
let mut parameters = inherited_parameters.unwrap_or_default().trim().to_owned();
if !parameters.is_empty() {
parameters.push(' ');
}
let hook_path = dir.to_string_lossy().replace('\'', "'\\''");
parameters.push_str("'core.hooksPath'='");
parameters.push_str(&hook_path);
parameters.push('\'');

HashMap::from([
("OPENHUMAN_GIT_ATTRIBUTION".into(), TRAILER.into()),
("GIT_CONFIG_COUNT".into(), "1".into()),
("GIT_CONFIG_KEY_0".into(), "core.hooksPath".into()),
(
"GIT_CONFIG_VALUE_0".into(),
dir.to_string_lossy().into_owned(),
),
// Parameters outrank GIT_CONFIG_COUNT. Preserve settings inherited
// from the parent harness, then append our hook so it wins if that
// harness also selected a hook path.
("GIT_CONFIG_PARAMETERS".into(), parameters),
])
}

Expand All @@ -94,6 +109,7 @@ fn build_hook_dir() -> std::io::Result<PathBuf> {
}

#[cfg(all(unix, test))]
pub(super) fn test_hook_dir() -> PathBuf {
build_hook_dir().expect("create OpenHuman hook directory")
pub(super) fn test_hook_env(inherited_parameters: Option<&str>) -> HashMap<String, String> {
let dir = build_hook_dir().expect("create OpenHuman hook directory");
build_hook_env(&dir, inherited_parameters)
}
18 changes: 13 additions & 5 deletions src/openhuman/agent/git_attribution/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,13 @@ fn hook_adds_openhuman_trailer_without_disabling_repository_hook() {
std::fs::write(repo.join("a"), "a").unwrap();
git(&["add", "a"]);

let hook_dir = super::hook::test_hook_dir();
let hook_env = super::hook::test_hook_env(Some(
"'test.openhuman-inherited'='kept' 'core.hooksPath'='/definitely-not-the-openhuman-hook'",
));
let output = Command::new("git")
.args(["commit", "-q", "-m", "subject"])
.current_dir(&repo)
.env("OPENHUMAN_GIT_ATTRIBUTION", super::hook::TRAILER)
.env("GIT_CONFIG_COUNT", "1")
.env("GIT_CONFIG_KEY_0", "core.hooksPath")
.env("GIT_CONFIG_VALUE_0", &hook_dir)
.envs(&hook_env)
.output()
.unwrap();
assert!(
Expand All @@ -55,4 +54,13 @@ fn hook_adds_openhuman_trailer_without_disabling_repository_hook() {
let message = String::from_utf8(output.stdout).unwrap();
assert!(message.contains("repo-hook-ran"), "{message:?}");
assert!(message.contains(super::hook::TRAILER), "{message:?}");

let inherited = Command::new("git")
.args(["config", "--get", "test.openhuman-inherited"])
.current_dir(&repo)
.envs(&hook_env)
.output()
.unwrap();
assert!(inherited.status.success());
assert_eq!(String::from_utf8(inherited.stdout).unwrap().trim(), "kept");
}
16 changes: 5 additions & 11 deletions src/openhuman/agent/harness/archivist/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,19 +272,13 @@ impl ArchivistHook {
// back to FTS5 in test paths or when config isn't wired.
let entries = self.read_session_entries(conn, session_id);

// Filter entries that fall within the segment's time window.
// Use <= for end_timestamp (entries at the boundary are part of this
// segment). The boundary-triggering turn has a timestamp AFTER
// end_timestamp, so it won't be included.
// Filter entries by their stable per-session sequence or episodic row
// id. The md store rounds timestamps to milliseconds, which can move a
// fast turn just before its segment's higher-precision start time.
let segment_entries: Vec<&EpisodicEntry> = entries
.iter()
.filter(|e| {
e.timestamp >= segment.start_timestamp
&& segment
.end_timestamp
.map(|end| e.timestamp <= end)
.unwrap_or(true)
})
.filter(|record| record.is_in_segment(segment))
.map(|record| &record.entry)
.collect();

if segment_entries.is_empty() {
Expand Down
164 changes: 144 additions & 20 deletions src/openhuman/agent/harness/archivist/recap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,74 @@

use super::types::ArchivistHook;
use crate::openhuman::memory::store::fts5::{self, EpisodicEntry};
use crate::openhuman::memory::store::segments;
use crate::openhuman::memory::store::segments::{self, ConversationSegment};
use crate::openhuman::memory::store::trees::types::TreeKind;
use crate::openhuman::memory::tree::summarise::{summarise, SummaryContext, SummaryInput};
use parking_lot::Mutex;
use rusqlite::Connection;
use std::sync::Arc;

/// An episodic entry paired with the stable identity exposed by its backing
/// store. The md archivist uses a per-session sequence while the legacy FTS5
/// store uses a row id.
pub(super) struct SessionEntry {
pub(super) entry: EpisodicEntry,
sequence: Option<u32>,
}

impl SessionEntry {
/// Whether this entry belongs to a closed segment.
///
/// Segment endpoints identify user turns. Each user turn is immediately
/// followed by its assistant entry, so the inclusive span ends one entry
/// after the recorded end user turn.
pub(super) fn is_in_segment(&self, segment: &ConversationSegment) -> bool {
if let (Some(sequence), Some(start)) = (self.sequence, segment.start_seq) {
let end = segment.end_seq.unwrap_or(start).saturating_add(1);
return sequence >= start && sequence <= end;
}

if let Some(id) = self.entry.id {
let start = segment.start_episodic_id;
let end = segment.end_episodic_id.unwrap_or(start).saturating_add(1);
return id >= start && id <= end;
}

self.entry.timestamp >= segment.start_timestamp
&& segment
.end_timestamp
.map(|end| self.entry.timestamp <= end)
.unwrap_or(true)
}

/// Whether this entry belongs to the open segment or a later turn.
pub(super) fn is_at_or_after_segment_start(&self, segment: &ConversationSegment) -> bool {
if let (Some(sequence), Some(start)) = (self.sequence, segment.start_seq) {
return sequence >= start;
}

if let Some(id) = self.entry.id {
return id >= segment.start_episodic_id;
}

self.entry.timestamp >= segment.start_timestamp
}
}

impl ArchivistHook {
/// Read every entry recorded for `session_id`, preferring the
/// crate-owned md-backed archivist store when `self.config` is set and
/// falling back to the legacy FTS5 episodic table otherwise.
///
/// Returns `EpisodicEntry` so the existing call sites (segment
/// gathering, recap rendering, tree push) keep their shape unchanged
/// during the FTS5 retirement migration.
/// Each entry retains the stable sequence or row identity needed for
/// segment selection. Timestamps are only a fallback for legacy records:
/// the md store records epoch milliseconds and therefore cannot preserve
/// the sub-millisecond timestamps used when a segment is opened.
pub(super) fn read_session_entries(
&self,
conn: &Arc<Mutex<Connection>>,
session_id: &str,
) -> Vec<EpisodicEntry> {
) -> Vec<SessionEntry> {
if let Some(cfg) = self.config.as_ref() {
let engine_config = crate::openhuman::memory::tinycortex::memory_config_from(
cfg,
Expand All @@ -32,17 +80,20 @@ impl ArchivistHook {
Ok(turns) => {
return turns
.into_iter()
.map(|t| EpisodicEntry {
id: None,
session_id: t.session_id,
// ArchivedTurn stores epoch-ms; EpisodicEntry
// takes epoch-seconds as f64.
timestamp: (t.timestamp_ms as f64) / 1000.0,
role: t.role,
content: t.content,
lesson: t.lesson,
tool_calls_json: t.tool_calls_json,
cost_microdollars: t.cost_microdollars,
.map(|t| SessionEntry {
sequence: Some(t.seq),
entry: EpisodicEntry {
id: None,
session_id: t.session_id,
// ArchivedTurn stores epoch-ms; EpisodicEntry
// takes epoch-seconds as f64.
timestamp: (t.timestamp_ms as f64) / 1000.0,
role: t.role,
content: t.content,
lesson: t.lesson,
tool_calls_json: t.tool_calls_json,
cost_microdollars: t.cost_microdollars,
},
})
.collect();
}
Expand All @@ -53,7 +104,14 @@ impl ArchivistHook {
}
}
}
fts5::episodic_session_entries(conn, session_id).unwrap_or_default()
fts5::episodic_session_entries(conn, session_id)
.unwrap_or_default()
.into_iter()
.map(|entry| SessionEntry {
entry,
sequence: None,
})
.collect()
}

/// Shared summarize helper — the **single LLM summarizer** used by both
Expand Down Expand Up @@ -246,11 +304,12 @@ impl ArchivistHook {
// Gather the episodic entries for this session so far.
let all_entries = self.read_session_entries(conn, session_id);

// Keep only entries within the open segment's time window (start →
// now, inclusive). An open segment has `end_timestamp = None`.
// Keep only entries belonging to the open segment. Prefer stable
// sequence/row identity because the md store rounds timestamps to ms.
let segment_entries: Vec<&EpisodicEntry> = all_entries
.iter()
.filter(|e| e.timestamp >= open_segment.start_timestamp)
.filter(|record| record.is_at_or_after_segment_start(&open_segment))
.map(|record| &record.entry)
.collect();

if segment_entries.is_empty() {
Expand Down Expand Up @@ -305,3 +364,68 @@ impl ArchivistHook {
Some(recap)
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::openhuman::memory::store::segments::SegmentStatus;

fn segment() -> ConversationSegment {
ConversationSegment {
segment_id: "segment".into(),
session_id: "session".into(),
namespace: "global".into(),
start_episodic_id: 20,
end_episodic_id: Some(24),
start_timestamp: 100.000_9,
end_timestamp: Some(100.001_1),
turn_count: 3,
summary: None,
embedding: None,
topic_keywords: None,
status: SegmentStatus::Closed,
created_at: 100.0,
updated_at: 100.0,
start_seq: Some(10),
end_seq: Some(14),
}
}

fn entry(sequence: Option<u32>, id: Option<i64>, timestamp: f64) -> SessionEntry {
SessionEntry {
sequence,
entry: EpisodicEntry {
id,
session_id: "session".into(),
timestamp,
role: "user".into(),
content: "content".into(),
lesson: None,
tool_calls_json: None,
cost_microdollars: 0,
},
}
}

#[test]
fn segment_membership_uses_sequence_instead_of_rounded_timestamp() {
let segment = segment();

assert!(!entry(Some(9), None, 100.001).is_in_segment(&segment));
assert!(entry(Some(10), None, 100.000).is_in_segment(&segment));
assert!(entry(Some(15), None, 101.0).is_in_segment(&segment));
assert!(!entry(Some(16), None, 100.001).is_in_segment(&segment));
}

#[test]
fn segment_membership_falls_back_to_episodic_id() {
let mut segment = segment();
segment.start_seq = None;
segment.end_seq = None;

assert!(!entry(None, Some(19), 100.001).is_in_segment(&segment));
assert!(entry(None, Some(20), 100.000).is_in_segment(&segment));
assert!(entry(None, Some(25), 101.0).is_in_segment(&segment));
assert!(!entry(None, Some(26), 100.001).is_in_segment(&segment));
}
}
4 changes: 2 additions & 2 deletions src/openhuman/memory/api/provider/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ fn join(families: &[Capability]) -> String {
/// # Examples
///
/// ```
/// # use crate::openhuman::memory::api::null::NullMemoryProvider;
/// # use crate::openhuman::memory::api::provider::audit_provider;
/// # use openhuman_core::openhuman::memory::api::null::NullMemoryProvider;
/// # use openhuman_core::openhuman::memory::api::provider::audit_provider;
/// // The reference null driver is self-consistent.
/// assert!(audit_provider(&NullMemoryProvider::new()).is_ok());
/// ```
Expand Down
4 changes: 2 additions & 2 deletions src/openhuman/memory/api/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl MemoryTaint {
/// # Examples
///
/// ```
/// use crate::openhuman::memory::api::types::MemoryTaint;
/// use openhuman_core::openhuman::memory::api::types::MemoryTaint;
///
/// assert_eq!(MemoryTaint::Internal.as_db_str(), "internal");
/// assert_eq!(MemoryTaint::ExternalSync.as_db_str(), "external_sync");
Expand All @@ -103,7 +103,7 @@ impl MemoryTaint {
/// # Examples
///
/// ```
/// use crate::openhuman::memory::api::types::MemoryTaint;
/// use openhuman_core::openhuman::memory::api::types::MemoryTaint;
///
/// assert_eq!(MemoryTaint::from_db_str("internal"), MemoryTaint::Internal);
/// assert_eq!(MemoryTaint::from_db_str("external_sync"), MemoryTaint::ExternalSync);
Expand Down
2 changes: 1 addition & 1 deletion src/openhuman/memory/api/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub const CONTRACT_VERSION: (u16, u16) = (2, 0);
/// # Examples
///
/// ```
/// use crate::openhuman::memory::api::{is_compatible, CONTRACT_VERSION};
/// use openhuman_core::openhuman::memory::api::{is_compatible, CONTRACT_VERSION};
///
/// // The version this build speaks is always compatible with itself.
/// assert!(is_compatible(CONTRACT_VERSION));
Expand Down
2 changes: 1 addition & 1 deletion src/openhuman/memory/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ fn module_provider(_workspace_dir: &Path) -> (Arc<dyn MemoryProvider>, DriverCla
// workspace as the process-global test client so concurrent tests cannot
// win module initialization with an unrelated tempdir and split guarded
// writes from legacy read-back calls.
let workspace_dir = crate::openhuman::memory::ops::ensure_shared_memory_client();
let workspace_dir = crate::openhuman::memory::ops::shared_memory_test_workspace();
let mut config = crate::openhuman::config::Config::default();
config.workspace_dir = workspace_dir.clone();
config.modules.install_dir = Some(workspace_dir.join("modules").to_string_lossy().into_owned());
Expand Down
2 changes: 1 addition & 1 deletion src/openhuman/memory/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub(crate) static GLOBAL_MEMORY_TEST_LOCK: tokio::sync::Mutex<()> =
#[cfg(test)]
mod test_support;
#[cfg(test)]
pub(crate) use test_support::ensure_shared_memory_client;
pub(crate) use test_support::{ensure_shared_memory_client, shared_memory_test_workspace};

#[cfg(test)]
#[path = "../ops_tests.rs"]
Expand Down
Loading
Loading