From c885be83da73517b912b0f8865c3a70b5f3c3dc5 Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Mon, 11 May 2026 15:42:48 +0200 Subject: [PATCH 1/2] feat(reborn): wire skill context into loop prompt --- Cargo.lock | 1 + crates/ironclaw_loop_support/Cargo.toml | 1 + crates/ironclaw_loop_support/src/lib.rs | 80 ++- .../src/skill_context.rs | 255 +++++++++ .../tests/thread_loop_support_contract.rs | 489 +++++++++++++++++- .../ironclaw_reborn/src/loop_driver_host.rs | 40 +- .../ironclaw_turns/src/run_profile/prompt.rs | 91 +++- .../tests/agent_loop_host_contract.rs | 38 +- 8 files changed, 963 insertions(+), 32 deletions(-) create mode 100644 crates/ironclaw_loop_support/src/skill_context.rs diff --git a/Cargo.lock b/Cargo.lock index 16969a2fb35..83b3d4b9b75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4390,6 +4390,7 @@ version = "0.1.0" dependencies = [ "async-trait", "ironclaw_host_api", + "ironclaw_skills", "ironclaw_threads", "ironclaw_turns", "serde", diff --git a/crates/ironclaw_loop_support/Cargo.toml b/crates/ironclaw_loop_support/Cargo.toml index 3b3b4cea684..62df1af5d86 100644 --- a/crates/ironclaw_loop_support/Cargo.toml +++ b/crates/ironclaw_loop_support/Cargo.toml @@ -14,6 +14,7 @@ publish = false async-trait = "0.1" tracing = "0.1" tokio = { version = "1", features = ["sync"] } +ironclaw_skills = { path = "../ironclaw_skills", version = "0.3.0", default-features = false } ironclaw_threads = { path = "../ironclaw_threads", version = "0.1.0" } ironclaw_turns = { path = "../ironclaw_turns", version = "0.1.0" } serde = { version = "1", features = ["derive"] } diff --git a/crates/ironclaw_loop_support/src/lib.rs b/crates/ironclaw_loop_support/src/lib.rs index 305f73667c8..64145fb44bb 100644 --- a/crates/ironclaw_loop_support/src/lib.rs +++ b/crates/ironclaw_loop_support/src/lib.rs @@ -9,6 +9,13 @@ use std::{ sync::Arc, }; +mod skill_context; + +pub use skill_context::{ + HostSkillContextBuildError, HostSkillContextCandidate, HostSkillContextSource, + build_skill_run_snapshot, +}; + use tokio::sync::Mutex; use async_trait::async_trait; @@ -46,6 +53,7 @@ where thread_scope: ThreadScope, run_context: LoopRunContext, max_messages: usize, + skill_context_source: Option>, } impl ThreadBackedLoopContextPort @@ -63,8 +71,14 @@ where thread_scope, run_context, max_messages, + skill_context_source: None, } } + + pub fn with_skill_context_source(mut self, source: Arc) -> Self { + self.skill_context_source = Some(source); + self + } } impl LoopRunInfoPort for ThreadBackedLoopContextPort @@ -98,13 +112,20 @@ where .await .map_err(context_read_error)?; + let instruction_snippets = match self.skill_context_source.as_deref() { + Some(source) => { + skill_context::build_skill_instruction_snippets(source, &self.run_context).await? + } + None => Vec::new(), + }; + Ok(LoopContextBundle { messages: context .messages .into_iter() .filter_map(context_message_to_loop_message) .collect(), - instruction_snippets: Vec::new(), + instruction_snippets, memory_snippets: Vec::new(), }) } @@ -426,6 +447,7 @@ where gateway: Arc, max_messages: usize, milestone_sink: Option>, + skill_context_source: Option>, } impl ThreadBackedLoopModelPort @@ -447,6 +469,7 @@ where gateway, max_messages, milestone_sink: None, + skill_context_source: None, } } @@ -465,8 +488,14 @@ where gateway, max_messages, milestone_sink: Some(milestone_sink), + skill_context_source: None, } } + + pub fn with_skill_context_source(mut self, source: Arc) -> Self { + self.skill_context_source = Some(source); + self + } } impl LoopRunInfoPort for ThreadBackedLoopModelPort @@ -590,6 +619,14 @@ where let needs_history_lookup = requested_messages .iter() .any(|message| !messages_by_ref.contains_key(message.content_ref.as_str())); + let snippet_messages_by_ref = if requested_messages + .iter() + .any(|message| skill_context::is_snippet_model_message_ref(&message.content_ref)) + { + self.instruction_snippet_messages_by_ref().await? + } else { + HashMap::new() + }; if needs_history_lookup { let history = self .thread_service @@ -604,6 +641,19 @@ where } let mut resolved = Vec::with_capacity(requested_messages.len()); for message in requested_messages { + let requested_role = HostManagedModelMessageRole::from_loop_role(&message.role)?; + if let Some(snippet_message) = snippet_messages_by_ref.get(message.content_ref.as_str()) + { + if requested_role != snippet_message.role { + return Err(AgentLoopHostError::new( + AgentLoopHostErrorKind::InvalidInvocation, + "model message role does not match skill context snippet", + )); + } + resolved.push(snippet_message.clone()); + continue; + } + let context_message = messages_by_ref .get(message.content_ref.as_str()) .ok_or_else(|| { @@ -612,7 +662,6 @@ where "model message reference is unavailable", ) })?; - let requested_role = HostManagedModelMessageRole::from_loop_role(&message.role)?; let durable_role = model_role_for_kind(context_message.kind); if requested_role != durable_role { return Err(AgentLoopHostError::new( @@ -628,6 +677,33 @@ where } Ok(resolved) } + + async fn instruction_snippet_messages_by_ref( + &self, + ) -> Result, AgentLoopHostError> { + let Some(source) = self.skill_context_source.as_deref() else { + return Ok(HashMap::new()); + }; + let snippets = + skill_context::build_skill_instruction_snippets(source, &self.run_context).await?; + let mut messages = HashMap::with_capacity(snippets.len()); + for (ordinal, snippet) in snippets.into_iter().enumerate() { + let content_ref = skill_context::snippet_model_message_ref( + &snippet.snippet_ref, + &snippet.safe_summary, + ordinal, + )?; + messages.insert( + content_ref.as_str().to_string(), + HostManagedModelMessage { + role: HostManagedModelMessageRole::System, + content: snippet.safe_summary, + content_ref, + }, + ); + } + Ok(messages) + } } /// Host-managed text-only model gateway. Implementations own provider selection, diff --git a/crates/ironclaw_loop_support/src/skill_context.rs b/crates/ironclaw_loop_support/src/skill_context.rs new file mode 100644 index 00000000000..6e18854318b --- /dev/null +++ b/crates/ironclaw_loop_support/src/skill_context.rs @@ -0,0 +1,255 @@ +use async_trait::async_trait; +use ironclaw_skills::{ParsedSkill, SkillTrust, parse_skill_md}; +use ironclaw_turns::{ + LoopMessageRef, + run_profile::{ + AgentLoopHostError, AgentLoopHostErrorKind, InstalledSkillSnapshot, LoopContextSnippet, + LoopRunContext, SkillContextError, SkillContextService, SkillContextSource, + SkillRunSnapshot, SkillTrustLevel, SkillVisibility, + }, +}; +use thiserror::Error; + +/// Host-owned source for production skill context candidates. +/// +/// Implementations own storage/policy lookups. This trait intentionally returns +/// host-approved trust/visibility decisions plus raw SKILL.md content only for +/// visible candidates so `ironclaw_turns` remains a snapshot-only loop boundary. +#[async_trait] +pub trait HostSkillContextSource: Send + Sync { + async fn load_skill_context_candidates( + &self, + run_context: &LoopRunContext, + ) -> Result, HostSkillContextBuildError>; +} + +/// One host-approved skill candidate before parsing and snapshot conversion. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HostSkillContextCandidate { + /// Raw SKILL.md content from the production skill source. + /// + /// Hidden/denied candidates may omit raw content; they are policy-filtered + /// before parsing so invisible skills cannot fail prompt construction via + /// malformed prompt files. + pub skill_md: Option, + /// Host-approved trust state. `None` fails the build closed. + pub trust: Option, + /// Host-approved model visibility. `None` fails the build closed. + pub visibility: Option, + /// Optional deterministic ordering key. Defaults to parsed skill name. + pub ordering_key: Option, +} + +impl HostSkillContextCandidate { + pub fn new( + skill_md: impl Into, + trust: Option, + visibility: Option, + ) -> Self { + Self { + skill_md: Some(skill_md.into()), + trust, + visibility, + ordering_key: None, + } + } + + pub fn unavailable(trust: Option, visibility: Option) -> Self { + Self { + skill_md: None, + trust, + visibility, + ordering_key: None, + } + } + + pub fn with_ordering_key(mut self, ordering_key: impl Into) -> Self { + self.ordering_key = Some(ordering_key.into()); + self + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum HostSkillContextBuildError { + #[error("skill context source unavailable")] + SourceUnavailable, + #[error("skill context parse failed")] + ParseFailed, + #[error("skill context trust data missing")] + TrustDataMissing, + #[error("skill context visibility data missing")] + VisibilityDataMissing, + #[error("skill context budget exceeded")] + ContextBudgetExceeded, + #[error("skill context internal error")] + Internal, +} + +impl HostSkillContextBuildError { + pub fn into_host_error(self) -> AgentLoopHostError { + let kind = match self { + Self::SourceUnavailable => AgentLoopHostErrorKind::Unavailable, + Self::ParseFailed => AgentLoopHostErrorKind::InvalidInvocation, + Self::TrustDataMissing | Self::VisibilityDataMissing => { + AgentLoopHostErrorKind::PolicyDenied + } + Self::ContextBudgetExceeded => AgentLoopHostErrorKind::BudgetExceeded, + Self::Internal => AgentLoopHostErrorKind::Internal, + }; + AgentLoopHostError::new(kind, self.to_string()) + } +} + +pub async fn build_skill_instruction_snippets( + source: &(dyn HostSkillContextSource + Send + Sync), + run_context: &LoopRunContext, +) -> Result, AgentLoopHostError> { + let candidates = source + .load_skill_context_candidates(run_context) + .await + .map_err(HostSkillContextBuildError::into_host_error)?; + let snapshot = build_skill_run_snapshot(candidates) + .map_err(HostSkillContextBuildError::into_host_error)?; + let service = SkillContextService::new(snapshot.clone()); + let snippets = service + .skill_snippets(&snapshot) + .await + .map_err(skill_context_error_to_host_error)?; + Ok(snippets + .into_iter() + .map(|snippet| snippet.into_loop_snippet()) + .collect()) +} + +pub fn build_skill_run_snapshot( + candidates: Vec, +) -> Result { + if candidates.is_empty() { + return Ok(SkillRunSnapshot::empty()); + } + + let mut entries = Vec::with_capacity(candidates.len()); + for candidate in candidates { + let trust = candidate + .trust + .ok_or(HostSkillContextBuildError::TrustDataMissing)?; + let visibility = candidate + .visibility + .ok_or(HostSkillContextBuildError::VisibilityDataMissing)?; + if visibility != SkillVisibility::Visible { + continue; + } + let skill_md = candidate + .skill_md + .ok_or(HostSkillContextBuildError::SourceUnavailable)?; + let parsed = + parse_skill_md(&skill_md).map_err(|_| HostSkillContextBuildError::ParseFailed)?; + entries.push(parsed_skill_to_snapshot_entry( + parsed, + trust, + visibility, + candidate.ordering_key, + )); + } + + Ok(SkillRunSnapshot::from_entries(entries)) +} + +fn parsed_skill_to_snapshot_entry( + parsed: ParsedSkill, + trust: SkillTrust, + visibility: SkillVisibility, + ordering_key: Option, +) -> InstalledSkillSnapshot { + let name = parsed.manifest.name; + InstalledSkillSnapshot { + ordering_key: ordering_key.unwrap_or_else(|| name.clone()), + name, + trust: skill_trust_level(trust), + visibility, + prompt_content: Some(parsed.prompt_content), + safe_description: parsed.manifest.description, + } +} + +fn skill_trust_level(trust: SkillTrust) -> SkillTrustLevel { + match trust { + SkillTrust::Installed => SkillTrustLevel::Installed, + SkillTrust::Trusted => SkillTrustLevel::Trusted, + } +} + +fn skill_context_error_to_host_error(error: SkillContextError) -> AgentLoopHostError { + let build_error = match error { + SkillContextError::TrustDataMissing => HostSkillContextBuildError::TrustDataMissing, + SkillContextError::VisibilityDataMissing => { + HostSkillContextBuildError::VisibilityDataMissing + } + SkillContextError::ContextBudgetExceeded => { + HostSkillContextBuildError::ContextBudgetExceeded + } + SkillContextError::InvalidSnapshotVersion | SkillContextError::Internal => { + HostSkillContextBuildError::Internal + } + }; + build_error.into_host_error() +} + +pub(crate) fn snippet_model_message_ref( + snippet_ref: &str, + safe_summary: &str, + ordinal: usize, +) -> Result { + let slug = sanitize_ref_suffix(snippet_ref); + let hash = stable_snippet_ref_hash(snippet_ref, safe_summary, ordinal); + LoopMessageRef::new(format!("msg:snippet.{slug}.{ordinal}.{hash:016x}")).map_err(|_| { + AgentLoopHostError::new( + AgentLoopHostErrorKind::Internal, + "skill context snippet reference could not be represented", + ) + }) +} + +pub(crate) fn is_snippet_model_message_ref(content_ref: &LoopMessageRef) -> bool { + content_ref.as_str().starts_with("msg:snippet.") +} + +fn sanitize_ref_suffix(value: &str) -> String { + let mut suffix = String::with_capacity(value.len().min(96)); + for character in value.chars() { + if character.is_ascii_alphanumeric() || matches!(character, '_' | '-' | '.') { + suffix.push(character); + } else { + suffix.push('.'); + } + if suffix.len() >= 96 { + break; + } + } + let suffix = suffix.trim_matches('.'); + if suffix.is_empty() { + "context".to_string() + } else { + suffix.to_string() + } +} + +fn stable_snippet_ref_hash(snippet_ref: &str, safe_summary: &str, ordinal: usize) -> u64 { + let mut hash = FNV_OFFSET; + feed_hash(&mut hash, snippet_ref.as_bytes()); + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, safe_summary.as_bytes()); + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, ordinal.to_string().as_bytes()); + hash +} + +const FNV_OFFSET: u64 = 0xcbf29ce484222325; +const FNV_PRIME: u64 = 0x00000100000001B3; + +fn feed_hash(hash: &mut u64, bytes: &[u8]) { + for &byte in bytes { + *hash ^= u64::from(byte); + *hash = hash.wrapping_mul(FNV_PRIME); + } +} diff --git a/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs b/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs index ebcfaccae9b..e1cf3ba74d6 100644 --- a/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs +++ b/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs @@ -8,9 +8,11 @@ use ironclaw_host_api::{AgentId, CapabilityId, ProjectId, TenantId, ThreadId, Us use ironclaw_loop_support::{ EmptyLoopCapabilityPort, HostManagedModelError, HostManagedModelErrorKind, HostManagedModelGateway, HostManagedModelMessageRole, HostManagedModelRequest, - HostManagedModelResponse, ThreadBackedLoopContextPort, ThreadBackedLoopModelPort, + HostManagedModelResponse, HostSkillContextBuildError, HostSkillContextCandidate, + HostSkillContextSource, ThreadBackedLoopContextPort, ThreadBackedLoopModelPort, ThreadBackedLoopTranscriptPort, }; +use ironclaw_skills::SkillTrust; use ironclaw_threads::{ AcceptInboundMessageRequest, AcceptedInboundMessage, AppendAssistantDraftRequest, ContextMessage, ContextWindow, CreateSummaryArtifactRequest, EnsureThreadRequest, @@ -25,11 +27,11 @@ use ironclaw_turns::{ run_profile::{ AgentLoopHostErrorKind, AssistantReply, BeginAssistantDraft, CapabilityDeniedReasonKind, CapabilityInputRef, CapabilityInvocation, CapabilityOutcome, CapabilitySurfaceVersion, - FinalizeAssistantMessage, InMemoryLoopHostMilestoneSink, InMemoryRunProfileResolver, - LoopCapabilityPort, LoopContextPort, LoopContextRequest, LoopHostMilestoneKind, - LoopInputCursor, LoopInputCursorToken, LoopModelMessage, LoopModelPort, LoopModelRequest, - LoopRunContext, LoopTranscriptPort, ParentLoopOutput, UpdateAssistantDraft, - VisibleCapabilityRequest, + FinalizeAssistantMessage, HostManagedLoopPromptPort, InMemoryLoopHostMilestoneSink, + InMemoryRunProfileResolver, LoopCapabilityPort, LoopContextPort, LoopContextRequest, + LoopHostMilestoneKind, LoopInputCursor, LoopInputCursorToken, LoopModelMessage, + LoopModelPort, LoopModelRequest, LoopPromptPort, LoopRunContext, LoopTranscriptPort, + ParentLoopOutput, SkillVisibility, UpdateAssistantDraft, VisibleCapabilityRequest, }, }; use tracing_test::traced_test; @@ -110,6 +112,428 @@ async fn thread_context_port_preserves_summary_replacements_as_system_messages() assert!(bundle.instruction_snippets.is_empty()); } +#[tokio::test] +async fn thread_context_port_builds_skill_instruction_snippets_from_real_skill_md() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md( + "alpha", + "safe alpha description", + "Use alpha prompt content.", + ), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + ), + ])); + let adapter = ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source); + + let bundle = adapter + .load_loop_context(LoopContextRequest { + after: None, + limit: 16, + }) + .await + .unwrap(); + + assert_eq!(bundle.instruction_snippets.len(), 1); + assert_eq!(bundle.instruction_snippets[0].snippet_ref, "skill:alpha"); + assert!( + bundle.instruction_snippets[0] + .safe_summary + .contains("safe alpha description") + ); + assert!( + bundle.instruction_snippets[0] + .safe_summary + .contains("Use alpha prompt content.") + ); + assert!(!bundle.instruction_snippets[0].safe_summary.contains("/tmp")); +} + +#[tokio::test] +async fn thread_context_port_filters_skill_visibility_and_installed_prompt_content() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md("alpha", "installed description", "installed prompt secret"), + Some(SkillTrust::Installed), + Some(SkillVisibility::Visible), + ), + HostSkillContextCandidate::new( + skill_md("hidden", "hidden description", "hidden prompt"), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Hidden), + ), + HostSkillContextCandidate::new( + skill_md("denied", "denied description", "denied prompt"), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Denied), + ), + ])); + let adapter = ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source); + + let bundle = adapter + .load_loop_context(LoopContextRequest { + after: None, + limit: 16, + }) + .await + .unwrap(); + + assert_eq!(bundle.instruction_snippets.len(), 1); + assert_eq!(bundle.instruction_snippets[0].snippet_ref, "skill:alpha"); + assert!( + bundle.instruction_snippets[0] + .safe_summary + .contains("installed description") + ); + assert!( + !bundle.instruction_snippets[0] + .safe_summary + .contains("installed prompt secret") + ); + let serialized = serde_json::to_string(&bundle).unwrap(); + assert!(!serialized.contains("hidden")); + assert!(!serialized.contains("denied")); +} + +#[tokio::test] +async fn thread_context_port_ignores_malformed_hidden_skill_content() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + "not valid SKILL.md", + Some(SkillTrust::Trusted), + Some(SkillVisibility::Hidden), + ), + HostSkillContextCandidate::unavailable( + Some(SkillTrust::Trusted), + Some(SkillVisibility::Denied), + ), + HostSkillContextCandidate::new( + skill_md("alpha", "visible description", "visible prompt"), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + ), + ])); + let adapter = ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source); + + let bundle = adapter + .load_loop_context(LoopContextRequest { + after: None, + limit: 16, + }) + .await + .unwrap(); + + assert_eq!(bundle.instruction_snippets.len(), 1); + assert_eq!(bundle.instruction_snippets[0].snippet_ref, "skill:alpha"); + assert!( + bundle.instruction_snippets[0] + .safe_summary + .contains("visible prompt") + ); +} + +#[tokio::test] +async fn thread_context_port_fails_closed_when_visible_skill_content_is_missing() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::unavailable( + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + ), + ])); + let adapter = ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source); + + let error = adapter + .load_loop_context(LoopContextRequest { + after: None, + limit: 16, + }) + .await + .unwrap_err(); + + assert_eq!(error.kind, AgentLoopHostErrorKind::Unavailable); +} + +#[tokio::test] +async fn thread_context_port_fails_closed_when_skill_policy_data_is_missing() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md( + "alpha", + "safe alpha description", + "Use alpha prompt content.", + ), + None, + Some(SkillVisibility::Visible), + ), + ])); + let adapter = ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source); + + let error = adapter + .load_loop_context(LoopContextRequest { + after: None, + limit: 16, + }) + .await + .unwrap_err(); + + assert_eq!(error.kind, AgentLoopHostErrorKind::PolicyDenied); + assert!(!serde_json::to_string(&error).unwrap().contains("alpha")); +} + +#[tokio::test] +async fn prompt_and_model_ports_send_selected_skill_context_to_gateway() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md( + "alpha", + "safe alpha description", + "Use alpha prompt content.", + ), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + ), + ])); + let context_port = Arc::new( + ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source.clone()), + ); + let milestones = Arc::new(InMemoryLoopHostMilestoneSink::default()); + let prompt_port = + HostManagedLoopPromptPort::new(fixture.run_context.clone(), context_port, milestones); + let prompt_bundle = prompt_port + .build_prompt_bundle(ironclaw_turns::run_profile::LoopPromptBundleRequest { + mode: ironclaw_turns::run_profile::PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: None, + }) + .await + .unwrap(); + assert_eq!(prompt_bundle.messages.len(), 2); + assert_eq!(prompt_bundle.messages[0].role, "system"); + assert!( + prompt_bundle.messages[0] + .content_ref + .as_str() + .starts_with("msg:snippet.skill.alpha.") + ); + + let gateway = Arc::new(RecordingGateway::reply("model says hi")); + let model_port = ThreadBackedLoopModelPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + gateway.clone(), + 16, + ) + .with_skill_context_source(source); + + model_port + .stream_model(LoopModelRequest { + messages: prompt_bundle.messages, + surface_version: None, + model_preference: None, + }) + .await + .unwrap(); + + let calls = gateway.calls.lock().unwrap(); + assert_eq!( + calls[0].messages[0].role, + HostManagedModelMessageRole::System + ); + assert!( + calls[0].messages[0] + .content + .contains("safe alpha description") + ); + assert!( + calls[0].messages[0] + .content + .contains("Use alpha prompt content.") + ); + assert_eq!(calls[0].messages[1].role, HostManagedModelMessageRole::User); + assert_eq!(calls[0].messages[1].content, "hello reborn"); +} + +#[tokio::test] +async fn prompt_and_model_ports_keep_duplicate_skill_names_distinct() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md("alpha", "first description", "first prompt"), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + ) + .with_ordering_key("alpha-1"), + HostSkillContextCandidate::new( + skill_md("alpha", "second description", "second prompt"), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + ) + .with_ordering_key("alpha-2"), + ])); + let context_port = Arc::new( + ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source.clone()), + ); + let prompt_port = HostManagedLoopPromptPort::new( + fixture.run_context.clone(), + context_port, + Arc::new(InMemoryLoopHostMilestoneSink::default()), + ); + let prompt_bundle = prompt_port + .build_prompt_bundle(ironclaw_turns::run_profile::LoopPromptBundleRequest { + mode: ironclaw_turns::run_profile::PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: None, + }) + .await + .unwrap(); + + assert_eq!(prompt_bundle.messages.len(), 3); + assert_ne!( + prompt_bundle.messages[0].content_ref, + prompt_bundle.messages[1].content_ref + ); + + let gateway = Arc::new(RecordingGateway::reply("model says hi")); + let model_port = ThreadBackedLoopModelPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + gateway.clone(), + 16, + ) + .with_skill_context_source(source); + + model_port + .stream_model(LoopModelRequest { + messages: prompt_bundle.messages, + surface_version: None, + model_preference: None, + }) + .await + .unwrap(); + + let calls = gateway.calls.lock().unwrap(); + assert!(calls[0].messages[0].content.contains("first prompt")); + assert!(calls[0].messages[1].content.contains("second prompt")); +} + +#[tokio::test] +async fn model_port_rejects_skill_context_refs_when_source_changes_after_prompt_build() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(MutableSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md("alpha", "original description", "original prompt"), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + ), + ])); + let context_port = Arc::new( + ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source.clone()), + ); + let prompt_port = HostManagedLoopPromptPort::new( + fixture.run_context.clone(), + context_port, + Arc::new(InMemoryLoopHostMilestoneSink::default()), + ); + let prompt_bundle = prompt_port + .build_prompt_bundle(ironclaw_turns::run_profile::LoopPromptBundleRequest { + mode: ironclaw_turns::run_profile::PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: None, + }) + .await + .unwrap(); + + source.set(vec![HostSkillContextCandidate::new( + skill_md("alpha", "changed description", "changed prompt"), + Some(SkillTrust::Trusted), + Some(SkillVisibility::Visible), + )]); + let gateway = Arc::new(RecordingGateway::reply("should not be called")); + let model_port = ThreadBackedLoopModelPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + gateway.clone(), + 16, + ) + .with_skill_context_source(source); + + let error = model_port + .stream_model(LoopModelRequest { + messages: prompt_bundle.messages, + surface_version: None, + model_preference: None, + }) + .await + .unwrap_err(); + + assert_eq!(error.kind, AgentLoopHostErrorKind::InvalidInvocation); + assert!(gateway.calls.lock().unwrap().is_empty()); +} + #[tokio::test] async fn thread_context_port_rejects_non_origin_context_cursor() { let fixture = ThreadFixture::new().await; @@ -895,6 +1319,59 @@ async fn model_port_surfaces_fail_closed_gateway_policy_errors_without_raw_detai assert!(!wire.contains("RAW_PROVIDER_SECRET")); } +#[derive(Clone)] +struct StaticSkillContextSource { + candidates: Vec, +} + +impl StaticSkillContextSource { + fn new(candidates: Vec) -> Self { + Self { candidates } + } +} + +#[async_trait] +impl HostSkillContextSource for StaticSkillContextSource { + async fn load_skill_context_candidates( + &self, + _run_context: &LoopRunContext, + ) -> Result, HostSkillContextBuildError> { + Ok(self.candidates.clone()) + } +} + +struct MutableSkillContextSource { + candidates: Mutex>, +} + +impl MutableSkillContextSource { + fn new(candidates: Vec) -> Self { + Self { + candidates: Mutex::new(candidates), + } + } + + fn set(&self, candidates: Vec) { + *self.candidates.lock().unwrap() = candidates; + } +} + +#[async_trait] +impl HostSkillContextSource for MutableSkillContextSource { + async fn load_skill_context_candidates( + &self, + _run_context: &LoopRunContext, + ) -> Result, HostSkillContextBuildError> { + Ok(self.candidates.lock().unwrap().clone()) + } +} + +fn skill_md(name: &str, description: &str, prompt: &str) -> String { + format!( + "---\nname: {name}\ndescription: {description}\nactivation:\n keywords: [{name}]\n---\n\n{prompt}\n" + ) +} + struct ThreadFixture { thread_service: Arc, thread_scope: ThreadScope, diff --git a/crates/ironclaw_reborn/src/loop_driver_host.rs b/crates/ironclaw_reborn/src/loop_driver_host.rs index 47e53fa9234..56c95338cc4 100644 --- a/crates/ironclaw_reborn/src/loop_driver_host.rs +++ b/crates/ironclaw_reborn/src/loop_driver_host.rs @@ -2,8 +2,8 @@ use std::{error::Error, fmt, sync::Arc}; use async_trait::async_trait; use ironclaw_loop_support::{ - EmptyLoopCapabilityPort, HostManagedModelGateway, ThreadBackedLoopContextPort, - ThreadBackedLoopModelPort, ThreadBackedLoopTranscriptPort, + EmptyLoopCapabilityPort, HostManagedModelGateway, HostSkillContextSource, + ThreadBackedLoopContextPort, ThreadBackedLoopModelPort, ThreadBackedLoopTranscriptPort, }; use ironclaw_threads::{SessionThreadService, ThreadScope}; use ironclaw_turns::{ @@ -73,6 +73,7 @@ where loop_checkpoint_store: Arc, milestone_sink: Arc, config: TextOnlyLoopHostConfig, + skill_context_source: Option>, } impl RebornLoopDriverHostFactory @@ -97,9 +98,15 @@ where loop_checkpoint_store, milestone_sink, config, + skill_context_source: None, } } + pub fn with_skill_context_source(mut self, source: Arc) -> Self { + self.skill_context_source = Some(source); + self + } + pub async fn build_text_only_host( &self, request: RebornLoopDriverHostRequest, @@ -109,12 +116,16 @@ where let max_messages = self.config.max_messages.max(1); let run_context = request.loop_run_context; - let context: Arc = Arc::new(ThreadBackedLoopContextPort::new( + let mut context_adapter = ThreadBackedLoopContextPort::new( Arc::clone(&self.thread_service), self.thread_scope.clone(), run_context.clone(), max_messages, - )); + ); + if let Some(source) = self.skill_context_source.clone() { + context_adapter = context_adapter.with_skill_context_source(source); + } + let context: Arc = Arc::new(context_adapter); let current_surface_version = EmptyLoopCapabilityPort .visible_capabilities(VisibleCapabilityRequest) .await @@ -133,15 +144,18 @@ where ); let input: Arc = Arc::new(NoExtraLoopInputPort::new(run_context.clone())); - let model: Arc = - Arc::new(ThreadBackedLoopModelPort::with_milestone_sink( - Arc::clone(&self.thread_service), - self.thread_scope.clone(), - run_context.clone(), - Arc::clone(&self.model_gateway), - max_messages, - Arc::clone(&self.milestone_sink), - )); + let mut model_adapter = ThreadBackedLoopModelPort::with_milestone_sink( + Arc::clone(&self.thread_service), + self.thread_scope.clone(), + run_context.clone(), + Arc::clone(&self.model_gateway), + max_messages, + Arc::clone(&self.milestone_sink), + ); + if let Some(source) = self.skill_context_source.clone() { + model_adapter = model_adapter.with_skill_context_source(source); + } + let model: Arc = Arc::new(model_adapter); let checkpoint: Arc = Arc::new(HostManagedLoopCheckpointPort::new( run_context.clone(), Arc::clone(&self.checkpoint_state_store), diff --git a/crates/ironclaw_turns/src/run_profile/prompt.rs b/crates/ironclaw_turns/src/run_profile/prompt.rs index 0cbeef0561d..4c328dac57e 100644 --- a/crates/ironclaw_turns/src/run_profile/prompt.rs +++ b/crates/ironclaw_turns/src/run_profile/prompt.rs @@ -19,8 +19,8 @@ const MAX_TEXT_ONLY_MESSAGE_LIMIT: usize = 128; /// [`LoopContextPort`], returns model-message references, and emits a /// `prompt_bundle_built` milestone containing only metadata. It currently /// supports [`PromptMode::TextOnly`] only; checkpoint-backed prompt state and -/// instruction/memory snippet materialization fail closed until dedicated host -/// stores are wired. +/// memory snippet materialization fail closed until dedicated host stores are +/// wired. Instruction snippets are surfaced as host-owned system message refs. #[derive(Clone)] pub struct HostManagedLoopPromptPort where @@ -140,10 +140,10 @@ where fn ensure_supported_context_shape( context: &LoopContextBundle, ) -> Result<(), AgentLoopHostError> { - if !context.instruction_snippets.is_empty() || !context.memory_snippets.is_empty() { + if !context.memory_snippets.is_empty() { return Err(AgentLoopHostError::new( AgentLoopHostErrorKind::PolicyDenied, - "text-only prompt port cannot materialize instruction or memory snippets", + "text-only prompt port cannot materialize memory snippets", )); } Ok(()) @@ -169,14 +169,30 @@ where }) .await?; Self::ensure_supported_context_shape(&context)?; - let messages = context - .messages + let mut messages = context + .instruction_snippets .into_iter() - .map(|message| LoopModelMessage { - role: message.role, - content_ref: message.message_ref, + .enumerate() + .map(|(ordinal, snippet)| { + Ok(LoopModelMessage { + role: "system".to_string(), + content_ref: snippet_model_message_ref( + &snippet.snippet_ref, + &snippet.safe_summary, + ordinal, + )?, + }) }) - .collect::>(); + .collect::, AgentLoopHostError>>()?; + messages.extend( + context + .messages + .into_iter() + .map(|message| LoopModelMessage { + role: message.role, + content_ref: message.message_ref, + }), + ); let bundle = LoopPromptBundle { bundle_ref: LoopPromptBundleRef::fresh_for_run(&self.context), messages, @@ -193,3 +209,58 @@ where Ok(bundle) } } + +fn snippet_model_message_ref( + snippet_ref: &str, + safe_summary: &str, + ordinal: usize, +) -> Result { + let slug = sanitize_ref_suffix(snippet_ref); + let hash = stable_snippet_ref_hash(snippet_ref, safe_summary, ordinal); + crate::LoopMessageRef::new(format!("msg:snippet.{slug}.{ordinal}.{hash:016x}")).map_err(|_| { + AgentLoopHostError::new( + AgentLoopHostErrorKind::Internal, + "instruction snippet reference could not be represented", + ) + }) +} + +fn sanitize_ref_suffix(value: &str) -> String { + let mut suffix = String::with_capacity(value.len().min(96)); + for character in value.chars() { + if character.is_ascii_alphanumeric() || matches!(character, '_' | '-' | '.') { + suffix.push(character); + } else { + suffix.push('.'); + } + if suffix.len() >= 96 { + break; + } + } + let suffix = suffix.trim_matches('.'); + if suffix.is_empty() { + "context".to_string() + } else { + suffix.to_string() + } +} + +fn stable_snippet_ref_hash(snippet_ref: &str, safe_summary: &str, ordinal: usize) -> u64 { + let mut hash = FNV_OFFSET; + feed_hash(&mut hash, snippet_ref.as_bytes()); + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, safe_summary.as_bytes()); + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, ordinal.to_string().as_bytes()); + hash +} + +const FNV_OFFSET: u64 = 0xcbf29ce484222325; +const FNV_PRIME: u64 = 0x00000100000001B3; + +fn feed_hash(hash: &mut u64, bytes: &[u8]) { + for &byte in bytes { + *hash ^= u64::from(byte); + *hash = hash.wrapping_mul(FNV_PRIME); + } +} diff --git a/crates/ironclaw_turns/tests/agent_loop_host_contract.rs b/crates/ironclaw_turns/tests/agent_loop_host_contract.rs index da77727e705..afb95e15abd 100644 --- a/crates/ironclaw_turns/tests/agent_loop_host_contract.rs +++ b/crates/ironclaw_turns/tests/agent_loop_host_contract.rs @@ -267,6 +267,42 @@ async fn loop_prompt_port_builds_text_only_bundle_from_context_refs() { assert_eq!(host.milestone_kind_names(), vec!["prompt_bundle_built"]); } +#[tokio::test] +async fn loop_prompt_port_materializes_instruction_snippets_as_system_refs() { + let host = Arc::new( + RecordingAgentLoopHost::new(claimed_run_context().await) + .with_context_instruction_snippet("skill:alpha", "alpha skill context available"), + ); + let port = HostManagedLoopPromptPort::new( + host.context.clone(), + host.clone(), + host.milestone_sink.clone(), + ); + + let bundle = port + .build_prompt_bundle(LoopPromptBundleRequest { + mode: PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: Some(8), + }) + .await + .unwrap(); + + assert_eq!(bundle.messages.len(), 2); + assert_eq!(bundle.messages[0].role, "system"); + assert!( + bundle.messages[0] + .content_ref + .as_str() + .starts_with("msg:snippet.skill.alpha.") + ); + assert_eq!(bundle.messages[1].role, "user"); + assert_eq!(host.effects(), vec!["context"]); + assert_eq!(host.milestone_kind_names(), vec!["prompt_bundle_built"]); +} + #[tokio::test] async fn loop_prompt_port_rejects_unsupported_prompt_mode() { let host = Arc::new(RecordingAgentLoopHost::new(codeact_run_context().await)); @@ -464,7 +500,7 @@ async fn loop_prompt_port_rejects_stale_surface_version() { } #[tokio::test] -async fn loop_prompt_port_rejects_snippets_it_cannot_materialize() { +async fn loop_prompt_port_rejects_memory_snippets_it_cannot_materialize() { let host = Arc::new( RecordingAgentLoopHost::new(claimed_run_context().await) .with_context_instruction_snippet("instruction:system", "system instruction available") From ec54e38982b5b73a90acc03c902e66bd722fa078 Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Mon, 11 May 2026 22:51:24 +0200 Subject: [PATCH 2/2] fix(reborn): address skill context review feedback (#3476) --- Cargo.lock | 1 + crates/ironclaw_loop_support/src/lib.rs | 1 + .../src/skill_context.rs | 83 ++------ .../tests/thread_loop_support_contract.rs | 87 +++++++- crates/ironclaw_reborn/Cargo.toml | 1 + .../ironclaw_reborn/src/loop_driver_host.rs | 8 +- .../ironclaw_reborn/tests/loop_driver_host.rs | 93 ++++++++- crates/ironclaw_turns/src/run_profile/host.rs | 13 ++ .../src/run_profile/milestones.rs | 15 +- crates/ironclaw_turns/src/run_profile/mod.rs | 18 +- .../ironclaw_turns/src/run_profile/prompt.rs | 124 +++++------- .../src/run_profile/skill_context.rs | 141 ++++++++++--- .../tests/agent_loop_host_contract.rs | 185 ++++++++++++++++-- 13 files changed, 568 insertions(+), 202 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 83b3d4b9b75..82ca654c306 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4522,6 +4522,7 @@ dependencies = [ "ironclaw_llm", "ironclaw_loop_support", "ironclaw_secrets", + "ironclaw_skills", "ironclaw_threads", "ironclaw_turns", "libsql", diff --git a/crates/ironclaw_loop_support/src/lib.rs b/crates/ironclaw_loop_support/src/lib.rs index 64145fb44bb..0a560f8ec92 100644 --- a/crates/ironclaw_loop_support/src/lib.rs +++ b/crates/ironclaw_loop_support/src/lib.rs @@ -120,6 +120,7 @@ where }; Ok(LoopContextBundle { + identity_messages: Vec::new(), messages: context .messages .into_iter() diff --git a/crates/ironclaw_loop_support/src/skill_context.rs b/crates/ironclaw_loop_support/src/skill_context.rs index 6e18854318b..0131d672b57 100644 --- a/crates/ironclaw_loop_support/src/skill_context.rs +++ b/crates/ironclaw_loop_support/src/skill_context.rs @@ -1,12 +1,13 @@ use async_trait::async_trait; use ironclaw_skills::{ParsedSkill, SkillTrust, parse_skill_md}; -use ironclaw_turns::{ - LoopMessageRef, - run_profile::{ - AgentLoopHostError, AgentLoopHostErrorKind, InstalledSkillSnapshot, LoopContextSnippet, - LoopRunContext, SkillContextError, SkillContextService, SkillContextSource, - SkillRunSnapshot, SkillTrustLevel, SkillVisibility, - }, +use ironclaw_turns::run_profile::{ + AgentLoopHostError, AgentLoopHostErrorKind, InstalledSkillSnapshot, LoopContextSnippet, + LoopRunContext, SkillContextError, SkillContextService, SkillContextSource, SkillRunSnapshot, + SkillTrustLevel, SkillVisibility, +}; +pub(crate) use ironclaw_turns::run_profile::{ + is_skill_snippet_model_message_ref as is_snippet_model_message_ref, + skill_snippet_model_message_ref as snippet_model_message_ref, }; use thiserror::Error; @@ -162,12 +163,17 @@ fn parsed_skill_to_snapshot_entry( ordering_key: Option, ) -> InstalledSkillSnapshot { let name = parsed.manifest.name; + let trust = skill_trust_level(trust); + let prompt_content = match trust { + SkillTrustLevel::Installed => None, + SkillTrustLevel::Trusted => Some(parsed.prompt_content), + }; InstalledSkillSnapshot { ordering_key: ordering_key.unwrap_or_else(|| name.clone()), name, - trust: skill_trust_level(trust), + trust, visibility, - prompt_content: Some(parsed.prompt_content), + prompt_content, safe_description: parsed.manifest.description, } } @@ -194,62 +200,3 @@ fn skill_context_error_to_host_error(error: SkillContextError) -> AgentLoopHostE }; build_error.into_host_error() } - -pub(crate) fn snippet_model_message_ref( - snippet_ref: &str, - safe_summary: &str, - ordinal: usize, -) -> Result { - let slug = sanitize_ref_suffix(snippet_ref); - let hash = stable_snippet_ref_hash(snippet_ref, safe_summary, ordinal); - LoopMessageRef::new(format!("msg:snippet.{slug}.{ordinal}.{hash:016x}")).map_err(|_| { - AgentLoopHostError::new( - AgentLoopHostErrorKind::Internal, - "skill context snippet reference could not be represented", - ) - }) -} - -pub(crate) fn is_snippet_model_message_ref(content_ref: &LoopMessageRef) -> bool { - content_ref.as_str().starts_with("msg:snippet.") -} - -fn sanitize_ref_suffix(value: &str) -> String { - let mut suffix = String::with_capacity(value.len().min(96)); - for character in value.chars() { - if character.is_ascii_alphanumeric() || matches!(character, '_' | '-' | '.') { - suffix.push(character); - } else { - suffix.push('.'); - } - if suffix.len() >= 96 { - break; - } - } - let suffix = suffix.trim_matches('.'); - if suffix.is_empty() { - "context".to_string() - } else { - suffix.to_string() - } -} - -fn stable_snippet_ref_hash(snippet_ref: &str, safe_summary: &str, ordinal: usize) -> u64 { - let mut hash = FNV_OFFSET; - feed_hash(&mut hash, snippet_ref.as_bytes()); - feed_hash(&mut hash, &[0xFF]); - feed_hash(&mut hash, safe_summary.as_bytes()); - feed_hash(&mut hash, &[0xFF]); - feed_hash(&mut hash, ordinal.to_string().as_bytes()); - hash -} - -const FNV_OFFSET: u64 = 0xcbf29ce484222325; -const FNV_PRIME: u64 = 0x00000100000001B3; - -fn feed_hash(hash: &mut u64, bytes: &[u8]) { - for &byte in bytes { - *hash ^= u64::from(byte); - *hash = hash.wrapping_mul(FNV_PRIME); - } -} diff --git a/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs b/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs index e1cf3ba74d6..8f5051403bd 100644 --- a/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs +++ b/crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs @@ -10,7 +10,7 @@ use ironclaw_loop_support::{ HostManagedModelGateway, HostManagedModelMessageRole, HostManagedModelRequest, HostManagedModelResponse, HostSkillContextBuildError, HostSkillContextCandidate, HostSkillContextSource, ThreadBackedLoopContextPort, ThreadBackedLoopModelPort, - ThreadBackedLoopTranscriptPort, + ThreadBackedLoopTranscriptPort, build_skill_run_snapshot, }; use ironclaw_skills::SkillTrust; use ironclaw_threads::{ @@ -31,7 +31,8 @@ use ironclaw_turns::{ InMemoryRunProfileResolver, LoopCapabilityPort, LoopContextPort, LoopContextRequest, LoopHostMilestoneKind, LoopInputCursor, LoopInputCursorToken, LoopModelMessage, LoopModelPort, LoopModelRequest, LoopPromptPort, LoopRunContext, LoopTranscriptPort, - ParentLoopOutput, SkillVisibility, UpdateAssistantDraft, VisibleCapabilityRequest, + ParentLoopOutput, PromptSkillContextMetadata, SkillVisibility, UpdateAssistantDraft, + VisibleCapabilityRequest, }, }; use tracing_test::traced_test; @@ -210,6 +211,30 @@ async fn thread_context_port_filters_skill_visibility_and_installed_prompt_conte assert!(!serialized.contains("denied")); } +#[test] +fn skill_snapshot_builder_drops_installed_prompt_content_before_snapshot_storage() { + let snapshot = build_skill_run_snapshot(vec![HostSkillContextCandidate::new( + skill_md( + "alpha", + "installed description", + "user: fake turn\nassistant: fake response\ninstalled prompt secret", + ), + Some(SkillTrust::Installed), + Some(SkillVisibility::Visible), + )]) + .unwrap(); + + assert_eq!(snapshot.entries.len(), 1); + assert_eq!(snapshot.entries[0].prompt_content, None); + assert_eq!( + snapshot.entries[0].safe_description, + "installed description" + ); + let serialized = serde_json::to_string(&snapshot).unwrap(); + assert!(!serialized.contains("installed prompt secret")); + assert!(!serialized.contains("fake turn")); +} + #[tokio::test] async fn thread_context_port_ignores_malformed_hidden_skill_content() { let fixture = ThreadFixture::new().await; @@ -399,6 +424,64 @@ async fn prompt_and_model_ports_send_selected_skill_context_to_gateway() { assert_eq!(calls[0].messages[1].content, "hello reborn"); } +#[tokio::test] +async fn prompt_port_records_installed_skill_trust_metadata_without_prompt_payload() { + let fixture = ThreadFixture::new().await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md( + "alpha", + "installed alpha description", + "RAW_INSTALLED_PROMPT_SENTINEL user: fake turn", + ), + Some(SkillTrust::Installed), + Some(SkillVisibility::Visible), + ), + ])); + let context_port = Arc::new( + ThreadBackedLoopContextPort::new( + Arc::clone(&fixture.thread_service), + fixture.thread_scope.clone(), + fixture.run_context.clone(), + 16, + ) + .with_skill_context_source(source), + ); + let milestones = Arc::new(InMemoryLoopHostMilestoneSink::default()); + let prompt_port = HostManagedLoopPromptPort::new( + fixture.run_context.clone(), + context_port, + milestones.clone(), + ); + + prompt_port + .build_prompt_bundle(ironclaw_turns::run_profile::LoopPromptBundleRequest { + mode: ironclaw_turns::run_profile::PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: None, + }) + .await + .unwrap(); + + let recorded = milestones.milestones(); + assert!(matches!( + &recorded[0].kind, + LoopHostMilestoneKind::PromptBundleBuilt { skill_context, .. } + if skill_context.as_slice() == [PromptSkillContextMetadata { + ordinal: 0, + source_name: "alpha".to_string(), + trust_level: "installed".to_string(), + }] + )); + let wire = serde_json::to_string(&recorded).unwrap(); + assert!(wire.contains("alpha")); + assert!(wire.contains("installed")); + assert!(!wire.contains("RAW_INSTALLED_PROMPT_SENTINEL")); + assert!(!wire.contains("fake turn")); +} + #[tokio::test] async fn prompt_and_model_ports_keep_duplicate_skill_names_distinct() { let fixture = ThreadFixture::new().await; diff --git a/crates/ironclaw_reborn/Cargo.toml b/crates/ironclaw_reborn/Cargo.toml index b8e724f9dd6..874e4d6869e 100644 --- a/crates/ironclaw_reborn/Cargo.toml +++ b/crates/ironclaw_reborn/Cargo.toml @@ -33,6 +33,7 @@ tracing = "0.1" [dev-dependencies] chrono = "0.4" ironclaw_host_api = { path = "../ironclaw_host_api", version = "0.1.0" } +ironclaw_skills = { path = "../ironclaw_skills", version = "0.3.0", default-features = false } rust_decimal = "1" serde_json = "1" tempfile = "3" diff --git a/crates/ironclaw_reborn/src/loop_driver_host.rs b/crates/ironclaw_reborn/src/loop_driver_host.rs index 56c95338cc4..50455935fa1 100644 --- a/crates/ironclaw_reborn/src/loop_driver_host.rs +++ b/crates/ironclaw_reborn/src/loop_driver_host.rs @@ -122,8 +122,8 @@ where run_context.clone(), max_messages, ); - if let Some(source) = self.skill_context_source.clone() { - context_adapter = context_adapter.with_skill_context_source(source); + if let Some(source) = self.skill_context_source.as_ref() { + context_adapter = context_adapter.with_skill_context_source(source.clone()); } let context: Arc = Arc::new(context_adapter); let current_surface_version = EmptyLoopCapabilityPort @@ -152,8 +152,8 @@ where max_messages, Arc::clone(&self.milestone_sink), ); - if let Some(source) = self.skill_context_source.clone() { - model_adapter = model_adapter.with_skill_context_source(source); + if let Some(source) = self.skill_context_source.as_ref() { + model_adapter = model_adapter.with_skill_context_source(source.clone()); } let model: Arc = Arc::new(model_adapter); let checkpoint: Arc = Arc::new(HostManagedLoopCheckpointPort::new( diff --git a/crates/ironclaw_reborn/tests/loop_driver_host.rs b/crates/ironclaw_reborn/tests/loop_driver_host.rs index 56a2933c8de..d66c15b4a18 100644 --- a/crates/ironclaw_reborn/tests/loop_driver_host.rs +++ b/crates/ironclaw_reborn/tests/loop_driver_host.rs @@ -5,12 +5,14 @@ use chrono::Utc; use ironclaw_host_api::{AgentId, CapabilityId, ProjectId, TenantId, ThreadId, UserId}; use ironclaw_loop_support::{ HostManagedModelError, HostManagedModelGateway, HostManagedModelRequest, - HostManagedModelResponse, + HostManagedModelResponse, HostSkillContextBuildError, HostSkillContextCandidate, + HostSkillContextSource, }; use ironclaw_reborn::{ RebornLoopDriverHostFactory, RebornLoopDriverHostRequest, TextOnlyLoopHostConfig, turn_runner::HostFactory, }; +use ironclaw_skills::SkillTrust; use ironclaw_threads::{ AcceptInboundMessageRequest, EnsureThreadRequest, InMemorySessionThreadService, MessageContent, MessageKind, MessageStatus, SessionThreadService, ThreadHistoryRequest, ThreadScope, @@ -30,7 +32,8 @@ use ironclaw_turns::{ LoopCheckpointKind, LoopCheckpointPort, LoopCheckpointRequest, LoopContextRequest, LoopDriverId, LoopDriverNoteKind, LoopHostMilestone, LoopInputCursor, LoopInputCursorToken, LoopInputPort, LoopModelRequest, LoopProgressEvent, LoopPromptBundleRequest, - LoopPromptPort, LoopRunContext, ParentLoopOutput, PromptMode, VisibleCapabilityRequest, + LoopPromptPort, LoopRunContext, ParentLoopOutput, PromptMode, SkillVisibility, + VisibleCapabilityRequest, }, runner::ClaimedTurnRun, }; @@ -640,6 +643,65 @@ async fn text_only_host_checkpoint_port_maps_store_failures_to_unavailable() { assert_eq!(error.kind, AgentLoopHostErrorKind::Unavailable); } +#[tokio::test] +async fn text_only_host_skill_context_does_not_expand_capability_surface() { + let fixture = HostFixture::new("thread-host-skill-capability", "hello").await; + let source = Arc::new(StaticSkillContextSource::new(vec![ + HostSkillContextCandidate::new( + skill_md( + "installed-alpha", + "installed skill description", + "installed prompt must not imply tool authority", + ), + Some(SkillTrust::Installed), + Some(SkillVisibility::Visible), + ), + ])); + let host = fixture + .factory() + .with_skill_context_source(source) + .build_text_only_host(RebornLoopDriverHostRequest { + claimed_run: fixture.claimed.clone(), + loop_run_context: fixture.context.clone(), + }) + .await + .unwrap(); + + let prompt_bundle = host + .build_prompt_bundle(LoopPromptBundleRequest { + mode: PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: Some(8), + }) + .await + .unwrap(); + assert_eq!(prompt_bundle.messages.len(), 2); + + let surface = host + .visible_capabilities(VisibleCapabilityRequest) + .await + .unwrap(); + assert!(surface.descriptors.is_empty()); + let outcome = host + .invoke_capability_batch(ironclaw_turns::run_profile::CapabilityBatchInvocation { + invocations: vec![CapabilityInvocation { + surface_version: surface.version, + capability_id: CapabilityId::new("demo.echo").unwrap(), + input_ref: CapabilityInputRef::new("input:opaque-tool-input").unwrap(), + }], + stop_on_first_suspension: true, + }) + .await + .unwrap(); + + assert!(matches!( + outcome.outcomes.as_slice(), + [CapabilityOutcome::Denied(denied)] if denied.reason_kind == CapabilityDeniedReasonKind::EmptySurface + )); +} + #[tokio::test] async fn text_only_host_empty_capability_surface_denies_invocation() { let fixture = HostFixture::new("thread-host-capability", "hello").await; @@ -677,6 +739,33 @@ async fn text_only_host_empty_capability_surface_denies_invocation() { assert_eq!(stale.kind, AgentLoopHostErrorKind::StaleSurface); } +#[derive(Clone)] +struct StaticSkillContextSource { + candidates: Vec, +} + +impl StaticSkillContextSource { + fn new(candidates: Vec) -> Self { + Self { candidates } + } +} + +#[async_trait] +impl HostSkillContextSource for StaticSkillContextSource { + async fn load_skill_context_candidates( + &self, + _run_context: &LoopRunContext, + ) -> Result, HostSkillContextBuildError> { + Ok(self.candidates.clone()) + } +} + +fn skill_md(name: &str, description: &str, prompt: &str) -> String { + format!( + "---\nname: {name}\ndescription: {description}\nactivation:\n keywords: [{name}]\n---\n\n{prompt}\n" + ) +} + struct HostFixture { thread_service: Arc, checkpoint_state_store: Arc, diff --git a/crates/ironclaw_turns/src/run_profile/host.rs b/crates/ironclaw_turns/src/run_profile/host.rs index a376ebfadc2..5b5c2a4c5f9 100644 --- a/crates/ironclaw_turns/src/run_profile/host.rs +++ b/crates/ironclaw_turns/src/run_profile/host.rs @@ -442,6 +442,8 @@ pub struct LoopContextRequest { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct LoopContextBundle { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub identity_messages: Vec, pub messages: Vec, pub instruction_snippets: Vec, pub memory_snippets: Vec, @@ -454,10 +456,21 @@ pub struct LoopContextMessage { pub safe_summary: String, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct LoopContextSnippetMetadata { + pub source_name: String, + pub trust_level: String, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct LoopContextSnippet { pub snippet_ref: String, pub safe_summary: String, + /// Safe metadata for prompt milestones. Skill snippet producers using the + /// `skill:` ref namespace must populate this so telemetry can record active + /// skill name/trust without leaking prompt content. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub metadata: Option, } #[async_trait] diff --git a/crates/ironclaw_turns/src/run_profile/milestones.rs b/crates/ironclaw_turns/src/run_profile/milestones.rs index e8261318274..5d8dbfb4d5e 100644 --- a/crates/ironclaw_turns/src/run_profile/milestones.rs +++ b/crates/ironclaw_turns/src/run_profile/milestones.rs @@ -36,6 +36,13 @@ impl LoopHostMilestone { } } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PromptSkillContextMetadata { + pub ordinal: usize, + pub source_name: String, + pub trust_level: String, +} + /// Public wire shape for host-loop milestones. /// /// Milestones may be serialized into traces or delivered across process @@ -43,8 +50,8 @@ impl LoopHostMilestone { /// [`LoopHostMilestoneKind::kind_name`] plus a catch-all branch rather than /// assuming the historical closed set. `PromptBundleBuilt` was added as an /// additive wire-format variant for prompt-bundle construction; it carries only -/// refs, mode, optional surface version, and counts, never raw prompt/model -/// content. +/// refs, mode, optional surface version, counts, and active-skill metadata, +/// never raw prompt/model content. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum LoopHostMilestoneKind { @@ -53,6 +60,8 @@ pub enum LoopHostMilestoneKind { mode: PromptMode, surface_version: Option, message_count: usize, + #[serde(default)] + skill_context: Vec, }, ModelStarted { requested_model_profile_id: Option, @@ -167,12 +176,14 @@ where mode: PromptMode, surface_version: Option, message_count: usize, + skill_context: Vec, ) -> Result<(), AgentLoopHostError> { self.publish(LoopHostMilestoneKind::PromptBundleBuilt { bundle_ref, mode, surface_version, message_count, + skill_context, }) .await } diff --git a/crates/ironclaw_turns/src/run_profile/mod.rs b/crates/ironclaw_turns/src/run_profile/mod.rs index 1f45d8968ee..e37d0b91e11 100644 --- a/crates/ironclaw_turns/src/run_profile/mod.rs +++ b/crates/ironclaw_turns/src/run_profile/mod.rs @@ -32,16 +32,17 @@ pub use host::{ CapabilitySurfaceVersion, FinalizeAssistantMessage, LoopCancelReasonKind, LoopCapabilityPort, LoopCheckpointKind, LoopCheckpointPort, LoopCheckpointRequest, LoopCheckpointStateRef, LoopContextBundle, LoopContextMessage, LoopContextPort, LoopContextRequest, LoopContextSnippet, - LoopDriverNoteKind, LoopInput, LoopInputBatch, LoopInputCursor, LoopInputCursorToken, - LoopInputPort, LoopInterruptKind, LoopModelMessage, LoopModelPort, LoopModelRequest, - LoopModelResponse, LoopProcessRef, LoopProgressEvent, LoopProgressPort, LoopPromptBundle, - LoopPromptBundleRef, LoopPromptBundleRequest, LoopPromptPort, LoopRunContext, LoopRunInfoPort, - LoopSafeSummary, LoopTranscriptPort, ModelStreamChunk, ParentLoopOutput, ProcessHandleSummary, - PromptMode, UpdateAssistantDraft, VisibleCapabilityRequest, VisibleCapabilitySurface, + LoopContextSnippetMetadata, LoopDriverNoteKind, LoopInput, LoopInputBatch, LoopInputCursor, + LoopInputCursorToken, LoopInputPort, LoopInterruptKind, LoopModelMessage, LoopModelPort, + LoopModelRequest, LoopModelResponse, LoopProcessRef, LoopProgressEvent, LoopProgressPort, + LoopPromptBundle, LoopPromptBundleRef, LoopPromptBundleRequest, LoopPromptPort, LoopRunContext, + LoopRunInfoPort, LoopSafeSummary, LoopTranscriptPort, ModelStreamChunk, ParentLoopOutput, + ProcessHandleSummary, PromptMode, UpdateAssistantDraft, VisibleCapabilityRequest, + VisibleCapabilitySurface, }; pub use milestones::{ InMemoryLoopHostMilestoneSink, LoopHostMilestone, LoopHostMilestoneEmitter, - LoopHostMilestoneKind, LoopHostMilestoneSink, + LoopHostMilestoneKind, LoopHostMilestoneSink, PromptSkillContextMetadata, }; pub use model::{ HostManagedLoopModelPort, LoopModelGateway, LoopModelGatewayError, LoopModelGatewayRequest, @@ -65,6 +66,7 @@ pub use resolver::{ pub use skill_context::{ InstalledSkillSnapshot, NoopSkillContextSource, SkillContextBudget, SkillContextError, SkillContextService, SkillContextSnippet, SkillContextSource, SkillRunSnapshot, - SkillTrustLevel, SkillVisibility, + SkillTrustLevel, SkillVisibility, is_skill_snippet_model_message_ref, + skill_snippet_model_message_ref, }; pub use snapshot::ResolvedRunProfile; diff --git a/crates/ironclaw_turns/src/run_profile/prompt.rs b/crates/ironclaw_turns/src/run_profile/prompt.rs index 4c328dac57e..ca4a70ef1d9 100644 --- a/crates/ironclaw_turns/src/run_profile/prompt.rs +++ b/crates/ironclaw_turns/src/run_profile/prompt.rs @@ -4,10 +4,13 @@ use async_trait::async_trait; use super::host::{ AgentLoopHostError, AgentLoopHostErrorKind, CapabilitySurfaceVersion, LoopContextBundle, - LoopContextPort, LoopContextRequest, LoopModelMessage, LoopPromptBundle, LoopPromptBundleRef, - LoopPromptBundleRequest, LoopPromptPort, LoopRunContext, PromptMode, + LoopContextMessage, LoopContextPort, LoopContextRequest, LoopModelMessage, LoopPromptBundle, + LoopPromptBundleRef, LoopPromptBundleRequest, LoopPromptPort, LoopRunContext, PromptMode, }; -use super::milestones::{LoopHostMilestoneEmitter, LoopHostMilestoneSink}; +use super::milestones::{ + LoopHostMilestoneEmitter, LoopHostMilestoneSink, PromptSkillContextMetadata, +}; +use super::skill_context::skill_snippet_model_message_ref; const DEFAULT_TEXT_ONLY_MESSAGE_LIMIT: usize = 32; const MAX_TEXT_ONLY_MESSAGE_LIMIT: usize = 128; @@ -169,29 +172,49 @@ where }) .await?; Self::ensure_supported_context_shape(&context)?; - let mut messages = context - .instruction_snippets - .into_iter() - .enumerate() - .map(|(ordinal, snippet)| { - Ok(LoopModelMessage { - role: "system".to_string(), - content_ref: snippet_model_message_ref( - &snippet.snippet_ref, - &snippet.safe_summary, - ordinal, - )?, - }) - }) - .collect::, AgentLoopHostError>>()?; + let mut messages = Vec::with_capacity( + context.identity_messages.len() + + context.instruction_snippets.len() + + context.messages.len(), + ); messages.extend( context - .messages + .identity_messages .into_iter() - .map(|message| LoopModelMessage { - role: message.role, - content_ref: message.message_ref, + .map(context_message_to_model_message), + ); + + let mut skill_context = Vec::with_capacity(context.instruction_snippets.len()); + for (ordinal, snippet) in context.instruction_snippets.into_iter().enumerate() { + let content_ref = skill_snippet_model_message_ref( + &snippet.snippet_ref, + &snippet.safe_summary, + ordinal, + )?; + match snippet.metadata.as_ref() { + Some(metadata) => skill_context.push(PromptSkillContextMetadata { + ordinal, + source_name: metadata.source_name.clone(), + trust_level: metadata.trust_level.clone(), }), + None if snippet.snippet_ref.starts_with("skill:") => { + return Err(AgentLoopHostError::new( + AgentLoopHostErrorKind::Internal, + "skill instruction snippet metadata is missing", + )); + } + None => {} + } + messages.push(LoopModelMessage { + role: "system".to_string(), + content_ref, + }); + } + messages.extend( + context + .messages + .into_iter() + .map(context_message_to_model_message), ); let bundle = LoopPromptBundle { bundle_ref: LoopPromptBundleRef::fresh_for_run(&self.context), @@ -204,63 +227,16 @@ where request.mode, bundle.surface_version.clone(), bundle.messages.len(), + skill_context, ) .await?; Ok(bundle) } } -fn snippet_model_message_ref( - snippet_ref: &str, - safe_summary: &str, - ordinal: usize, -) -> Result { - let slug = sanitize_ref_suffix(snippet_ref); - let hash = stable_snippet_ref_hash(snippet_ref, safe_summary, ordinal); - crate::LoopMessageRef::new(format!("msg:snippet.{slug}.{ordinal}.{hash:016x}")).map_err(|_| { - AgentLoopHostError::new( - AgentLoopHostErrorKind::Internal, - "instruction snippet reference could not be represented", - ) - }) -} - -fn sanitize_ref_suffix(value: &str) -> String { - let mut suffix = String::with_capacity(value.len().min(96)); - for character in value.chars() { - if character.is_ascii_alphanumeric() || matches!(character, '_' | '-' | '.') { - suffix.push(character); - } else { - suffix.push('.'); - } - if suffix.len() >= 96 { - break; - } - } - let suffix = suffix.trim_matches('.'); - if suffix.is_empty() { - "context".to_string() - } else { - suffix.to_string() - } -} - -fn stable_snippet_ref_hash(snippet_ref: &str, safe_summary: &str, ordinal: usize) -> u64 { - let mut hash = FNV_OFFSET; - feed_hash(&mut hash, snippet_ref.as_bytes()); - feed_hash(&mut hash, &[0xFF]); - feed_hash(&mut hash, safe_summary.as_bytes()); - feed_hash(&mut hash, &[0xFF]); - feed_hash(&mut hash, ordinal.to_string().as_bytes()); - hash -} - -const FNV_OFFSET: u64 = 0xcbf29ce484222325; -const FNV_PRIME: u64 = 0x00000100000001B3; - -fn feed_hash(hash: &mut u64, bytes: &[u8]) { - for &byte in bytes { - *hash ^= u64::from(byte); - *hash = hash.wrapping_mul(FNV_PRIME); +fn context_message_to_model_message(message: LoopContextMessage) -> LoopModelMessage { + LoopModelMessage { + role: message.role, + content_ref: message.message_ref, } } diff --git a/crates/ironclaw_turns/src/run_profile/skill_context.rs b/crates/ironclaw_turns/src/run_profile/skill_context.rs index 7f86ce86069..429059ad539 100644 --- a/crates/ironclaw_turns/src/run_profile/skill_context.rs +++ b/crates/ironclaw_turns/src/run_profile/skill_context.rs @@ -34,7 +34,11 @@ use async_trait::async_trait; use serde::{Deserialize, Serialize}; use thiserror::Error; -use super::LoopContextSnippet; +use crate::LoopMessageRef; + +use super::{ + AgentLoopHostError, AgentLoopHostErrorKind, LoopContextSnippet, LoopContextSnippetMetadata, +}; // --------------------------------------------------------------------------- // Errors @@ -100,6 +104,15 @@ pub enum SkillTrustLevel { Trusted, } +impl SkillTrustLevel { + pub const fn as_str(self) -> &'static str { + match self { + Self::Installed => "installed", + Self::Trusted => "trusted", + } + } +} + // --------------------------------------------------------------------------- // Snapshot types and context budgets // --------------------------------------------------------------------------- @@ -107,6 +120,8 @@ pub enum SkillTrustLevel { const EMPTY_SNAPSHOT_VERSION: &str = "empty"; const DEFAULT_MAX_SKILL_SNIPPET_BYTES: usize = 8 * 1024; const DEFAULT_MAX_SKILL_CONTEXT_BYTES: usize = 32 * 1024; +const FNV_OFFSET: u64 = 0xcbf29ce484222325; +const FNV_PRIME: u64 = 0x00000100000001B3; /// Byte budgets for model-visible skill context produced by [`SkillContextService`]. /// @@ -211,6 +226,10 @@ pub struct SkillContextSnippet { pub snippet_ref: String, /// Sanitized summary containing only the safe description and optionally prompt content. pub safe_summary: String, + /// Model-visible skill name used for telemetry, never for authority decisions. + pub skill_name: String, + /// Host-approved trust tier used for telemetry and downstream attenuation checks. + pub trust: SkillTrustLevel, } impl SkillContextSnippet { @@ -219,6 +238,10 @@ impl SkillContextSnippet { LoopContextSnippet { snippet_ref: self.snippet_ref, safe_summary: self.safe_summary, + metadata: Some(LoopContextSnippetMetadata { + source_name: self.skill_name, + trust_level: self.trust.as_str().to_string(), + }), } } } @@ -327,6 +350,8 @@ impl SkillContextSource for SkillContextService { snippets.push(SkillContextSnippet { snippet_ref, safe_summary, + skill_name: entry.name.clone(), + trust: entry.trust, }); } @@ -357,6 +382,66 @@ impl SkillContextSource for NoopSkillContextSource { // Helpers // --------------------------------------------------------------------------- +/// Build the model-message ref for a skill snippet. +/// +/// Prompt construction and model-message resolution both use this exact helper +/// so source/ordering drift fails closed instead of producing mismatched refs. +pub fn skill_snippet_model_message_ref( + snippet_ref: &str, + safe_summary: &str, + ordinal: usize, +) -> Result { + let slug = sanitize_ref_suffix(snippet_ref); + let hash = stable_snippet_ref_hash(snippet_ref, safe_summary, ordinal); + LoopMessageRef::new(format!("msg:snippet.{slug}.{ordinal}.{hash:016x}")).map_err(|_| { + AgentLoopHostError::new( + AgentLoopHostErrorKind::Internal, + "skill context snippet reference could not be represented", + ) + }) +} + +pub fn is_skill_snippet_model_message_ref(content_ref: &LoopMessageRef) -> bool { + content_ref.as_str().starts_with("msg:snippet.") +} + +fn sanitize_ref_suffix(value: &str) -> String { + let mut suffix = String::with_capacity(value.len().min(96)); + for character in value.chars() { + if character.is_ascii_alphanumeric() || matches!(character, '_' | '-' | '.') { + suffix.push(character); + } else { + suffix.push('.'); + } + if suffix.len() >= 96 { + break; + } + } + let suffix = suffix.trim_matches('.'); + if suffix.is_empty() { + "context".to_string() + } else { + suffix.to_string() + } +} + +fn stable_snippet_ref_hash(snippet_ref: &str, safe_summary: &str, ordinal: usize) -> u64 { + let mut hash = FNV_OFFSET; + feed_hash(&mut hash, snippet_ref.as_bytes()); + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, safe_summary.as_bytes()); + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, ordinal.to_string().as_bytes()); + hash +} + +fn feed_hash(hash: &mut u64, bytes: &[u8]) { + for &byte in bytes { + *hash ^= u64::from(byte); + *hash = hash.wrapping_mul(FNV_PRIME); + } +} + fn validate_snapshot(snapshot: &SkillRunSnapshot) -> Result<(), SkillContextError> { if snapshot.snapshot_version.is_empty() { return Err(SkillContextError::TrustDataMissing); @@ -422,40 +507,36 @@ const fn visibility_rank(visibility: SkillVisibility) -> u8 { /// and should not be used for security purposes. fn compute_snapshot_version(sorted_entries: &[InstalledSkillSnapshot]) -> String { // FNV-1a 64-bit — stable, simple, no external dependency. - const FNV_OFFSET: u64 = 0xcbf29ce484222325; - const FNV_PRIME: u64 = 0x00000100000001B3; - let mut hash = FNV_OFFSET; - let mut feed = |bytes: &[u8]| { - for &b in bytes { - hash ^= u64::from(b); - hash = hash.wrapping_mul(FNV_PRIME); - } - }; - for entry in sorted_entries { - feed(entry.name.as_bytes()); - feed(&[0xFF]); // separator - feed(match entry.trust { - SkillTrustLevel::Installed => b"installed", - SkillTrustLevel::Trusted => b"trusted", - }); - feed(&[0xFF]); - feed(match entry.visibility { - SkillVisibility::Visible => b"visible", - SkillVisibility::Hidden => b"hidden", - SkillVisibility::Denied => b"denied", - }); - feed(&[0xFF]); + feed_hash(&mut hash, entry.name.as_bytes()); + feed_hash(&mut hash, &[0xFF]); // separator + feed_hash( + &mut hash, + match entry.trust { + SkillTrustLevel::Installed => b"installed", + SkillTrustLevel::Trusted => b"trusted", + }, + ); + feed_hash(&mut hash, &[0xFF]); + feed_hash( + &mut hash, + match entry.visibility { + SkillVisibility::Visible => b"visible", + SkillVisibility::Hidden => b"hidden", + SkillVisibility::Denied => b"denied", + }, + ); + feed_hash(&mut hash, &[0xFF]); if let Some(ref content) = entry.prompt_content { - feed(content.as_bytes()); + feed_hash(&mut hash, content.as_bytes()); } - feed(&[0xFF]); - feed(entry.safe_description.as_bytes()); - feed(&[0xFF]); - feed(entry.ordering_key.as_bytes()); - feed(&[0xFE]); // entry separator + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, entry.safe_description.as_bytes()); + feed_hash(&mut hash, &[0xFF]); + feed_hash(&mut hash, entry.ordering_key.as_bytes()); + feed_hash(&mut hash, &[0xFE]); // entry separator } format!("v1:{hash:016x}") diff --git a/crates/ironclaw_turns/tests/agent_loop_host_contract.rs b/crates/ironclaw_turns/tests/agent_loop_host_contract.rs index afb95e15abd..a005e0d88fe 100644 --- a/crates/ironclaw_turns/tests/agent_loop_host_contract.rs +++ b/crates/ironclaw_turns/tests/agent_loop_host_contract.rs @@ -21,14 +21,15 @@ use ironclaw_turns::{ FinalizeAssistantMessage, HostManagedLoopModelPort, HostManagedLoopPromptPort, InMemoryLoopHostMilestoneSink, LoopCapabilityPort, LoopCheckpointKind, LoopCheckpointPort, LoopCheckpointRequest, LoopCheckpointStateRef, LoopContextBundle, LoopContextMessage, - LoopContextPort, LoopContextRequest, LoopContextSnippet, LoopDriverId, LoopDriverNoteKind, - LoopHostMilestone, LoopHostMilestoneEmitter, LoopHostMilestoneKind, LoopHostMilestoneSink, - LoopInputBatch, LoopInputCursor, LoopInputCursorToken, LoopInputPort, LoopModelGateway, - LoopModelGatewayError, LoopModelGatewayRequest, LoopModelMessage, LoopModelPort, - LoopModelRequest, LoopModelResponse, LoopProgressEvent, LoopProgressPort, LoopPromptBundle, + LoopContextPort, LoopContextRequest, LoopContextSnippet, LoopContextSnippetMetadata, + LoopDriverId, LoopDriverNoteKind, LoopHostMilestone, LoopHostMilestoneEmitter, + LoopHostMilestoneKind, LoopHostMilestoneSink, LoopInputBatch, LoopInputCursor, + LoopInputCursorToken, LoopInputPort, LoopModelGateway, LoopModelGatewayError, + LoopModelGatewayRequest, LoopModelMessage, LoopModelPort, LoopModelRequest, + LoopModelResponse, LoopProgressEvent, LoopProgressPort, LoopPromptBundle, LoopPromptBundleRef, LoopPromptBundleRequest, LoopPromptPort, LoopRunContext, LoopRunInfoPort, LoopTranscriptPort, ParentLoopOutput, PromptMode, - VisibleCapabilityRequest, VisibleCapabilitySurface, + PromptSkillContextMetadata, VisibleCapabilityRequest, VisibleCapabilitySurface, }, runner::{ClaimRunRequest, TurnRunTransitionPort}, }; @@ -303,6 +304,121 @@ async fn loop_prompt_port_materializes_instruction_snippets_as_system_refs() { assert_eq!(host.milestone_kind_names(), vec!["prompt_bundle_built"]); } +#[tokio::test] +async fn loop_prompt_port_preserves_mid_conversation_system_message_order() { + let host = Arc::new( + RecordingAgentLoopHost::new(claimed_run_context().await) + .with_context_instruction_snippet("skill:alpha", "alpha skill context available") + .with_context_tail_message("system", "msg:summary", "summary context available"), + ); + let port = HostManagedLoopPromptPort::new( + host.context.clone(), + host.clone(), + host.milestone_sink.clone(), + ); + + let bundle = port + .build_prompt_bundle(LoopPromptBundleRequest { + mode: PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: Some(8), + }) + .await + .unwrap(); + + assert_eq!(bundle.messages.len(), 3); + assert_eq!(bundle.messages[0].role, "system"); + assert!( + bundle.messages[0] + .content_ref + .as_str() + .starts_with("msg:snippet.skill.alpha.") + ); + assert_eq!(bundle.messages[1].role, "user"); + assert_eq!( + bundle.messages[1].content_ref, + LoopMessageRef::new("msg:user-message").unwrap() + ); + assert_eq!(bundle.messages[2].role, "system"); + assert_eq!( + bundle.messages[2].content_ref, + LoopMessageRef::new("msg:summary").unwrap() + ); +} + +#[tokio::test] +async fn loop_prompt_port_keeps_identity_before_skill_snippets_and_records_skill_metadata() { + let host = Arc::new( + RecordingAgentLoopHost::new(claimed_run_context().await) + .with_context_system_message("msg:identity", "identity context available") + .with_context_instruction_snippet("skill:alpha", "alpha skill context available"), + ); + let port = HostManagedLoopPromptPort::new( + host.context.clone(), + host.clone(), + host.milestone_sink.clone(), + ); + + let bundle = port + .build_prompt_bundle(LoopPromptBundleRequest { + mode: PromptMode::TextOnly, + context_cursor: None, + surface_version: None, + checkpoint_state_ref: None, + max_messages: Some(8), + }) + .await + .unwrap(); + + assert_eq!(bundle.messages.len(), 3); + assert_eq!(bundle.messages[0].role, "system"); + assert_eq!( + bundle.messages[0].content_ref, + LoopMessageRef::new("msg:identity").unwrap() + ); + assert_eq!(bundle.messages[1].role, "system"); + assert!( + bundle.messages[1] + .content_ref + .as_str() + .starts_with("msg:snippet.skill.alpha.") + ); + assert_eq!(bundle.messages[2].role, "user"); + + let milestones = host.milestones(); + assert!(matches!( + &milestones[0].kind, + LoopHostMilestoneKind::PromptBundleBuilt { skill_context, .. } + if skill_context.as_slice() == [PromptSkillContextMetadata { + ordinal: 0, + source_name: "alpha".to_string(), + trust_level: "trusted".to_string(), + }] + )); +} + +#[test] +fn prompt_bundle_built_deserializes_legacy_without_skill_context_metadata() { + let legacy = serde_json::json!({ + "prompt_bundle_built": { + "bundle_ref": "prompt:00000000-0000-0000-0000-000000000001:legacy", + "mode": "text_only", + "surface_version": null, + "message_count": 1 + } + }); + + let kind: LoopHostMilestoneKind = serde_json::from_value(legacy).unwrap(); + + assert!(matches!( + kind, + LoopHostMilestoneKind::PromptBundleBuilt { skill_context, .. } + if skill_context.is_empty() + )); +} + #[tokio::test] async fn loop_prompt_port_rejects_unsupported_prompt_mode() { let host = Arc::new(RecordingAgentLoopHost::new(codeact_run_context().await)); @@ -1086,6 +1202,8 @@ struct RecordingAgentLoopHost { visible_surface: VisibleCapabilitySurface, milestone_sink: Arc, context_message_safe_summary: String, + context_system_messages: Vec, + context_tail_messages: Vec, context_instruction_snippets: Vec, context_memory_snippets: Vec, } @@ -1110,6 +1228,8 @@ impl RecordingAgentLoopHost { }], }, context_message_safe_summary: "hello".to_string(), + context_system_messages: Vec::new(), + context_tail_messages: Vec::new(), context_instruction_snippets: Vec::new(), context_memory_snippets: Vec::new(), } @@ -1120,14 +1240,50 @@ impl RecordingAgentLoopHost { self } + fn with_context_system_message( + mut self, + message_ref: impl Into, + safe_summary: impl Into, + ) -> Self { + self.context_system_messages.push(LoopContextMessage { + message_ref: LoopMessageRef::new(message_ref.into()).unwrap(), + role: "system".to_string(), + safe_summary: safe_summary.into(), + }); + self + } + + fn with_context_tail_message( + mut self, + role: impl Into, + message_ref: impl Into, + safe_summary: impl Into, + ) -> Self { + self.context_tail_messages.push(LoopContextMessage { + message_ref: LoopMessageRef::new(message_ref.into()).unwrap(), + role: role.into(), + safe_summary: safe_summary.into(), + }); + self + } + fn with_context_instruction_snippet( mut self, snippet_ref: impl Into, safe_summary: impl Into, ) -> Self { + let snippet_ref = snippet_ref.into(); + let metadata = + snippet_ref + .strip_prefix("skill:") + .map(|source_name| LoopContextSnippetMetadata { + source_name: source_name.to_string(), + trust_level: "trusted".to_string(), + }); self.context_instruction_snippets.push(LoopContextSnippet { - snippet_ref: snippet_ref.into(), + snippet_ref, safe_summary: safe_summary.into(), + metadata, }); self } @@ -1140,6 +1296,7 @@ impl RecordingAgentLoopHost { self.context_memory_snippets.push(LoopContextSnippet { snippet_ref: snippet_ref.into(), safe_summary: safe_summary.into(), + metadata: None, }); self } @@ -1201,12 +1358,15 @@ impl LoopContextPort for RecordingAgentLoopHost { ) -> Result { self.context_requests.lock().unwrap().push(request); self.record("context"); + let mut messages = vec![LoopContextMessage { + message_ref: LoopMessageRef::new("msg:user-message").unwrap(), + role: "user".to_string(), + safe_summary: self.context_message_safe_summary.clone(), + }]; + messages.extend(self.context_tail_messages.clone()); Ok(LoopContextBundle { - messages: vec![LoopContextMessage { - message_ref: LoopMessageRef::new("msg:user-message").unwrap(), - role: "user".to_string(), - safe_summary: self.context_message_safe_summary.clone(), - }], + identity_messages: self.context_system_messages.clone(), + messages, instruction_snippets: self.context_instruction_snippets.clone(), memory_snippets: self.context_memory_snippets.clone(), }) @@ -1267,6 +1427,7 @@ impl LoopPromptPort for RecordingAgentLoopHost { request.mode, bundle.surface_version.clone(), bundle.messages.len(), + Vec::new(), ) .await?; self.record("milestone:prompt_bundle_built");