mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-25 03:32:32 +00:00
feat(sync): add websocket fallback and approval parity
Add websocket v1/v2 selection with sticky HTTP fallback, expand app-server approval payloads for network/additional permissions, and cap/exclude auto-review diff path lists to prevent oversized review prompts.
This commit is contained in:
@@ -1202,4 +1202,34 @@ mod tests {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&request);
|
||||
assert_eq!(reason, Some("thread/start.mockExperimentalField"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_execution_request_approval_additional_permissions_is_marked_experimental() {
|
||||
let params = v2::CommandExecutionRequestApprovalParams {
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
approval_id: None,
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
command: Some("cat file".to_string()),
|
||||
cwd: None,
|
||||
command_actions: None,
|
||||
additional_permissions: Some(v2::AdditionalPermissionProfile {
|
||||
network: None,
|
||||
file_system: Some(v2::AdditionalFileSystemPermissions {
|
||||
read: Some(vec![std::path::PathBuf::from("/tmp/allowed")]),
|
||||
write: None,
|
||||
}),
|
||||
macos: None,
|
||||
}),
|
||||
proposed_execpolicy_amendment: None,
|
||||
};
|
||||
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¶ms);
|
||||
assert_eq!(
|
||||
reason,
|
||||
Some("item/commandExecution/requestApproval.additionalPermissions")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,8 @@ use crate::protocol::common::AuthMode;
|
||||
use code_experimental_api_macros::ExperimentalApi;
|
||||
use code_protocol::account::PlanType;
|
||||
use code_protocol::approvals::ExecPolicyAmendment as CoreExecPolicyAmendment;
|
||||
use code_protocol::approvals::NetworkApprovalContext as CoreNetworkApprovalContext;
|
||||
use code_protocol::approvals::NetworkApprovalProtocol as CoreNetworkApprovalProtocol;
|
||||
use code_protocol::config_types::CollaborationMode;
|
||||
use code_protocol::config_types::CollaborationModeMask;
|
||||
use code_protocol::config_types::ForcedLoginMethod;
|
||||
@@ -18,6 +20,11 @@ use code_protocol::items::TurnItem as CoreTurnItem;
|
||||
use code_protocol::mcp::Resource as McpResource;
|
||||
use code_protocol::mcp::ResourceTemplate as McpResourceTemplate;
|
||||
use code_protocol::mcp::Tool as McpTool;
|
||||
use code_protocol::models::FileSystemPermissions as CoreFileSystemPermissions;
|
||||
use code_protocol::models::MacOsAutomationValue as CoreMacOsAutomationValue;
|
||||
use code_protocol::models::MacOsPermissions as CoreMacOsPermissions;
|
||||
use code_protocol::models::MacOsPreferencesValue as CoreMacOsPreferencesValue;
|
||||
use code_protocol::models::PermissionProfile as CorePermissionProfile;
|
||||
use code_protocol::models::ResponseItem;
|
||||
use code_protocol::openai_models::InputModality;
|
||||
use code_protocol::openai_models::ReasoningEffort;
|
||||
@@ -644,6 +651,89 @@ pub enum CommandExecutionApprovalDecision {
|
||||
Cancel,
|
||||
}
|
||||
|
||||
v2_enum_from_core! {
|
||||
pub enum NetworkApprovalProtocol from CoreNetworkApprovalProtocol {
|
||||
Http,
|
||||
Https,
|
||||
Socks5Tcp,
|
||||
Socks5Udp,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct NetworkApprovalContext {
|
||||
pub host: String,
|
||||
pub protocol: NetworkApprovalProtocol,
|
||||
}
|
||||
|
||||
impl From<CoreNetworkApprovalContext> for NetworkApprovalContext {
|
||||
fn from(value: CoreNetworkApprovalContext) -> Self {
|
||||
Self {
|
||||
host: value.host,
|
||||
protocol: value.protocol.into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct AdditionalFileSystemPermissions {
|
||||
pub read: Option<Vec<PathBuf>>,
|
||||
pub write: Option<Vec<PathBuf>>,
|
||||
}
|
||||
|
||||
impl From<CoreFileSystemPermissions> for AdditionalFileSystemPermissions {
|
||||
fn from(value: CoreFileSystemPermissions) -> Self {
|
||||
Self {
|
||||
read: value.read,
|
||||
write: value.write,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct AdditionalMacOsPermissions {
|
||||
pub preferences: Option<CoreMacOsPreferencesValue>,
|
||||
pub automations: Option<CoreMacOsAutomationValue>,
|
||||
pub accessibility: Option<bool>,
|
||||
pub calendar: Option<bool>,
|
||||
}
|
||||
|
||||
impl From<CoreMacOsPermissions> for AdditionalMacOsPermissions {
|
||||
fn from(value: CoreMacOsPermissions) -> Self {
|
||||
Self {
|
||||
preferences: value.preferences,
|
||||
automations: value.automations,
|
||||
accessibility: value.accessibility,
|
||||
calendar: value.calendar,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct AdditionalPermissionProfile {
|
||||
pub network: Option<bool>,
|
||||
pub file_system: Option<AdditionalFileSystemPermissions>,
|
||||
pub macos: Option<AdditionalMacOsPermissions>,
|
||||
}
|
||||
|
||||
impl From<CorePermissionProfile> for AdditionalPermissionProfile {
|
||||
fn from(value: CorePermissionProfile) -> Self {
|
||||
Self {
|
||||
network: value.network,
|
||||
file_system: value.file_system.map(AdditionalFileSystemPermissions::from),
|
||||
macos: value.macos.map(AdditionalMacOsPermissions::from),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -2991,7 +3081,7 @@ pub struct ContextCompactedNotification {
|
||||
pub turn_id: String,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct CommandExecutionRequestApprovalParams {
|
||||
@@ -3003,8 +3093,13 @@ pub struct CommandExecutionRequestApprovalParams {
|
||||
#[ts(optional = nullable)]
|
||||
pub approval_id: Option<String>,
|
||||
/// Optional explanatory reason (e.g. request for network access).
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub reason: Option<String>,
|
||||
/// Optional context for a managed-network approval prompt.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub network_approval_context: Option<NetworkApprovalContext>,
|
||||
/// The command to be executed.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
@@ -3017,11 +3112,23 @@ pub struct CommandExecutionRequestApprovalParams {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub command_actions: Option<Vec<CommandAction>>,
|
||||
/// Optional additional permissions requested for this command.
|
||||
#[experimental("item/commandExecution/requestApproval.additionalPermissions")]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
/// Optional proposed execpolicy amendment to allow similar commands without prompting.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
|
||||
}
|
||||
|
||||
impl CommandExecutionRequestApprovalParams {
|
||||
pub fn strip_experimental_fields(&mut self) {
|
||||
self.additional_permissions = None;
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
|
||||
@@ -1518,6 +1518,7 @@ async fn apply_bespoke_event_handling(
|
||||
cwd,
|
||||
reason,
|
||||
network_approval_context: _,
|
||||
additional_permissions,
|
||||
}) => {
|
||||
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
|
||||
let params = ExecCommandApprovalParams {
|
||||
@@ -1527,6 +1528,7 @@ async fn apply_bespoke_event_handling(
|
||||
command,
|
||||
cwd,
|
||||
reason,
|
||||
additional_permissions,
|
||||
};
|
||||
let value = serde_json::to_value(¶ms).unwrap_or_default();
|
||||
let rx = outgoing
|
||||
|
||||
+84
-18
@@ -83,9 +83,23 @@ use std::sync::RwLock;
|
||||
|
||||
const RESPONSES_BETA_HEADER_V1: &str = "responses=v1";
|
||||
const RESPONSES_BETA_HEADER_EXPERIMENTAL: &str = "responses=experimental";
|
||||
const RESPONSES_WEBSOCKETS_BETA_HEADER: &str = "responses_websockets=2026-02-06";
|
||||
const RESPONSES_WEBSOCKETS_BETA_HEADER_V1: &str = "responses_websockets=2026-02-04";
|
||||
const RESPONSES_WEBSOCKETS_BETA_HEADER_V2: &str = "responses_websockets=2026-02-06";
|
||||
const RESPONSES_WEBSOCKET_INGRESS_BUFFER: usize = 256;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum ResponsesWebsocketVersion {
|
||||
V1,
|
||||
V2,
|
||||
}
|
||||
|
||||
fn preferred_ws_version_from_env() -> ResponsesWebsocketVersion {
|
||||
match std::env::var("CODE_RESPONSES_WEBSOCKET_VERSION") {
|
||||
Ok(value) if value.eq_ignore_ascii_case("v1") => ResponsesWebsocketVersion::V1,
|
||||
_ => ResponsesWebsocketVersion::V2,
|
||||
}
|
||||
}
|
||||
|
||||
// Sticky-routing token captured at the start of a turn. When present, it must
|
||||
// be replayed on every subsequent request within the same turn (retries,
|
||||
// continuations, websocket reconnects).
|
||||
@@ -246,6 +260,7 @@ pub struct ModelClient {
|
||||
effort: ReasoningEffortConfig,
|
||||
summary: ReasoningSummaryConfig,
|
||||
reasoning_summary_disabled: AtomicBool,
|
||||
websockets_disabled: AtomicBool,
|
||||
verbosity: TextVerbosityConfig,
|
||||
debug_logger: Arc<Mutex<DebugLogger>>,
|
||||
}
|
||||
@@ -264,6 +279,9 @@ impl Clone for ModelClient {
|
||||
reasoning_summary_disabled: AtomicBool::new(
|
||||
self.reasoning_summary_disabled.load(Ordering::Relaxed),
|
||||
),
|
||||
websockets_disabled: AtomicBool::new(
|
||||
self.websockets_disabled.load(Ordering::Relaxed),
|
||||
),
|
||||
verbosity: self.verbosity,
|
||||
debug_logger: Arc::clone(&self.debug_logger),
|
||||
}
|
||||
@@ -296,11 +314,39 @@ impl ModelClient {
|
||||
effort: clamped_effort,
|
||||
summary,
|
||||
reasoning_summary_disabled: AtomicBool::new(false),
|
||||
websockets_disabled: AtomicBool::new(false),
|
||||
verbosity: effective_verbosity,
|
||||
debug_logger,
|
||||
}
|
||||
}
|
||||
|
||||
fn active_ws_version_for_prompt(&self, prompt: &Prompt) -> Option<ResponsesWebsocketVersion> {
|
||||
if self.websockets_disabled.load(Ordering::Relaxed) {
|
||||
return None;
|
||||
}
|
||||
|
||||
match self.provider.wire_api {
|
||||
WireApi::ResponsesWebsocket => Some(preferred_ws_version_from_env()),
|
||||
WireApi::Responses => {
|
||||
let prefer_websockets = prompt
|
||||
.model_family_override
|
||||
.as_ref()
|
||||
.map(|family| family.prefer_websockets)
|
||||
.or_else(|| {
|
||||
prompt
|
||||
.model_override
|
||||
.as_deref()
|
||||
.and_then(find_family_for_model)
|
||||
.map(|family| family.prefer_websockets)
|
||||
})
|
||||
.unwrap_or(self.config.model_family.prefer_websockets);
|
||||
|
||||
prefer_websockets.then_some(preferred_ws_version_from_env())
|
||||
}
|
||||
WireApi::Chat => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Get the reasoning effort configuration
|
||||
pub fn get_reasoning_effort(&self) -> ReasoningEffortConfig {
|
||||
self.effort
|
||||
@@ -490,23 +536,14 @@ impl ModelClient {
|
||||
.or(prompt.log_tag.as_deref());
|
||||
match self.provider.wire_api {
|
||||
WireApi::Responses => {
|
||||
let prefer_websockets = prompt
|
||||
.model_family_override
|
||||
.as_ref()
|
||||
.map(|family| family.prefer_websockets)
|
||||
.or_else(|| {
|
||||
prompt
|
||||
.model_override
|
||||
.as_deref()
|
||||
.and_then(find_family_for_model)
|
||||
.map(|family| family.prefer_websockets)
|
||||
})
|
||||
.unwrap_or(self.config.model_family.prefer_websockets);
|
||||
|
||||
if prefer_websockets {
|
||||
match self.stream_responses_websocket(prompt, log_tag).await {
|
||||
if let Some(ws_version) = self.active_ws_version_for_prompt(prompt) {
|
||||
match self
|
||||
.stream_responses_websocket(prompt, log_tag, ws_version)
|
||||
.await
|
||||
{
|
||||
Ok(stream) => Ok(stream),
|
||||
Err(err) => {
|
||||
self.websockets_disabled.store(true, Ordering::Relaxed);
|
||||
warn!(
|
||||
"preferred websocket transport failed; falling back to responses HTTP stream: {err}"
|
||||
);
|
||||
@@ -517,7 +554,30 @@ impl ModelClient {
|
||||
self.stream_responses(prompt, log_tag).await
|
||||
}
|
||||
}
|
||||
WireApi::ResponsesWebsocket => self.stream_responses_websocket(prompt, log_tag).await,
|
||||
WireApi::ResponsesWebsocket => {
|
||||
if self.websockets_disabled.load(Ordering::Relaxed) {
|
||||
warn!(
|
||||
"responses_websocket transport disabled for this session; using responses HTTP stream"
|
||||
);
|
||||
return self.stream_responses(prompt, log_tag).await;
|
||||
}
|
||||
let ws_version = self
|
||||
.active_ws_version_for_prompt(prompt)
|
||||
.unwrap_or(preferred_ws_version_from_env());
|
||||
match self
|
||||
.stream_responses_websocket(prompt, log_tag, ws_version)
|
||||
.await
|
||||
{
|
||||
Ok(stream) => Ok(stream),
|
||||
Err(err) => {
|
||||
self.websockets_disabled.store(true, Ordering::Relaxed);
|
||||
warn!(
|
||||
"responses_websocket transport failed; falling back to responses HTTP stream: {err}"
|
||||
);
|
||||
self.stream_responses(prompt, log_tag).await
|
||||
}
|
||||
}
|
||||
}
|
||||
WireApi::Chat => {
|
||||
let effective_family = prompt
|
||||
.model_family_override
|
||||
@@ -573,6 +633,7 @@ impl ModelClient {
|
||||
&self,
|
||||
prompt: &Prompt,
|
||||
log_tag: Option<&str>,
|
||||
ws_version: ResponsesWebsocketVersion,
|
||||
) -> Result<ResponseStream> {
|
||||
let auth_manager = self.auth_manager.clone();
|
||||
let auth_mode = auth_manager
|
||||
@@ -775,7 +836,10 @@ impl ModelClient {
|
||||
// `responses=v1` / `responses=experimental`).
|
||||
ws_request.headers_mut().insert(
|
||||
reqwest::header::HeaderName::from_static("openai-beta"),
|
||||
HeaderValue::from_static(RESPONSES_WEBSOCKETS_BETA_HEADER),
|
||||
HeaderValue::from_static(match ws_version {
|
||||
ResponsesWebsocketVersion::V2 => RESPONSES_WEBSOCKETS_BETA_HEADER_V2,
|
||||
ResponsesWebsocketVersion::V1 => RESPONSES_WEBSOCKETS_BETA_HEADER_V1,
|
||||
}),
|
||||
);
|
||||
|
||||
// Wrap the normal /responses request payload in the WebSocket envelope.
|
||||
@@ -945,6 +1009,7 @@ impl ModelClient {
|
||||
}
|
||||
Err(err) => {
|
||||
if websocket_connect_is_upgrade_required(&err) {
|
||||
self.websockets_disabled.store(true, Ordering::Relaxed);
|
||||
warn!("responses websocket upgrade required; falling back to HTTP responses transport");
|
||||
return self.stream_responses(prompt, log_tag).await;
|
||||
}
|
||||
@@ -958,6 +1023,7 @@ impl ModelClient {
|
||||
tokio::time::sleep(backoff(attempt as u64)).await;
|
||||
continue;
|
||||
}
|
||||
self.websockets_disabled.store(true, Ordering::Relaxed);
|
||||
return Err(err);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1194,6 +1194,8 @@ impl Session {
|
||||
command: Vec<String>,
|
||||
cwd: PathBuf,
|
||||
reason: Option<String>,
|
||||
network_approval_context: Option<crate::protocol::NetworkApprovalContext>,
|
||||
additional_permissions: Option<code_protocol::models::PermissionProfile>,
|
||||
) -> oneshot::Receiver<ReviewDecision> {
|
||||
let (tx_approve, rx_approve) = oneshot::channel();
|
||||
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
|
||||
@@ -1206,7 +1208,8 @@ impl Session {
|
||||
command,
|
||||
cwd,
|
||||
reason,
|
||||
network_approval_context: None,
|
||||
network_approval_context,
|
||||
additional_permissions,
|
||||
}),
|
||||
);
|
||||
let _ = self.tx_event.send(event).await;
|
||||
|
||||
@@ -9341,6 +9341,8 @@ async fn handle_container_exec_with_params(
|
||||
params.command.clone(),
|
||||
params.cwd.clone(),
|
||||
params.justification.clone(),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -9793,6 +9795,8 @@ async fn handle_sandbox_error(
|
||||
params.command.clone(),
|
||||
cwd.clone(),
|
||||
Some("command failed; retry without sandbox?".to_string()),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -1328,6 +1328,9 @@ pub struct ExecApprovalRequestEvent {
|
||||
/// Optional network context for a blocked request that can be approved.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub network_approval_context: Option<NetworkApprovalContext>,
|
||||
/// Optional additional permissions requested for this command.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub additional_permissions: Option<code_protocol::models::PermissionProfile>,
|
||||
}
|
||||
|
||||
impl ExecApprovalRequestEvent {
|
||||
|
||||
+116
-5
@@ -95,6 +95,14 @@ use code_core::timeboxed_exec_guidance::{
|
||||
/// How long exec waits after task completion before sending Shutdown when Auto Review
|
||||
/// may be about to start. Guarded so sub-agents are not delayed.
|
||||
const AUTO_REVIEW_SHUTDOWN_GRACE_MS: u64 = 1_500;
|
||||
const REVIEW_SCOPE_MAX_LISTED_PATHS: usize = 120;
|
||||
const REVIEW_SCOPE_EXCLUDED_PREFIXES: [&str; 5] = [
|
||||
"codex-rs/",
|
||||
"node_modules/",
|
||||
"target/",
|
||||
".git/",
|
||||
".code/",
|
||||
];
|
||||
|
||||
pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()> {
|
||||
if let Err(err) = set_default_originator("code_exec") {
|
||||
@@ -2103,6 +2111,12 @@ fn snapshot_parent_diff_paths(cwd: &Path, parent: &str, head: &str) -> Option<Ve
|
||||
Some(paths)
|
||||
}
|
||||
|
||||
fn should_include_review_scope_path(path: &str) -> bool {
|
||||
!REVIEW_SCOPE_EXCLUDED_PREFIXES
|
||||
.iter()
|
||||
.any(|prefix| path.starts_with(prefix))
|
||||
}
|
||||
|
||||
fn apply_commit_scope_to_review_request(
|
||||
mut request: ReviewRequest,
|
||||
commit: &str,
|
||||
@@ -2122,11 +2136,51 @@ fn apply_commit_scope_to_review_request(
|
||||
|
||||
if let Some(paths) = paths {
|
||||
if !paths.is_empty() {
|
||||
prompt.push_str("\nFiles changed in this snapshot:\n");
|
||||
for path in paths {
|
||||
prompt.push_str("- ");
|
||||
prompt.push_str(path);
|
||||
prompt.push('\n');
|
||||
let mut listed_paths: Vec<&str> = Vec::new();
|
||||
let mut seen_paths: HashSet<&str> = HashSet::new();
|
||||
let mut excluded_count = 0usize;
|
||||
let mut truncated_count = 0usize;
|
||||
|
||||
for raw_path in paths {
|
||||
let normalized = raw_path.trim().trim_start_matches("./");
|
||||
if normalized.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if !seen_paths.insert(normalized) {
|
||||
continue;
|
||||
}
|
||||
if !should_include_review_scope_path(normalized) {
|
||||
excluded_count = excluded_count.saturating_add(1);
|
||||
continue;
|
||||
}
|
||||
if listed_paths.len() < REVIEW_SCOPE_MAX_LISTED_PATHS {
|
||||
listed_paths.push(normalized);
|
||||
} else {
|
||||
truncated_count = truncated_count.saturating_add(1);
|
||||
}
|
||||
}
|
||||
|
||||
if !listed_paths.is_empty() {
|
||||
prompt.push_str("\nFiles changed in this snapshot:\n");
|
||||
for path in listed_paths {
|
||||
prompt.push_str("- ");
|
||||
prompt.push_str(path);
|
||||
prompt.push('\n');
|
||||
}
|
||||
}
|
||||
|
||||
let omitted_count = excluded_count.saturating_add(truncated_count);
|
||||
if omitted_count > 0 {
|
||||
prompt.push_str(&format!(
|
||||
"\nOmitted {omitted_count} changed path(s) from this list ({excluded_count} excluded by path policy; {truncated_count} truncated for prompt size)."
|
||||
));
|
||||
}
|
||||
|
||||
if excluded_count > 0 {
|
||||
let excluded = REVIEW_SCOPE_EXCLUDED_PREFIXES.join(", ");
|
||||
prompt.push_str(&format!(
|
||||
"\nExcluded prefixes: {excluded}. Do not spend review effort on those paths unless explicitly asked."
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2703,6 +2757,63 @@ mod tests {
|
||||
assert!(cleaned.contains("Please review."));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_commit_scope_filters_and_truncates_paths() {
|
||||
let mut paths = vec![
|
||||
"./src/main.rs".to_string(),
|
||||
"codex-rs/Cargo.lock".to_string(),
|
||||
"./src/main.rs".to_string(),
|
||||
];
|
||||
for idx in 0..130 {
|
||||
paths.push(format!("src/generated_{idx}.rs"));
|
||||
}
|
||||
|
||||
let request = ReviewRequest {
|
||||
target: code_protocol::protocol::ReviewTarget::Custom {
|
||||
instructions: "baseline".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
prompt: "baseline".to_string(),
|
||||
};
|
||||
|
||||
let scoped = apply_commit_scope_to_review_request(request, "abc1234", "def5678", Some(&paths));
|
||||
assert!(scoped.prompt.contains("Files changed in this snapshot:"));
|
||||
assert!(scoped.prompt.contains("- src/main.rs"));
|
||||
assert!(!scoped.prompt.contains("codex-rs/Cargo.lock"));
|
||||
assert!(
|
||||
scoped
|
||||
.prompt
|
||||
.contains("Omitted 12 changed path(s) from this list (1 excluded by path policy; 11 truncated for prompt size).")
|
||||
);
|
||||
assert!(scoped.prompt.contains("Excluded prefixes: codex-rs/"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_commit_scope_reports_when_all_paths_excluded() {
|
||||
let paths = vec![
|
||||
"codex-rs/Cargo.lock".to_string(),
|
||||
"target/debug/code".to_string(),
|
||||
"node_modules/pkg/index.js".to_string(),
|
||||
];
|
||||
|
||||
let request = ReviewRequest {
|
||||
target: code_protocol::protocol::ReviewTarget::Custom {
|
||||
instructions: "baseline".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
prompt: "baseline".to_string(),
|
||||
};
|
||||
|
||||
let scoped = apply_commit_scope_to_review_request(request, "abc1234", "def5678", Some(&paths));
|
||||
assert!(!scoped.prompt.contains("Files changed in this snapshot:"));
|
||||
assert!(
|
||||
scoped
|
||||
.prompt
|
||||
.contains("Omitted 3 changed path(s) from this list (3 excluded by path policy; 0 truncated for prompt size).")
|
||||
);
|
||||
assert!(scoped.prompt.contains("Excluded prefixes: codex-rs/"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn should_skip_followup_detects_duplicate_snapshot() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::config_types::ReasoningSummary;
|
||||
use crate::config_types::SandboxMode;
|
||||
use crate::config_types::Verbosity;
|
||||
use crate::dynamic_tools::DynamicToolSpec;
|
||||
use crate::models::PermissionProfile;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::FileChange;
|
||||
@@ -755,6 +756,8 @@ pub struct ExecCommandApprovalParams {
|
||||
pub cwd: PathBuf,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub reason: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub additional_permissions: Option<PermissionProfile>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)]
|
||||
|
||||
@@ -465,6 +465,7 @@ fn smoke_exec_approval_event_structure() {
|
||||
cwd: PathBuf::from("/tmp"),
|
||||
reason: Some("verify".into()),
|
||||
network_approval_context: None,
|
||||
additional_permissions: None,
|
||||
};
|
||||
|
||||
let encoded = serde_json::to_string(&approval).expect("serialize exec approval");
|
||||
|
||||
Reference in New Issue
Block a user