mirror of
https://github.com/openai/codex.git
synced 2026-11-09 00:39:32 +00:00
Move trusted skill evidence into the Guardian context registry (#43599)
## What changed Move trusted skill rendering into `codex-guardian-context` as `TrustedSkills` and collect it through an async-only context section. Pass the collected section to the sampler, preserving the separate developer message and bounded JSON rendering. Omit skill paths from `Debug` output, exposing only their count. ## Testing Move rendering tests alongside the shared type, covering multiple paths and escaped UTF-8 within the size budget. Extend registry tests to cover async-only inclusion, omission for empty input, section ordering, and path redaction in debug output. GitOrigin-RevId: c0778067f19d31e693ca954aee1a7d7be8f9739a
This commit is contained in:
1 parent
2554239561
commit
f5331dc237
14 files changed
+222
-129
No files matched your search
@@ -51,7 +51,9 @@ mod authorization;
|
||||
mod entry;
|
||||
mod history;
|
||||
mod permissions;
|
||||
mod trusted_skills;
|
||||
mod trusted_tool;
|
||||
pub use trusted_skills::TrustedSkills;
|
||||
pub use trusted_tool::TrustedTool;
|
||||
mod reviews;
|
||||
pub use reviews::MAX_PREVIOUS_REVIEWS;
|
||||
@@ -118,6 +120,8 @@ pub struct SectionInput<'a> {
|
||||
pub previous_reviews: Option<&'a PreviousReviews>,
|
||||
/// Metadata verified by the host for the exact action being classified.
|
||||
pub trusted_tool: Option<&'a TrustedTool>,
|
||||
/// Current-turn and delegated skill paths verified and bounded by the host.
|
||||
pub trusted_skill_paths: &'a [String],
|
||||
}
|
||||
|
||||
/// Supplies repeatable, zero-copy access to a host-owned conversation snapshot.
|
||||
@@ -207,6 +211,7 @@ pub fn default_registry() -> &'static SectionRegistry {
|
||||
let mut registry = SectionRegistry::default();
|
||||
registry.register(reviews::PreviousReviewsSection);
|
||||
registry.register(trusted_tool::TrustedToolSection);
|
||||
registry.register(trusted_skills::TrustedSkillsSection);
|
||||
registry.register(RootConversationSection);
|
||||
registry.register(RetainedUserInstructionsSection);
|
||||
registry.register(TrustedUserAnswersSection);
|
||||
|
||||
@@ -92,6 +92,7 @@ fn registry_collects_target_specific_sections_in_registration_order() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
});
|
||||
let async_sections = registry.collect(&SectionInput {
|
||||
target: ContextTarget::Async,
|
||||
@@ -103,6 +104,7 @@ fn registry_collects_target_specific_sections_in_registration_order() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
@@ -165,6 +167,7 @@ fn registry_skips_optional_sections_and_stops_on_missing_required_evidence() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
}),
|
||||
Err(error.clone())
|
||||
);
|
||||
@@ -228,6 +231,7 @@ fn reused_registry_preserves_section_identity_and_source_roles() {
|
||||
permissions: Some(&permissions),
|
||||
previous_reviews: Some(&reviews),
|
||||
trusted_tool: Some(&tool),
|
||||
trusted_skill_paths: &["debug-secret/SKILL.md".into()],
|
||||
})
|
||||
.unwrap();
|
||||
assert!(!format!("{context:?}").contains("debug-secret"));
|
||||
@@ -260,6 +264,12 @@ fn reused_registry_preserves_section_identity_and_source_roles() {
|
||||
] });
|
||||
}
|
||||
if target == ContextTarget::Async {
|
||||
expected.insert(
|
||||
0,
|
||||
ContextSection::TrustedSkills(super::TrustedSkills {
|
||||
paths: vec!["debug-secret/SKILL.md".into()],
|
||||
}),
|
||||
);
|
||||
expected.insert(0, ContextSection::TrustedTool(tool.clone()));
|
||||
expected.insert(0, ContextSection::PreviousReviews(reviews.clone()));
|
||||
}
|
||||
@@ -277,6 +287,7 @@ fn reused_registry_preserves_section_identity_and_source_roles() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
})
|
||||
.unwrap(),
|
||||
vec![ContextSection::ConversationTranscript { items: Vec::new() }]
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
use crate::ConversationTranscriptEntry;
|
||||
use crate::PlannedAction;
|
||||
use crate::PreviousReviews;
|
||||
use crate::TrustedSkills;
|
||||
use crate::TrustedTool;
|
||||
|
||||
/// Ordered evidence with a stable section identity and source-specific content.
|
||||
@@ -21,5 +22,6 @@ pub enum ContextSection<T = ConversationTranscriptEntry> {
|
||||
PlannedAction(PlannedAction),
|
||||
PreviousReviews(PreviousReviews),
|
||||
TrustedTool(TrustedTool),
|
||||
TrustedSkills(TrustedSkills),
|
||||
PermissionContext { items: Vec<String> },
|
||||
}
|
||||
@@ -102,6 +102,7 @@ fn registered_transcript_preserves_shared_roles_and_node_repl_tool_attribution()
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
})
|
||||
.expect("transcript collection should succeed");
|
||||
|
||||
@@ -178,6 +179,7 @@ fn excluded_tool_calls_still_attribute_included_results() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
})
|
||||
.expect("transcript collection should succeed");
|
||||
|
||||
@@ -273,6 +275,7 @@ fn outputs_with_call_ids_or_explicit_names_are_retained() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
})
|
||||
.expect("transcript collection should succeed");
|
||||
let mut expected = vec![
|
||||
@@ -322,6 +325,7 @@ fn reused_registry_applies_current_history_sources_and_entry_limits() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
})
|
||||
.expect("transcript collection should succeed");
|
||||
assert_eq!(
|
||||
@@ -358,6 +362,7 @@ fn reused_registry_applies_current_history_sources_and_entry_limits() {
|
||||
permissions: None,
|
||||
previous_reviews: None,
|
||||
trusted_tool: None,
|
||||
trusted_skill_paths: &[],
|
||||
})
|
||||
.expect("transcript collection should succeed");
|
||||
let mut expected = vec![ConversationTranscriptEntry {
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
//! Bounded rendering of host-verified invoked skill paths for async review.
|
||||
//! Ownership checks, turn storage and delegated evidence selection stay in the host.
|
||||
|
||||
use crate::ContextSection;
|
||||
use crate::SectionContributor;
|
||||
use crate::SectionError;
|
||||
use crate::SectionInput;
|
||||
use crate::SectionScope;
|
||||
use codex_context_fragments::ContextualUserFragment;
|
||||
use codex_protocol::models::ContentItemKind;
|
||||
use codex_protocol::protocol::TruncationPolicy;
|
||||
use serde_json::json;
|
||||
|
||||
const MAX_TRUSTED_SKILL_TOKENS: usize = 768;
|
||||
const TRUSTED_SKILLS_PREFIX: &str = "Codex-verified invoked user-owned skill paths:\n";
|
||||
|
||||
/// Host-verified user-owned skill paths for the current Guardian classification.
|
||||
#[derive(Clone, PartialEq, Eq)]
|
||||
pub struct TrustedSkills {
|
||||
pub paths: Vec<String>,
|
||||
}
|
||||
|
||||
impl ContextualUserFragment for TrustedSkills {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("guardian.trusted_skills".to_owned())
|
||||
}
|
||||
|
||||
fn role(&self) -> &'static str {
|
||||
"developer"
|
||||
}
|
||||
|
||||
fn requires_separate_message(&self) -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
fn markers(&self) -> (&'static str, &'static str) {
|
||||
Self::type_markers()
|
||||
}
|
||||
|
||||
fn type_markers() -> (&'static str, &'static str) {
|
||||
("", "")
|
||||
}
|
||||
|
||||
fn body(&self) -> String {
|
||||
let mut paths = String::from("[");
|
||||
let max_path_bytes = TruncationPolicy::Tokens(MAX_TRUSTED_SKILL_TOKENS)
|
||||
.byte_budget()
|
||||
.saturating_sub(TRUSTED_SKILLS_PREFIX.len());
|
||||
for path in &self.paths {
|
||||
let separator = if paths.ends_with('[') { "" } else { "," };
|
||||
let encoded_path = json!(path).to_string();
|
||||
if paths
|
||||
.len()
|
||||
.saturating_add(separator.len())
|
||||
.saturating_add(encoded_path.len())
|
||||
.saturating_add(1)
|
||||
> max_path_bytes
|
||||
{
|
||||
continue;
|
||||
}
|
||||
paths.push_str(separator);
|
||||
paths.push_str(&encoded_path);
|
||||
}
|
||||
paths.push(']');
|
||||
format!("{TRUSTED_SKILLS_PREFIX}{paths}")
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for TrustedSkills {
|
||||
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
formatter
|
||||
.debug_struct("TrustedSkills")
|
||||
.field("count", &self.paths.len())
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) struct TrustedSkillsSection;
|
||||
|
||||
impl SectionContributor for TrustedSkillsSection {
|
||||
fn scope(&self) -> SectionScope {
|
||||
SectionScope::AsyncOnly
|
||||
}
|
||||
fn contribute(&self, input: &SectionInput<'_>) -> Result<Option<ContextSection>, SectionError> {
|
||||
Ok((!input.trusted_skill_paths.is_empty()).then(|| {
|
||||
ContextSection::TrustedSkills(TrustedSkills {
|
||||
paths: input.trusted_skill_paths.to_vec(),
|
||||
})
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "trusted_skills_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,56 @@
|
||||
use super::MAX_TRUSTED_SKILL_TOKENS;
|
||||
use super::TRUSTED_SKILLS_PREFIX;
|
||||
use super::TrustedSkills;
|
||||
use codex_context_fragments::ContextualUserFragment;
|
||||
use codex_protocol::protocol::TruncationPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn rendered_paths(paths: Vec<String>) -> Vec<String> {
|
||||
let rendered = TrustedSkills { paths }.render();
|
||||
assert!(rendered.len() <= TruncationPolicy::Tokens(MAX_TRUSTED_SKILL_TOKENS).byte_budget());
|
||||
serde_json::from_str(
|
||||
rendered
|
||||
.strip_prefix(TRUSTED_SKILLS_PREFIX)
|
||||
.expect("trusted skill context should contain JSON evidence"),
|
||||
)
|
||||
.expect("trusted skill evidence should remain valid JSON")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_verified_skill_paths() {
|
||||
assert_eq!(
|
||||
rendered_paths(vec!["/home/user/.codex/skills/demo/SKILL.md".to_owned()]),
|
||||
vec!["/home/user/.codex/skills/demo/SKILL.md"],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bounds_escaped_skill_paths_without_corrupting_json_or_utf8() {
|
||||
let paths = (0..16)
|
||||
.map(|index| {
|
||||
format!(
|
||||
"/home/user/.codex/skills/{index:03}/{}SKILL.md",
|
||||
"\u{0001}é".repeat(80)
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let retained = rendered_paths(paths.clone());
|
||||
|
||||
assert!(!retained.is_empty());
|
||||
assert!(retained.len() < paths.len());
|
||||
assert!(retained.iter().all(|path| paths.contains(path)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_multiple_invoked_skill_paths() {
|
||||
assert_eq!(
|
||||
rendered_paths(vec![
|
||||
"/home/user/.codex/skills/first/SKILL.md".to_owned(),
|
||||
"/home/user/.codex/skills/second/SKILL.md".to_owned(),
|
||||
]),
|
||||
vec![
|
||||
"/home/user/.codex/skills/first/SKILL.md",
|
||||
"/home/user/.codex/skills/second/SKILL.md",
|
||||
],
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user