diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 71a3c52411..84555cdbf3 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3321,6 +3321,14 @@ dependencies = [ "tracing", ] +[[package]] +name = "codex-guardian-context" +version = "0.0.0" +dependencies = [ + "codex-protocol", + "pretty_assertions", +] + [[package]] name = "codex-guardian-v2" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index f992ac7814..ebdee5fce9 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -44,6 +44,7 @@ members = [ "core-api", "core-plugins", "diagnostics", + "guardian-context", "hooks", "history", "http-client", diff --git a/codex-rs/guardian-context/BUILD.bazel b/codex-rs/guardian-context/BUILD.bazel new file mode 100644 index 0000000000..33ce23f0c4 --- /dev/null +++ b/codex-rs/guardian-context/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "guardian-context", + crate_name = "codex_guardian_context", +) diff --git a/codex-rs/guardian-context/Cargo.toml b/codex-rs/guardian-context/Cargo.toml new file mode 100644 index 0000000000..97ebab48f6 --- /dev/null +++ b/codex-rs/guardian-context/Cargo.toml @@ -0,0 +1,19 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-guardian-context" +version.workspace = true + +[lib] +doctest = false +name = "codex_guardian_context" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-protocol = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } diff --git a/codex-rs/guardian-context/src/entry.rs b/codex-rs/guardian-context/src/entry.rs new file mode 100644 index 0000000000..02ed8d5dc7 --- /dev/null +++ b/codex-rs/guardian-context/src/entry.rs @@ -0,0 +1,52 @@ +//! Structured transcript evidence shared by synchronous and asynchronous Guardian. +//! +//! Entry kinds preserve source attribution for consumer-specific retention and +//! rendering. Text is bounded during collection, with its original size retained +//! for truncation accounting. + +/// Semantic role of one parent-conversation transcript entry. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum ConversationTranscriptEntryKind { + /// Actual parent-thread user message. + User, + /// Explicit developer message preserving a user-approved action. + Developer, + /// Assistant commentary or an inter-agent message. + Assistant, + /// Final assistant answer eligible for stronger async retention. + ProtectedAssistant, + /// Named tool invocation, including shell and web-search calls. + ToolCall(String), + /// Named or unnamed tool result. + ToolOutput(String), + /// Result from a Node REPL-backed tool that may receive a larger sync cap. + NodeReplToolOutput(String), + /// Plaintext model reasoning that the async consumer explicitly enables. + Reasoning, +} + +impl ConversationTranscriptEntryKind { + /// Returns the role label currently used in Guardian transcript prompts. + pub fn role(&self) -> &str { + match self { + Self::User => "user", + Self::Developer => "developer", + Self::Assistant | Self::ProtectedAssistant => "assistant", + Self::ToolCall(role) | Self::ToolOutput(role) | Self::NodeReplToolOutput(role) => { + role.as_str() + } + Self::Reasoning => "reasoning", + } + } +} + +/// Structured text evidence shared by sync Guardian and async scoring. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ConversationTranscriptEntry { + /// Semantic role used for consumer-specific retention and truncation. + pub kind: ConversationTranscriptEntryKind, + /// Text bounded by the current request's per-entry limits. + pub text: String, + /// Size before truncation, retained for omission and truncation accounting. + pub original_bytes: usize, +} diff --git a/codex-rs/guardian-context/src/lib.rs b/codex-rs/guardian-context/src/lib.rs new file mode 100644 index 0000000000..78a8ff026f --- /dev/null +++ b/codex-rs/guardian-context/src/lib.rs @@ -0,0 +1,131 @@ +//! Shared context sections for synchronous Guardian review and asynchronous scoring. +//! +//! Contributor failures abort collection without returning partial context. +//! Sections carry structured transcript evidence without depending on either +//! consumer's rendering, retention, compaction, or request lifecycle. +//! Registered contributors declare their scope once and are collected only for +//! matching context consumers. + +use std::sync::Arc; + +use codex_protocol::models::ResponseItem; + +pub use entry::ConversationTranscriptEntry; +pub use entry::ConversationTranscriptEntryKind; +pub use truncation::truncate_text; + +mod entry; +mod truncation; + +/// Consumer for which a Guardian context is composed. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ContextTarget { + /// The reusable synchronous Guardian reviewer. + Sync, + /// The asynchronous Guardian action scorer. + Async, +} + +/// Consumers to which a context section contributes. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum SectionScope { + /// Include the section in both synchronous review and asynchronous scoring. + Shared, + /// Include the section only in synchronous review. + SyncOnly, + /// Include the section only in asynchronous scoring. + AsyncOnly, +} + +impl SectionScope { + /// Whether this section is included for the requested context consumer. + pub fn includes(self, target: ContextTarget) -> bool { + match self { + Self::Shared => true, + Self::SyncOnly => matches!(target, ContextTarget::Sync), + Self::AsyncOnly => matches!(target, ContextTarget::Async), + } + } +} + +/// Borrowed host inputs available while one Guardian context section is built. +#[derive(Clone, Copy, Debug)] +pub struct SectionInput<'a> { + /// Consumer for which the host is collecting context sections. + pub target: ContextTarget, + /// Parent conversation history available to this contribution. + pub history: &'a [ResponseItem], +} + +/// Supplies one independently scoped section to Guardian context assembly. +/// +/// Implementations declare whether they apply to synchronous review, +/// asynchronous scoring, or both. The registry filters contributors by scope +/// before invoking them. Contributors distinguish sections that do not apply +/// from required evidence that could not be collected. +pub trait SectionContributor: Send + Sync { + /// Guardian consumers that should receive this contribution. + fn scope(&self) -> SectionScope; + + /// Builds this section using the host's current conversation snapshot. + /// + /// Return `Ok(None)` only when this section is optional or does not apply. + /// Missing required evidence must return `Err`; callers must not review a + /// partial context as though collection succeeded. + fn contribute(&self, input: &SectionInput<'_>) -> Result, SectionError>; +} + +/// A section could not provide the evidence needed for a valid review context. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum SectionError { + /// Evidence required by this contributor for the current input is missing. + MissingRequiredEvidence { section: &'static str }, +} + +impl std::fmt::Display for SectionError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::MissingRequiredEvidence { section } => { + write!(formatter, "missing required evidence for section {section}") + } + } + } +} + +impl std::error::Error for SectionError {} + +/// Ordered collection of independently scoped Guardian section contributors. +#[derive(Clone, Default)] +pub struct SectionRegistry { + contributors: Vec>, +} + +impl SectionRegistry { + /// Adds a contributor to the end of the section collection order. + pub fn register(&mut self, contributor: impl SectionContributor + 'static) { + self.contributors.push(Arc::new(contributor)); + } + + /// Collects applicable sections in their original registration order. + /// + /// Stops at the first error without returning any partial context. The host + /// decides whether to fall back to synchronous review or deny approval. + pub fn collect(&self, input: &SectionInput<'_>) -> Result, SectionError> { + self.contributors + .iter() + .filter(|contributor| contributor.scope().includes(input.target)) + .filter_map(|contributor| contributor.contribute(input).transpose()) + .collect() + } +} + +/// Ordered transcript evidence produced by one section contributor. +#[derive(Clone, Debug, PartialEq)] +pub struct ContextSection { + /// Structured evidence before consumer-specific selection and rendering. + pub items: Vec, +} + +#[cfg(test)] +#[path = "registry_tests.rs"] +mod registry_tests; diff --git a/codex-rs/guardian-context/src/registry_tests.rs b/codex-rs/guardian-context/src/registry_tests.rs new file mode 100644 index 0000000000..c6523fa03d --- /dev/null +++ b/codex-rs/guardian-context/src/registry_tests.rs @@ -0,0 +1,142 @@ +use std::sync::Arc; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; + +use codex_protocol::models::ResponseItem; +use pretty_assertions::assert_eq; + +use super::ContextSection; +use super::ContextTarget; +use super::ConversationTranscriptEntry; +use super::ConversationTranscriptEntryKind; +use super::SectionContributor; +use super::SectionError; +use super::SectionInput; +use super::SectionRegistry; +use super::SectionScope; + +struct TestContributor { + outcome: Result, SectionError>, + scope: SectionScope, + invocations: Arc, +} + +impl SectionContributor for TestContributor { + fn scope(&self) -> SectionScope { + self.scope + } + + fn contribute(&self, input: &SectionInput<'_>) -> Result, SectionError> { + self.invocations.fetch_add(/*val*/ 1, Ordering::Relaxed); + let history_len = input.history.len(); + Ok(self + .outcome + .clone()? + .map(|label| section(label, history_len))) + } +} + +fn section(label: &str, history_len: usize) -> ContextSection { + let text = format!("{label}: history items: {history_len}"); + ContextSection { + items: vec![ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::User, + original_bytes: text.len(), + text, + }], + } +} + +#[test] +fn registry_collects_target_specific_sections_in_registration_order() { + let mut registry = SectionRegistry::default(); + let mut invocations = Vec::new(); + for (label, scope) in [ + ("root", SectionScope::Shared), + ("permissions", SectionScope::SyncOnly), + ("reviews", SectionScope::AsyncOnly), + ("action", SectionScope::Shared), + ] { + let calls = Arc::new(AtomicUsize::new(/*v*/ 0)); + registry.register(TestContributor { + outcome: Ok(Some(label)), + scope, + invocations: Arc::clone(&calls), + }); + invocations.push(calls); + } + let history = [ResponseItem::Other]; + + let sync_sections = registry.collect(&SectionInput { + target: ContextTarget::Sync, + history: &history, + }); + let async_sections = registry.collect(&SectionInput { + target: ContextTarget::Async, + history: &history, + }); + + assert_eq!( + sync_sections, + Ok(vec![ + section("root", /*history_len*/ 1), + section("permissions", /*history_len*/ 1), + section("action", /*history_len*/ 1), + ]) + ); + assert_eq!( + async_sections, + Ok(vec![ + section("root", /*history_len*/ 1), + section("reviews", /*history_len*/ 1), + section("action", /*history_len*/ 1), + ]) + ); + assert_eq!( + invocations + .iter() + .map(|calls| calls.load(Ordering::Relaxed)) + .collect::>(), + vec![2, 1, 1, 2] + ); +} + +#[test] +fn registry_skips_optional_sections_and_stops_on_missing_required_evidence() { + let error = SectionError::MissingRequiredEvidence { + section: "permissions", + }; + for target in [ContextTarget::Sync, ContextTarget::Async] { + let mut registry = SectionRegistry::default(); + let mut invocations = Vec::new(); + for outcome in [ + Ok(Some("root")), + Ok(None), + Err(error.clone()), + Ok(Some("action")), + ] { + let calls = Arc::new(AtomicUsize::new(/*v*/ 0)); + registry.register(TestContributor { + outcome, + scope: SectionScope::Shared, + invocations: Arc::clone(&calls), + }); + invocations.push(calls); + } + + assert_eq!( + registry.collect(&SectionInput { + target, + history: &[ResponseItem::Other], + }), + Err(error.clone()) + ); + assert_eq!( + invocations + .iter() + .map(|calls| calls.load(Ordering::Relaxed)) + .collect::>(), + vec![1, 1, 1, 0] + ); + } +} diff --git a/codex-rs/guardian-context/src/truncation.rs b/codex-rs/guardian-context/src/truncation.rs new file mode 100644 index 0000000000..752f84523a --- /dev/null +++ b/codex-rs/guardian-context/src/truncation.rs @@ -0,0 +1,42 @@ +//! Guardian's shared UTF-8-safe, prefix/suffix text truncation primitive. +//! +//! The existing XML omission marker is preserved, including returning the whole +//! marker when a token budget is too small to contain it. + +use codex_protocol::protocol::TruncationPolicy; + +/// Truncates text using Guardian's approximate token budget and omission marker. +/// +/// Retains both ends on UTF-8 boundaries. Budgets smaller than the marker still +/// return the marker, so callers should reserve room for that fixed overhead. +pub fn truncate_text(text: &str, max_tokens: usize) -> String { + let max_bytes = TruncationPolicy::Tokens(max_tokens).byte_budget(); + if text.len() <= max_bytes { + return text.to_owned(); + } + + let omitted_tokens = + TruncationPolicy::Bytes(text.len().saturating_sub(max_bytes)).token_budget(); + let marker = format!(""); + if max_bytes <= marker.len() { + return marker; + } + + let available_bytes = max_bytes - marker.len(); + let prefix_bytes = available_bytes / 2; + let suffix_bytes = available_bytes - prefix_bytes; + let mut prefix_end = prefix_bytes; + while !text.is_char_boundary(prefix_end) { + prefix_end -= 1; + } + let mut suffix_start = text.len() - suffix_bytes; + while !text.is_char_boundary(suffix_start) { + suffix_start += 1; + } + + format!("{}{marker}{}", &text[..prefix_end], &text[suffix_start..]) +} + +#[cfg(test)] +#[path = "truncation_tests.rs"] +mod tests; diff --git a/codex-rs/guardian-context/src/truncation_tests.rs b/codex-rs/guardian-context/src/truncation_tests.rs new file mode 100644 index 0000000000..a49e93f42e --- /dev/null +++ b/codex-rs/guardian-context/src/truncation_tests.rs @@ -0,0 +1,29 @@ +//! Regression coverage for Guardian's truncation marker and UTF-8 boundaries. + +use codex_protocol::protocol::TruncationPolicy; +use pretty_assertions::assert_eq; + +use super::truncate_text; + +#[test] +fn truncation_preserves_prefix_suffix_and_utf8_boundaries() { + let text = format!("start {} end", "é🙂".repeat(/*n*/ 2_000)); + let truncated = truncate_text(&text, /*max_tokens*/ 200); + let omitted_tokens = TruncationPolicy::Bytes(text.len() - 800).token_budget(); + let marker = format!(""); + assert!(truncated.starts_with("start ")); + assert!(truncated.ends_with(" end")); + assert!(truncated.contains(&marker)); + assert!(truncated.len() <= 800); + + assert_eq!(truncate_text("é🙂", /*max_tokens*/ 2), "é🙂"); + assert_eq!(truncate_text("", /*max_tokens*/ 0), ""); + assert_eq!( + truncate_text("é🙂", /*max_tokens*/ 0), + "" + ); + assert_eq!( + truncate_text("é🙂", /*max_tokens*/ 1), + "" + ); +}