mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Enable --deny-warnings for cargo shear (#21616)
## Summary In https://github.com/openai/codex/pull/21584, we disabled doctests for crates that lack any doctests. We can enforce that property via `cargo shear --deny-warnings`: crates that lack doctests will be flagged if doctests are enabled, and crates with doctests will be flagged if doctests are disabled. A few additional notes: - By adding `--deny-warnings`, `cargo shear` also flagged a number of modules that were not reachable at all. Some of those have been removed. - This PR removes a usage of `windows_modules!` (since `cargo shear` and `rustfmt` couldn't see through it) in favor of simple `#[cfg(target_os = "windows")]` macros. As a consequence, many of these files exhibit churn in this PR, since they weren't being formatted by `rustfmt` at all on main. - Again, to make the code more analyzable, this PR also removes some usages of `#[path = "cwd_junction.rs"]` in favor of a more standard module structure. The bin sidecar structure is still retained, but, e.g., `windows-sandbox-rs/src/bin/command_runner.rs` was moved to `windows-sandbox-rs/src/bin/command_runner/main.rs`, and so on. --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -65,6 +65,7 @@ mod models_cache_ttl;
|
||||
mod models_etag_responses;
|
||||
mod openai_file_mcp;
|
||||
mod otel;
|
||||
mod override_updates;
|
||||
mod pending_input;
|
||||
mod permissions_messages;
|
||||
mod personality;
|
||||
|
||||
@@ -1,25 +1,15 @@
|
||||
use anyhow::Result;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::COLLABORATION_MODE_CLOSE_TAG;
|
||||
use codex_protocol::protocol::COLLABORATION_MODE_OPEN_TAG;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
use codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use std::time::Duration;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn collab_mode_with_instructions(instructions: Option<&str>) -> CollaborationMode {
|
||||
@@ -33,77 +23,9 @@ fn collab_mode_with_instructions(instructions: Option<&str>) -> CollaborationMod
|
||||
}
|
||||
}
|
||||
|
||||
fn collab_xml(text: &str) -> String {
|
||||
format!("{COLLABORATION_MODE_OPEN_TAG}{text}{COLLABORATION_MODE_CLOSE_TAG}")
|
||||
}
|
||||
|
||||
async fn read_rollout_text(path: &Path) -> anyhow::Result<String> {
|
||||
for _ in 0..50 {
|
||||
if path.exists()
|
||||
&& let Ok(text) = std::fs::read_to_string(path)
|
||||
&& !text.trim().is_empty()
|
||||
{
|
||||
return Ok(text);
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
}
|
||||
Ok(std::fs::read_to_string(path)?)
|
||||
}
|
||||
|
||||
fn rollout_developer_texts(text: &str) -> Vec<String> {
|
||||
let mut texts = Vec::new();
|
||||
for line in text.lines() {
|
||||
let trimmed = line.trim();
|
||||
if trimmed.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let rollout: RolloutLine = match serde_json::from_str(trimmed) {
|
||||
Ok(rollout) => rollout,
|
||||
Err(_) => continue,
|
||||
};
|
||||
if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) =
|
||||
rollout.item
|
||||
&& role == "developer"
|
||||
{
|
||||
for item in content {
|
||||
if let ContentItem::InputText { text } = item {
|
||||
texts.push(text);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
texts
|
||||
}
|
||||
|
||||
fn rollout_environment_texts(text: &str) -> Vec<String> {
|
||||
let mut texts = Vec::new();
|
||||
for line in text.lines() {
|
||||
let trimmed = line.trim();
|
||||
if trimmed.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let rollout: RolloutLine = match serde_json::from_str(trimmed) {
|
||||
Ok(rollout) => rollout,
|
||||
Err(_) => continue,
|
||||
};
|
||||
if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) =
|
||||
rollout.item
|
||||
&& role == "user"
|
||||
{
|
||||
for item in content {
|
||||
if let ContentItem::InputText { text } = item
|
||||
&& text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG)
|
||||
{
|
||||
texts.push(text);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
texts
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn override_turn_context_without_user_turn_does_not_record_permissions_update() -> Result<()> {
|
||||
async fn override_turn_context_without_user_turn_does_not_record_permissions_update() -> Result<()>
|
||||
{
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -133,22 +55,17 @@ async fn override_turn_context_without_user_turn_does_not_record_permissions_upd
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await;
|
||||
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let rollout_text = read_rollout_text(&rollout_path).await?;
|
||||
let developer_texts = rollout_developer_texts(&rollout_text);
|
||||
let approval_texts: Vec<&String> = developer_texts
|
||||
.iter()
|
||||
.filter(|text| text.contains("`approval_policy`"))
|
||||
.collect();
|
||||
assert!(
|
||||
approval_texts.is_empty(),
|
||||
"did not expect permissions updates before a new user turn: {approval_texts:?}"
|
||||
!rollout_path.exists(),
|
||||
"did not expect a rollout before a new user turn"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn override_turn_context_without_user_turn_does_not_record_environment_update() -> Result<()> {
|
||||
async fn override_turn_context_without_user_turn_does_not_record_environment_update() -> Result<()>
|
||||
{
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -176,18 +93,17 @@ async fn override_turn_context_without_user_turn_does_not_record_environment_upd
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await;
|
||||
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let rollout_text = read_rollout_text(&rollout_path).await?;
|
||||
let env_texts = rollout_environment_texts(&rollout_text);
|
||||
assert!(
|
||||
env_texts.is_empty(),
|
||||
"did not expect environment updates before a new user turn: {env_texts:?}"
|
||||
!rollout_path.exists(),
|
||||
"did not expect a rollout before a new user turn"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn override_turn_context_without_user_turn_does_not_record_collaboration_update() -> Result<()> {
|
||||
async fn override_turn_context_without_user_turn_does_not_record_collaboration_update() -> Result<()>
|
||||
{
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -216,14 +132,10 @@ async fn override_turn_context_without_user_turn_does_not_record_collaboration_u
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await;
|
||||
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let rollout_text = read_rollout_text(&rollout_path).await?;
|
||||
let developer_texts = rollout_developer_texts(&rollout_text);
|
||||
let collab_text = collab_xml(collab_text);
|
||||
let collab_count = developer_texts
|
||||
.iter()
|
||||
.filter(|text| text.as_str() == collab_text.as_str())
|
||||
.count();
|
||||
assert_eq!(collab_count, 0);
|
||||
assert!(
|
||||
!rollout_path.exists(),
|
||||
"did not expect a rollout before a new user turn"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user