Skip to content
48 changes: 39 additions & 9 deletions src/openhuman/agent/git_attribution/hook.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::collections::HashMap;
#[cfg(unix)]
use std::ffi::{OsStr, OsString};
#[cfg(unix)]
use std::path::PathBuf;
#[cfg(unix)]
use std::sync::OnceLock;
Expand Down Expand Up @@ -58,23 +60,50 @@ static HOOK_DIR: OnceLock<Option<PathBuf>> = OnceLock::new();
/// The result is intended for an agent-owned child process. It does not change
/// repository configuration or the parent application's environment.
#[cfg(unix)]
pub fn hook_env() -> HashMap<String, String> {
pub fn hook_env() -> HashMap<OsString, OsString> {
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_os("GIT_CONFIG_PARAMETERS").as_deref())
}

#[cfg(unix)]
fn build_hook_env(
dir: &std::path::Path,
inherited_parameters: Option<&OsStr>,
) -> HashMap<OsString, OsString> {
use std::os::unix::ffi::{OsStrExt, OsStringExt};

let mut parameters = inherited_parameters
.map(|value| value.as_bytes().to_vec())
.unwrap_or_default();
if !parameters.is_empty() {
parameters.push(b' ');
}
parameters.extend_from_slice(b"'core.hooksPath'='");
for byte in dir.as_os_str().as_bytes() {
if *byte == b'\'' {
parameters.extend_from_slice(b"'\\''");
} else {
parameters.push(*byte);
}
}
parameters.push(b'\'');

HashMap::from([
("OPENHUMAN_GIT_ATTRIBUTION".into(), TRAILER.into()),
("GIT_CONFIG_COUNT".into(), "1".into()),
("GIT_CONFIG_KEY_0".into(), "core.hooksPath".into()),
(OsString::from("OPENHUMAN_GIT_ATTRIBUTION"), TRAILER.into()),
// 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_VALUE_0".into(),
dir.to_string_lossy().into_owned(),
OsString::from("GIT_CONFIG_PARAMETERS"),
OsString::from_vec(parameters),
),
])
}

#[cfg(not(unix))]
pub fn hook_env() -> HashMap<String, String> {
pub fn hook_env() -> HashMap<std::ffi::OsString, std::ffi::OsString> {
HashMap::new()
}

Expand All @@ -94,6 +123,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<&OsStr>) -> HashMap<OsString, OsString> {
let dir = build_hook_dir().expect("create OpenHuman hook directory");
build_hook_env(&dir, inherited_parameters)
}
50 changes: 45 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(std::ffi::OsStr::new(
"'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,45 @@ 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");
}

#[cfg(unix)]
#[test]
fn hook_env_does_not_drop_inherited_parameters_containing_non_utf8() {
use std::os::unix::ffi::{OsStrExt, OsStringExt};

let inherited_bytes = b"'test.openhuman-inherited'='before-\xff-after' 'test.second'='kept'";
let inherited = std::ffi::OsStr::from_bytes(inherited_bytes);
let hook_env = super::hook::test_hook_env(Some(inherited));
let parameters = hook_env
.get(std::ffi::OsStr::new("GIT_CONFIG_PARAMETERS"))
.unwrap()
.clone()
.into_vec();

assert!(parameters.starts_with(inherited_bytes));
assert_eq!(parameters[inherited_bytes.len()], b' ');
assert!(parameters[inherited_bytes.len() + 1..].starts_with(b"'core.hooksPath'='"));
assert!(parameters.contains(&0xff));

let output = std::process::Command::new("git")
.args(["config", "--get", "test.openhuman-inherited"])
.envs(&hook_env)
.output()
.unwrap();
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
assert_eq!(output.stdout, b"before-\xff-after\n");
}
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));
}
}
2 changes: 1 addition & 1 deletion src/openhuman/flows/tinyflows/caps/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl CodeRunner for OpenHumanCode {

let mut extra_env = std::collections::HashMap::new();
if let Ok(host_path) = std::env::var("PATH") {
extra_env.insert("PATH".to_string(), host_path);
extra_env.insert("PATH".into(), host_path.into());
}

tracing::debug!(
Expand Down
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
Loading
Loading