mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Add structured full approval reviews to extensions (#40472)
## What changed - Split the approval review contributor into a `fast_decision` path for existing evidence and a `full_review` path for structured reviews. - Add host-owned full-review inputs with action evidence, conversation history, thread and turn identifiers, approval context, and extension data. - Add structured assessment and error types, and resolve full reviews using the first contributor that claims the request. ## Testing - Cover full-review input propagation, first-claim ordering, and short-circuit behavior in the extension registry tests. GitOrigin-RevId: 6f763d874af8c0b66a0d760b7b7c7dfe32251c9e
This commit is contained in:
1 parent
bb8cada846
commit
333beecd41
11 files changed
+261
-43
No files matched your search
@@ -347,7 +347,7 @@ async fn run_guardian_review(
|
|||||||
&& let Some(decision) = session
|
&& let Some(decision) = session
|
||||||
.services
|
.services
|
||||||
.extensions
|
.extensions
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session.services.session_extension_data,
|
&session.services.session_extension_data,
|
||||||
&session.services.thread_extension_data,
|
&session.services.thread_extension_data,
|
||||||
&action.to_string(),
|
&action.to_string(),
|
||||||
|
|||||||
@@ -3095,7 +3095,7 @@ async fn escalated_retry_bypasses_extension_approval_and_runs_guardian() -> anyh
|
|||||||
struct AutoApprovingReviewContributor;
|
struct AutoApprovingReviewContributor;
|
||||||
|
|
||||||
impl codex_extension_api::ApprovalReviewContributor for AutoApprovingReviewContributor {
|
impl codex_extension_api::ApprovalReviewContributor for AutoApprovingReviewContributor {
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
_session_store: &'a codex_extension_api::ExtensionData,
|
_session_store: &'a codex_extension_api::ExtensionData,
|
||||||
_thread_store: &'a codex_extension_api::ExtensionData,
|
_thread_store: &'a codex_extension_api::ExtensionData,
|
||||||
|
|||||||
@@ -65,7 +65,7 @@ use wiremock::matchers::path;
|
|||||||
struct ApprovedReviewContributor;
|
struct ApprovedReviewContributor;
|
||||||
|
|
||||||
impl ApprovalReviewContributor for ApprovedReviewContributor {
|
impl ApprovalReviewContributor for ApprovedReviewContributor {
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
_session_store: &'a ExtensionData,
|
_session_store: &'a ExtensionData,
|
||||||
_thread_store: &'a ExtensionData,
|
_thread_store: &'a ExtensionData,
|
||||||
@@ -83,7 +83,7 @@ impl ApprovalReviewContributor for ApprovedReviewContributor {
|
|||||||
struct EscalationApprovingReviewContributor;
|
struct EscalationApprovingReviewContributor;
|
||||||
|
|
||||||
impl ApprovalReviewContributor for EscalationApprovingReviewContributor {
|
impl ApprovalReviewContributor for EscalationApprovingReviewContributor {
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
_session_store: &'a ExtensionData,
|
_session_store: &'a ExtensionData,
|
||||||
_thread_store: &'a ExtensionData,
|
_thread_store: &'a ExtensionData,
|
||||||
|
|||||||
@@ -1161,7 +1161,7 @@ async fn guardian_timeout_rejects_tool_call_with_acting_model_instructions(
|
|||||||
struct TimedOutReviewContributor;
|
struct TimedOutReviewContributor;
|
||||||
|
|
||||||
impl codex_extension_api::ApprovalReviewContributor for TimedOutReviewContributor {
|
impl codex_extension_api::ApprovalReviewContributor for TimedOutReviewContributor {
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
_session_store: &'a codex_extension_api::ExtensionData,
|
_session_store: &'a codex_extension_api::ExtensionData,
|
||||||
_thread_store: &'a codex_extension_api::ExtensionData,
|
_thread_store: &'a codex_extension_api::ExtensionData,
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ use codex_tools::ToolExecutor;
|
|||||||
use crate::ExtensionData;
|
use crate::ExtensionData;
|
||||||
use crate::ExtensionMetrics;
|
use crate::ExtensionMetrics;
|
||||||
|
|
||||||
|
mod approval_review;
|
||||||
mod context;
|
mod context;
|
||||||
mod mcp;
|
mod mcp;
|
||||||
mod prompt;
|
mod prompt;
|
||||||
@@ -22,6 +23,9 @@ mod turn_input;
|
|||||||
mod turn_lifecycle;
|
mod turn_lifecycle;
|
||||||
mod world_state;
|
mod world_state;
|
||||||
|
|
||||||
|
pub use approval_review::ApprovalAssessment;
|
||||||
|
pub use approval_review::ApprovalReviewError;
|
||||||
|
pub use approval_review::ApprovalReviewInput;
|
||||||
pub use context::TurnContextContributionInput;
|
pub use context::TurnContextContributionInput;
|
||||||
pub use mcp::McpServerContribution;
|
pub use mcp::McpServerContribution;
|
||||||
pub use mcp::McpServerContributionContext;
|
pub use mcp::McpServerContributionContext;
|
||||||
@@ -326,16 +330,30 @@ pub trait ToolLifecycleContributor: Send + Sync {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Extension contribution that can claim rendered approval-review prompts.
|
/// Extension contribution for fast approval decisions and full action reviews.
|
||||||
|
///
|
||||||
|
/// Implementations can provide a fast decision from existing evidence, perform
|
||||||
|
/// a full structured review, or support both paths. Returning `None` leaves the
|
||||||
|
/// request available to the next contributor or the host's fallback path.
|
||||||
pub trait ApprovalReviewContributor: Send + Sync {
|
pub trait ApprovalReviewContributor: Send + Sync {
|
||||||
/// Reviews one action using metrics bound to the active turn's model.
|
/// Returns an available approval decision without performing a full review.
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
session_store: &'a ExtensionData,
|
_session_store: &'a ExtensionData,
|
||||||
thread_store: &'a ExtensionData,
|
_thread_store: &'a ExtensionData,
|
||||||
prompt: &'a str,
|
_prompt: &'a str,
|
||||||
extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
_extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
||||||
) -> ExtensionFuture<'a, Option<ReviewDecision>>;
|
) -> ExtensionFuture<'a, Option<ReviewDecision>> {
|
||||||
|
Box::pin(std::future::ready(None))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Performs a full review of a structured host-owned approval request.
|
||||||
|
fn full_review<'a>(
|
||||||
|
&'a self,
|
||||||
|
_input: &'a ApprovalReviewInput<'_>,
|
||||||
|
) -> ExtensionFuture<'a, Option<Result<ApprovalAssessment, ApprovalReviewError>>> {
|
||||||
|
Box::pin(std::future::ready(None))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Ordered post-processing contribution for one parsed turn item.
|
/// Ordered post-processing contribution for one parsed turn item.
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
use std::fmt;
|
||||||
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
use codex_protocol::ThreadId;
|
||||||
|
use codex_protocol::approvals::GuardianAssessmentOutcome;
|
||||||
|
use codex_protocol::approvals::GuardianRiskLevel;
|
||||||
|
use codex_protocol::approvals::GuardianUserAuthorization;
|
||||||
|
use serde_json::Value;
|
||||||
|
|
||||||
|
use crate::ConversationHistorySnapshot;
|
||||||
|
use crate::ExtensionData;
|
||||||
|
|
||||||
|
/// Classification returned by an approval reviewer.
|
||||||
|
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||||
|
pub struct ApprovalAssessment {
|
||||||
|
/// Final allow-or-deny decision for the requested action.
|
||||||
|
pub outcome: GuardianAssessmentOutcome,
|
||||||
|
/// Risk level assigned to the action by the reviewer.
|
||||||
|
pub risk_level: GuardianRiskLevel,
|
||||||
|
/// Whether the conversation authorizes the requested action.
|
||||||
|
pub user_authorization: GuardianUserAuthorization,
|
||||||
|
/// Human-readable explanation of the assessment.
|
||||||
|
pub rationale: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Operational failure returned by an approval reviewer.
|
||||||
|
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||||
|
pub enum ApprovalReviewError {
|
||||||
|
/// The reviewer could not produce a valid assessment.
|
||||||
|
Failed(String),
|
||||||
|
/// The request exceeded its host-owned review deadline.
|
||||||
|
TimedOut,
|
||||||
|
/// The request was cancelled by its parent approval.
|
||||||
|
Cancelled,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl fmt::Display for ApprovalReviewError {
|
||||||
|
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||||
|
match self {
|
||||||
|
Self::Failed(message) => write!(formatter, "approval review failed: {message}"),
|
||||||
|
Self::TimedOut => formatter.write_str("approval review timed out"),
|
||||||
|
Self::Cancelled => formatter.write_str("approval review was cancelled"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl std::error::Error for ApprovalReviewError {}
|
||||||
|
|
||||||
|
/// Immutable context for reviewing an existing host-owned approval action.
|
||||||
|
pub struct ApprovalReviewInput<'a> {
|
||||||
|
/// Structured action evidence rendered by the host's existing approval path.
|
||||||
|
pub action: &'a Value,
|
||||||
|
/// Canonical immutable conversation snapshot captured before review.
|
||||||
|
pub conversation_history: Arc<dyn ConversationHistorySnapshot>,
|
||||||
|
/// Stable host-owned thread identifier.
|
||||||
|
pub thread_id: ThreadId,
|
||||||
|
/// Store scoped to this thread runtime.
|
||||||
|
pub thread_store: &'a ExtensionData,
|
||||||
|
/// Stable host-owned turn identifier.
|
||||||
|
pub turn_id: &'a str,
|
||||||
|
/// Policy reason that caused the host to request approval.
|
||||||
|
pub approval_reason: Option<&'a str>,
|
||||||
|
/// Reason an earlier request is being retried, when applicable.
|
||||||
|
pub retry_reason: Option<&'a str>,
|
||||||
|
}
|
||||||
@@ -39,7 +39,10 @@ pub use codex_tools::TurnItemEmissionFuture;
|
|||||||
pub use codex_tools::TurnItemEmitter;
|
pub use codex_tools::TurnItemEmitter;
|
||||||
pub use codex_tools::parse_tool_input_schema;
|
pub use codex_tools::parse_tool_input_schema;
|
||||||
pub use codex_tools::parse_tool_input_schema_without_compaction;
|
pub use codex_tools::parse_tool_input_schema_without_compaction;
|
||||||
|
pub use contributors::ApprovalAssessment;
|
||||||
pub use contributors::ApprovalReviewContributor;
|
pub use contributors::ApprovalReviewContributor;
|
||||||
|
pub use contributors::ApprovalReviewError;
|
||||||
|
pub use contributors::ApprovalReviewInput;
|
||||||
pub use contributors::ConfigContributor;
|
pub use contributors::ConfigContributor;
|
||||||
pub use contributors::ContextContributor;
|
pub use contributors::ContextContributor;
|
||||||
pub use contributors::ExtensionFuture;
|
pub use contributors::ExtensionFuture;
|
||||||
|
|||||||
@@ -2,7 +2,10 @@ use std::sync::Arc;
|
|||||||
|
|
||||||
use codex_protocol::protocol::ReviewDecision;
|
use codex_protocol::protocol::ReviewDecision;
|
||||||
|
|
||||||
|
use crate::ApprovalAssessment;
|
||||||
use crate::ApprovalReviewContributor;
|
use crate::ApprovalReviewContributor;
|
||||||
|
use crate::ApprovalReviewError;
|
||||||
|
use crate::ApprovalReviewInput;
|
||||||
use crate::ConfigContributor;
|
use crate::ConfigContributor;
|
||||||
use crate::ContextContributor;
|
use crate::ContextContributor;
|
||||||
use crate::ExtensionData;
|
use crate::ExtensionData;
|
||||||
@@ -188,9 +191,22 @@ impl<C: Sync> ExtensionRegistry<C> {
|
|||||||
&self.skill_invocation_contributors
|
&self.skill_invocation_contributors
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Claims the first rendered approval-review prompt accepted by an
|
/// Returns the first full approval assessment claimed by a contributor.
|
||||||
/// installed contributor.
|
pub async fn full_approval_review(
|
||||||
pub async fn approval_review(
|
&self,
|
||||||
|
input: ApprovalReviewInput<'_>,
|
||||||
|
) -> Option<Result<ApprovalAssessment, ApprovalReviewError>> {
|
||||||
|
for contributor in &self.approval_review_contributors {
|
||||||
|
if let Some(assessment) = contributor.full_review(&input).await {
|
||||||
|
return Some(assessment);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
None
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns the first fast approval decision claimed by a contributor.
|
||||||
|
pub async fn fast_approval_decision(
|
||||||
&self,
|
&self,
|
||||||
session_store: &ExtensionData,
|
session_store: &ExtensionData,
|
||||||
thread_store: &ExtensionData,
|
thread_store: &ExtensionData,
|
||||||
@@ -199,7 +215,7 @@ impl<C: Sync> ExtensionRegistry<C> {
|
|||||||
) -> Option<ReviewDecision> {
|
) -> Option<ReviewDecision> {
|
||||||
for contributor in &self.approval_review_contributors {
|
for contributor in &self.approval_review_contributors {
|
||||||
if let Some(decision) = contributor
|
if let Some(decision) = contributor
|
||||||
.contribute(
|
.fast_decision(
|
||||||
session_store,
|
session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
prompt,
|
prompt,
|
||||||
|
|||||||
@@ -3,11 +3,15 @@
|
|||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use std::sync::Mutex;
|
use std::sync::Mutex;
|
||||||
|
|
||||||
|
use codex_extension_api::ApprovalAssessment;
|
||||||
use codex_extension_api::ApprovalReviewContributor;
|
use codex_extension_api::ApprovalReviewContributor;
|
||||||
|
use codex_extension_api::ApprovalReviewError;
|
||||||
|
use codex_extension_api::ApprovalReviewInput;
|
||||||
use codex_extension_api::ConfigContributor;
|
use codex_extension_api::ConfigContributor;
|
||||||
use codex_extension_api::ContentItemKind;
|
use codex_extension_api::ContentItemKind;
|
||||||
use codex_extension_api::ContextContributor;
|
use codex_extension_api::ContextContributor;
|
||||||
use codex_extension_api::ContextualUserFragment;
|
use codex_extension_api::ContextualUserFragment;
|
||||||
|
use codex_extension_api::ConversationHistorySnapshot;
|
||||||
use codex_extension_api::ExtensionData;
|
use codex_extension_api::ExtensionData;
|
||||||
use codex_extension_api::ExtensionDataInit;
|
use codex_extension_api::ExtensionDataInit;
|
||||||
use codex_extension_api::ExtensionEventSink;
|
use codex_extension_api::ExtensionEventSink;
|
||||||
@@ -17,6 +21,7 @@ use codex_extension_api::ExtensionRegistryBuilder;
|
|||||||
use codex_extension_api::ExtensionWarning;
|
use codex_extension_api::ExtensionWarning;
|
||||||
use codex_extension_api::McpServerContributionContext;
|
use codex_extension_api::McpServerContributionContext;
|
||||||
use codex_extension_api::PromptFragment;
|
use codex_extension_api::PromptFragment;
|
||||||
|
use codex_extension_api::ResponseItem;
|
||||||
use codex_extension_api::SkillInvocationContributor;
|
use codex_extension_api::SkillInvocationContributor;
|
||||||
use codex_extension_api::ThreadLifecycleContributor;
|
use codex_extension_api::ThreadLifecycleContributor;
|
||||||
use codex_extension_api::TokenUsageContributor;
|
use codex_extension_api::TokenUsageContributor;
|
||||||
@@ -30,6 +35,10 @@ use codex_extension_api::TurnInputContributor;
|
|||||||
use codex_extension_api::TurnItemContributor;
|
use codex_extension_api::TurnItemContributor;
|
||||||
use codex_extension_api::TurnLifecycleContributor;
|
use codex_extension_api::TurnLifecycleContributor;
|
||||||
use codex_extension_api::empty_extension_registry;
|
use codex_extension_api::empty_extension_registry;
|
||||||
|
use codex_protocol::ThreadId;
|
||||||
|
use codex_protocol::approvals::GuardianAssessmentOutcome;
|
||||||
|
use codex_protocol::approvals::GuardianRiskLevel;
|
||||||
|
use codex_protocol::approvals::GuardianUserAuthorization;
|
||||||
use codex_protocol::items::HookPromptItem;
|
use codex_protocol::items::HookPromptItem;
|
||||||
use codex_protocol::items::TurnItem;
|
use codex_protocol::items::TurnItem;
|
||||||
use codex_protocol::protocol::Event;
|
use codex_protocol::protocol::Event;
|
||||||
@@ -39,6 +48,7 @@ use codex_protocol::protocol::SessionSource;
|
|||||||
use codex_protocol::protocol::SubAgentSource;
|
use codex_protocol::protocol::SubAgentSource;
|
||||||
use codex_protocol::protocol::WarningEvent;
|
use codex_protocol::protocol::WarningEvent;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
|
use serde_json::json;
|
||||||
|
|
||||||
struct AllContributors;
|
struct AllContributors;
|
||||||
|
|
||||||
@@ -130,7 +140,7 @@ impl TurnItemContributor for AllContributors {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl ApprovalReviewContributor for AllContributors {
|
impl ApprovalReviewContributor for AllContributors {
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
_session_store: &'a ExtensionData,
|
_session_store: &'a ExtensionData,
|
||||||
_thread_store: &'a ExtensionData,
|
_thread_store: &'a ExtensionData,
|
||||||
@@ -142,6 +152,22 @@ impl ApprovalReviewContributor for AllContributors {
|
|||||||
Some(ReviewDecision::ApprovedForSession)
|
Some(ReviewDecision::ApprovedForSession)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn full_review<'a>(
|
||||||
|
&'a self,
|
||||||
|
input: &'a ApprovalReviewInput<'_>,
|
||||||
|
) -> ExtensionFuture<'a, Option<Result<ApprovalAssessment, ApprovalReviewError>>> {
|
||||||
|
assert_eq!(
|
||||||
|
(input.approval_reason, input.retry_reason),
|
||||||
|
(Some("A policy rule requires approval."), None)
|
||||||
|
);
|
||||||
|
Box::pin(std::future::ready(Some(Ok(ApprovalAssessment {
|
||||||
|
outcome: GuardianAssessmentOutcome::Allow,
|
||||||
|
risk_level: GuardianRiskLevel::Low,
|
||||||
|
user_authorization: GuardianUserAuthorization::High,
|
||||||
|
rationale: "approved".to_string(),
|
||||||
|
}))))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
@@ -173,7 +199,7 @@ async fn build_round_trips_every_contributor_category() {
|
|||||||
assert_eq!(registry.turn_item_contributors().len(), 1);
|
assert_eq!(registry.turn_item_contributors().len(), 1);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&ExtensionData::new("session"),
|
&ExtensionData::new("session"),
|
||||||
&ExtensionData::new("thread"),
|
&ExtensionData::new("thread"),
|
||||||
"review this",
|
"review this",
|
||||||
@@ -184,6 +210,96 @@ async fn build_round_trips_every_contributor_category() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl ConversationHistorySnapshot for AllContributors {
|
||||||
|
fn history_version(&self) -> u64 {
|
||||||
|
0
|
||||||
|
}
|
||||||
|
|
||||||
|
fn items(&self) -> Box<dyn Iterator<Item = &ResponseItem> + Send + '_> {
|
||||||
|
Box::new(std::iter::empty())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn full_approval_review_returns_first_claim_and_short_circuits() {
|
||||||
|
let assessment = ApprovalAssessment {
|
||||||
|
outcome: GuardianAssessmentOutcome::Allow,
|
||||||
|
risk_level: GuardianRiskLevel::Low,
|
||||||
|
user_authorization: GuardianUserAuthorization::High,
|
||||||
|
rationale: "approved".to_string(),
|
||||||
|
};
|
||||||
|
let action = json!({
|
||||||
|
"tool": "exec_command",
|
||||||
|
"command": ["echo", "hello"],
|
||||||
|
});
|
||||||
|
let thread_store = ExtensionData::new("thread");
|
||||||
|
let input = ApprovalReviewInput {
|
||||||
|
action: &action,
|
||||||
|
conversation_history: Arc::new(AllContributors),
|
||||||
|
thread_id: ThreadId::default(),
|
||||||
|
thread_store: &thread_store,
|
||||||
|
turn_id: "turn",
|
||||||
|
approval_reason: Some("A policy rule requires approval."),
|
||||||
|
retry_reason: None,
|
||||||
|
};
|
||||||
|
|
||||||
|
let calls = Arc::new(Mutex::new(Vec::new()));
|
||||||
|
let mut builder = ExtensionRegistryBuilder::<()>::new();
|
||||||
|
for (name, result) in [
|
||||||
|
("first", None),
|
||||||
|
("second", Some(Ok(assessment.clone()))),
|
||||||
|
(
|
||||||
|
"third",
|
||||||
|
Some(Err(ApprovalReviewError::Failed(
|
||||||
|
"should not run".to_string(),
|
||||||
|
))),
|
||||||
|
),
|
||||||
|
] {
|
||||||
|
builder.approval_review_contributor(Arc::new(RecordingStructuredApprovalContributor {
|
||||||
|
name,
|
||||||
|
result,
|
||||||
|
calls: Arc::clone(&calls),
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
builder.build().full_approval_review(input).await,
|
||||||
|
Some(Ok(assessment))
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
calls.lock().expect("approval calls lock").as_slice(),
|
||||||
|
["first", "second"]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
struct RecordingStructuredApprovalContributor {
|
||||||
|
name: &'static str,
|
||||||
|
result: Option<Result<ApprovalAssessment, ApprovalReviewError>>,
|
||||||
|
calls: Arc<Mutex<Vec<&'static str>>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl ApprovalReviewContributor for RecordingStructuredApprovalContributor {
|
||||||
|
fn full_review<'a>(
|
||||||
|
&'a self,
|
||||||
|
input: &'a ApprovalReviewInput<'_>,
|
||||||
|
) -> ExtensionFuture<'a, Option<Result<ApprovalAssessment, ApprovalReviewError>>> {
|
||||||
|
Box::pin(async move {
|
||||||
|
assert_eq!(
|
||||||
|
input.action,
|
||||||
|
&json!({
|
||||||
|
"tool": "exec_command",
|
||||||
|
"command": ["echo", "hello"],
|
||||||
|
})
|
||||||
|
);
|
||||||
|
self.calls
|
||||||
|
.lock()
|
||||||
|
.expect("approval calls lock should not be poisoned")
|
||||||
|
.push(self.name);
|
||||||
|
self.result.clone()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
struct NamedContextContributor(&'static str);
|
struct NamedContextContributor(&'static str);
|
||||||
|
|
||||||
impl ContextContributor for NamedContextContributor {
|
impl ContextContributor for NamedContextContributor {
|
||||||
@@ -334,7 +450,7 @@ struct RecordingApprovalContributor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl ApprovalReviewContributor for RecordingApprovalContributor {
|
impl ApprovalReviewContributor for RecordingApprovalContributor {
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
session_store: &'a ExtensionData,
|
session_store: &'a ExtensionData,
|
||||||
thread_store: &'a ExtensionData,
|
thread_store: &'a ExtensionData,
|
||||||
@@ -357,7 +473,7 @@ impl ApprovalReviewContributor for RecordingApprovalContributor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn approval_review_returns_first_claim_and_short_circuits() {
|
async fn fast_approval_decision_returns_first_claim_and_short_circuits() {
|
||||||
let calls = Arc::new(Mutex::new(Vec::new()));
|
let calls = Arc::new(Mutex::new(Vec::new()));
|
||||||
let mut builder = ExtensionRegistryBuilder::<()>::new();
|
let mut builder = ExtensionRegistryBuilder::<()>::new();
|
||||||
for (name, decision) in [
|
for (name, decision) in [
|
||||||
@@ -377,7 +493,7 @@ async fn approval_review_returns_first_claim_and_short_circuits() {
|
|||||||
let registry = builder.build();
|
let registry = builder.build();
|
||||||
|
|
||||||
let decision = registry
|
let decision = registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&ExtensionData::new("session-1"),
|
&ExtensionData::new("session-1"),
|
||||||
&ExtensionData::new("thread-1"),
|
&ExtensionData::new("thread-1"),
|
||||||
"allow command?",
|
"allow command?",
|
||||||
@@ -461,12 +577,12 @@ fn custom_event_sink_survives_registry_build() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn empty_registry_does_not_claim_approval_review() {
|
async fn empty_registry_does_not_claim_fast_approval_decision() {
|
||||||
let registry = empty_extension_registry::<()>();
|
let registry = empty_extension_registry::<()>();
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&ExtensionData::new("session"),
|
&ExtensionData::new("session"),
|
||||||
&ExtensionData::new("thread"),
|
&ExtensionData::new("thread"),
|
||||||
"unclaimed",
|
"unclaimed",
|
||||||
|
|||||||
@@ -363,7 +363,7 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl ApprovalReviewContributor for GuardianV2Extension {
|
impl ApprovalReviewContributor for GuardianV2Extension {
|
||||||
fn contribute<'a>(
|
fn fast_decision<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
_session_store: &'a ExtensionData,
|
_session_store: &'a ExtensionData,
|
||||||
thread_store: &'a ExtensionData,
|
thread_store: &'a ExtensionData,
|
||||||
|
|||||||
@@ -170,7 +170,7 @@ async fn installed_extension_reconnects_after_auth_refresh() -> Result<()> {
|
|||||||
.await?;
|
.await?;
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -622,7 +622,7 @@ async fn sample_configured_conversation_history(
|
|||||||
}
|
}
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -751,7 +751,7 @@ impl GuardianFailureFixture {
|
|||||||
.await?;
|
.await?;
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
self.registry
|
self.registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&self.session_store,
|
&self.session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1071,7 +1071,7 @@ max_recent_non_user_entries = 8
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1092,7 +1092,7 @@ max_recent_non_user_entries = 8
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1115,7 +1115,7 @@ max_recent_non_user_entries = 8
|
|||||||
.store(/*val*/ 3, Ordering::Release);
|
.store(/*val*/ 3, Ordering::Release);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1134,7 +1134,7 @@ max_recent_non_user_entries = 8
|
|||||||
.store(/*val*/ 4, Ordering::Release);
|
.store(/*val*/ 4, Ordering::Release);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1155,7 +1155,7 @@ max_recent_non_user_entries = 8
|
|||||||
.store(/*val*/ 2, Ordering::Release);
|
.store(/*val*/ 2, Ordering::Release);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1447,7 +1447,7 @@ async fn contributor_uses_model_defaults_and_preserves_local_overrides() -> Resu
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1603,7 +1603,7 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<
|
|||||||
assert!(score.sampled_at.is_some());
|
assert!(score.sampled_at.is_some());
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1618,7 +1618,7 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1634,7 +1634,7 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1651,7 +1651,7 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
&disabled_thread_store,
|
&disabled_thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1738,7 +1738,7 @@ async fn contributor_skips_models_requiring_managed_guardian_review() -> Result<
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1818,7 +1818,7 @@ async fn contributor_counts_failed_thread_lookups_toward_score_lag() -> Result<(
|
|||||||
});
|
});
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -1854,7 +1854,7 @@ async fn contributor_counts_failed_thread_lookups_toward_score_lag() -> Result<(
|
|||||||
assert_eq!(score_progress.latest_tool_call.load(Ordering::Acquire), 2);
|
assert_eq!(score_progress.latest_tool_call.load(Ordering::Acquire), 2);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -2197,7 +2197,7 @@ async fn contributor_reuses_the_latest_compatible_parent_compaction() -> Result<
|
|||||||
assert_eq!(previous_score.scores.get("action_risk"), Some(&0.0));
|
assert_eq!(previous_score.scores.get("action_risk"), Some(&0.0));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
@@ -2241,7 +2241,7 @@ async fn contributor_reuses_the_latest_compatible_parent_compaction() -> Result<
|
|||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.approval_review(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
"review action",
|
||||||
|
|||||||
Reference in new issue
Block a user