diff --git a/src/openhuman/agent/prompts/mod_tests.rs b/src/openhuman/agent/prompts/mod_tests.rs index e201b7275..f16483c43 100644 --- a/src/openhuman/agent/prompts/mod_tests.rs +++ b/src/openhuman/agent/prompts/mod_tests.rs @@ -933,6 +933,100 @@ fn render_subagent_system_prompt_skips_profile_md_when_include_profile_false() { let _ = std::fs::remove_dir_all(workspace); } +#[test] +fn render_subagent_system_prompt_frames_memory_md_as_background() { + // GH-4745 regression for the sub-agent path: Inline/File sub-agents inject + // MEMORY.md through `render_subagent_system_prompt`, a separate renderer + // from `UserFilesSection`. It must share the same background-memory frame, + // otherwise a fresh thread reads the bare `### MEMORY.md` block as prior + // in-thread conversation and asserts continuity that isn't there. + let workspace = std::env::temp_dir().join(format!( + "openhuman_subagent_memory_framing_{}", + uuid::Uuid::new_v4() + )); + std::fs::create_dir_all(&workspace).unwrap(); + std::fs::write( + workspace.join("MEMORY.md"), + "# Long-term memory\nReviewed `def f(x)` last week; user prefers terse notes.", + ) + .unwrap(); + + let tools: Vec> = vec![Box::new(TestTool)]; + let rendered = render_subagent_system_prompt( + &workspace, + "test-model", + &[0], + &tools, + &[], + "You are a specialist agent.", + SubagentRenderOptions { + include_identity: false, + include_safety_preamble: false, + include_skills_catalog: false, + include_profile: false, + include_memory_md: true, + }, + ToolCallFormat::PFormat, + &[], + ); + + assert!( + rendered.contains("### MEMORY.md") && rendered.contains("terse notes"), + "MEMORY.md must still be injected in the sub-agent path, got:\n{rendered}" + ); + assert!( + rendered.contains("background — not this conversation"), + "sub-agent MEMORY.md must be framed as durable background memory, got:\n{rendered}" + ); + let frame_at = rendered.find("background — not this conversation").unwrap(); + let heading_at = rendered.find("### MEMORY.md").unwrap(); + assert!( + frame_at < heading_at, + "the guardrail note must precede the MEMORY.md block, got:\n{rendered}" + ); + + let _ = std::fs::remove_dir_all(workspace); +} + +#[test] +fn render_subagent_system_prompt_omits_memory_framing_when_no_memory_content() { + // Companion to the framing test: with `include_memory_md = true` but no + // MEMORY.md on disk (a genuinely fresh workspace) the dangling frame must + // NOT appear — emitting a "background memory" note pointing at nothing + // would itself imply phantom history. + let workspace = std::env::temp_dir().join(format!( + "openhuman_subagent_memory_noframe_{}", + uuid::Uuid::new_v4() + )); + std::fs::create_dir_all(&workspace).unwrap(); + + let tools: Vec> = vec![Box::new(TestTool)]; + let rendered = render_subagent_system_prompt( + &workspace, + "test-model", + &[0], + &tools, + &[], + "You are a specialist agent.", + SubagentRenderOptions { + include_identity: false, + include_safety_preamble: false, + include_skills_catalog: false, + include_profile: false, + include_memory_md: true, + }, + ToolCallFormat::PFormat, + &[], + ); + + assert!( + !rendered.contains("background — not this conversation"), + "no MEMORY.md content → no dangling framing note in sub-agent path, got:\n{rendered}" + ); + + let _ = std::fs::remove_dir_all(workspace); +} + #[test] fn render_subagent_system_prompt_injects_profile_md_when_identity_included() { // When identity is on, PROFILE.md must still be injected alongside @@ -1407,6 +1501,101 @@ fn for_subagent_builder_injects_user_files_even_when_identity_omitted() { let _ = std::fs::remove_dir_all(workspace); } +/// Shared `PromptContext` for the MEMORY.md-framing tests below. Both +/// exercise `UserFilesSection` with memory injection enabled and differ +/// only in workspace contents, so they build an identical 19-field +/// context — factor it out so the two can't drift when `PromptContext` +/// gains fields. Borrows the caller's `workspace` and pre-built +/// `prompt_tools` so the returned context outlives neither. +fn memory_framing_ctx<'a>( + workspace: &'a std::path::Path, + prompt_tools: &'a [PromptTool<'a>], +) -> PromptContext<'a> { + PromptContext { + workspace_dir: workspace, + model_name: "test-model", + agent_id: "", + tools: prompt_tools, + workflows: &[], + dispatcher_instructions: "", + learned: LearnedContextData::default(), + visible_tool_names: &NO_FILTER, + tool_call_format: ToolCallFormat::PFormat, + connected_integrations: &[], + connected_identities_md: String::new(), + include_profile: false, + include_memory_md: true, + curated_snapshot: None, + user_identity: None, + personality_soul_md: None, + personality_memory_md: None, + personality_roster: vec![], + } +} + +#[test] +fn memory_md_injection_is_framed_as_background_not_prior_chat() { + // GH-4745 regression: MEMORY.md is durable cross-session memory. Without + // a frame, a relevant curated observation reads to the model as prior + // *in-thread* conversation, so on a brand-new thread it opens with + // "already covered this in a previous chat" and shortcuts its answer. + // Pin that the rendered prompt frames the block as background memory and + // that the guardrail precedes the injected `### MEMORY.md` heading. + // + // `tempfile::tempdir()` cleans up via `Drop` even when an assertion + // below panics — a bare `remove_dir_all` at the tail would leak the + // dir exactly on the failing run we most want to inspect. + let workspace = tempfile::tempdir().unwrap(); + std::fs::write( + workspace.path().join("MEMORY.md"), + "# Long-term memory\nReviewed `def f(x)` last week; user prefers terse notes.", + ) + .unwrap(); + + let tools: Vec> = vec![]; + let prompt_tools = PromptTool::from_tools(&tools); + let ctx = memory_framing_ctx(workspace.path(), &prompt_tools); + + let rendered = UserFilesSection.build(&ctx).unwrap(); + + assert!( + rendered.contains("### MEMORY.md") && rendered.contains("terse notes"), + "MEMORY.md must still be injected, got:\n{rendered}" + ); + assert!( + rendered.contains("background — not this conversation"), + "MEMORY.md must be framed as durable background memory, got:\n{rendered}" + ); + assert!( + rendered.contains("already covered this in a previous chat"), + "framing must explicitly forbid asserting prior-chat continuity, got:\n{rendered}" + ); + let frame_at = rendered.find("background — not this conversation").unwrap(); + let heading_at = rendered.find("### MEMORY.md").unwrap(); + assert!( + frame_at < heading_at, + "the guardrail note must precede the MEMORY.md block, got:\n{rendered}" + ); +} + +#[test] +fn memory_md_framing_absent_when_no_memory_content() { + // The frame must never appear on its own: when MEMORY.md is missing/empty + // (a genuinely fresh workspace) there is nothing to scope, so emitting a + // dangling "background memory" note would itself imply phantom history. + let workspace = tempfile::tempdir().unwrap(); + + let tools: Vec> = vec![]; + let prompt_tools = PromptTool::from_tools(&tools); + let ctx = memory_framing_ctx(workspace.path(), &prompt_tools); + + let rendered = UserFilesSection.build(&ctx).unwrap(); + assert!( + !rendered.contains("background — not this conversation"), + "no MEMORY.md content → no dangling framing note, got:\n{rendered}" + ); +} + #[test] fn sync_workspace_file_updates_hash_and_inject_workspace_file_truncates() { let workspace = std::env::temp_dir().join(format!( diff --git a/src/openhuman/agent/prompts/render_helpers.rs b/src/openhuman/agent/prompts/render_helpers.rs index 993198c7c..ce9a18f80 100644 --- a/src/openhuman/agent/prompts/render_helpers.rs +++ b/src/openhuman/agent/prompts/render_helpers.rs @@ -301,7 +301,19 @@ pub fn render_subagent_system_prompt_with_format( inject_workspace_file_capped(&mut out, workspace_dir, "PROFILE.md", USER_FILE_MAX_CHARS); } if options.include_memory_md { - inject_workspace_file_capped(&mut out, workspace_dir, "MEMORY.md", USER_FILE_MAX_CHARS); + // Frame MEMORY.md as durable, cross-session background memory — + // byte-identical to `UserFilesSection::build` (GH-4745). Without the + // frame an Inline/File sub-agent on a brand-new thread reads the bare + // `### MEMORY.md` block as prior in-thread conversation and asserts + // continuity that isn't there. Buffer first so the note is emitted + // only when the file actually carries content — a dangling frame + // pointing at nothing would itself imply phantom history. + let mut mem = String::new(); + inject_workspace_file_capped(&mut mem, workspace_dir, "MEMORY.md", USER_FILE_MAX_CHARS); + if !mem.trim().is_empty() { + out.push_str(MEMORY_MD_FRAMING); + out.push_str(&mem); + } } // 2. Filtered tool catalogue. Indices are taken in ascending order diff --git a/src/openhuman/agent/prompts/sections.rs b/src/openhuman/agent/prompts/sections.rs index bf64b72e7..94f0133a1 100644 --- a/src/openhuman/agent/prompts/sections.rs +++ b/src/openhuman/agent/prompts/sections.rs @@ -185,6 +185,31 @@ pub struct UserIdentitySection; /// [`AgentDefinition::omit_profile`] / `omit_memory_md`. pub struct UserFilesSection; +/// Framing preamble emitted immediately before the injected `MEMORY.md` +/// (and, in the snapshot path, `USER.md`) block. +/// +/// `MEMORY.md` is durable, cross-session memory — archivist-curated facts +/// carried over from *past* sessions and previously ingested history. Without +/// a frame, a relevant curated observation reads to the model as something +/// already said *in this thread*, so on a brand-new thread it asserts +/// continuity that isn't there ("already covered this in a previous chat") +/// and shortcuts its answer (GH-4745). This note scopes the block as +/// background knowledge and forbids claiming in-thread continuity. +/// +/// `pub(crate)` so the sub-agent renderer +/// ([`super::render_helpers::render_subagent_system_prompt_with_format`]) +/// can share the exact same frame — Inline/File sub-agents inject +/// `MEMORY.md` through their own path and must not drift from this note. +pub(crate) const MEMORY_MD_FRAMING: &str = + "### Long-term memory (background — not this conversation)\n\n\ +The block below is your durable, cross-session memory: facts and observations \ +carried over from *past* sessions and previously ingested history. Treat it as \ +background knowledge only — it is **not** part of the current thread. Do not \ +treat it as messages already exchanged here, never claim you \"already covered \ +this in a previous chat\" or otherwise assert continuity that isn't present in \ +the visible messages, and answer each new thread in full even when related \ +prior context appears here.\n\n"; + /// Renders the personality roster for the master agent's system prompt. /// /// When [`PromptContext::personality_roster`] is non-empty, emits an @@ -321,23 +346,33 @@ impl PromptSection for UserFilesSection { // Personality-specific MEMORY.md takes highest priority, then // the session-frozen curated-memory snapshot, then the // workspace file (pure prompt-unit tests and older call sites). + // + // Render into a scratch buffer first so the `MEMORY_MD_FRAMING` + // note is only emitted when the block actually carries content — + // the inject helpers silently skip empty/missing files, and a + // dangling frame pointing at nothing would be worse than none. + let mut mem = String::new(); if let Some(ref memory_md) = ctx.personality_memory_md { tracing::debug!( "[user_files] personality MEMORY.md override active ({} chars)", memory_md.len() ); - inject_inline_content(&mut out, "MEMORY.md", memory_md, USER_FILE_MAX_CHARS); + inject_inline_content(&mut mem, "MEMORY.md", memory_md, USER_FILE_MAX_CHARS); } else if let Some(snap) = &ctx.curated_snapshot { - inject_snapshot_content(&mut out, "MEMORY.md", &snap.memory, USER_FILE_MAX_CHARS); - inject_snapshot_content(&mut out, "USER.md", &snap.user, USER_FILE_MAX_CHARS); + inject_snapshot_content(&mut mem, "MEMORY.md", &snap.memory, USER_FILE_MAX_CHARS); + inject_snapshot_content(&mut mem, "USER.md", &snap.user, USER_FILE_MAX_CHARS); } else { inject_workspace_file_capped( - &mut out, + &mut mem, ctx.workspace_dir, "MEMORY.md", USER_FILE_MAX_CHARS, ); } + if !mem.trim().is_empty() { + out.push_str(MEMORY_MD_FRAMING); + out.push_str(&mem); + } } Ok(out) }