Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 125 additions & 0 deletions .github/scripts/verify_cargo_workspace_manifests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
#!/usr/bin/env python3

"""Verify that codex-rs crates inherit workspace metadata, lints, and names.

This keeps `cargo clippy` aligned with the workspace lint policy by ensuring
each crate opts into `[lints] workspace = true`, and it also checks the crate
name conventions for top-level `codex-rs/*` crates and `codex-rs/utils/*`
crates.
"""

from __future__ import annotations

import sys
import tomllib
from pathlib import Path


ROOT = Path(__file__).resolve().parents[2]
CARGO_RS_ROOT = ROOT / "codex-rs"
WORKSPACE_PACKAGE_FIELDS = ("version", "edition", "license")
TOP_LEVEL_NAME_EXCEPTIONS = {
"windows-sandbox-rs": "codex-windows-sandbox",
}
UTILITY_NAME_EXCEPTIONS = {
"path-utils": "codex-utils-path",
}


def main() -> int:
failures = [
(path.relative_to(ROOT), errors)
for path in cargo_manifests()
if (errors := manifest_errors(path))
]
if not failures:
return 0

print(
"Cargo manifests under codex-rs must inherit workspace package metadata and "
"opt into workspace lints."
)
print(
"Cargo only applies `codex-rs/Cargo.toml` `[workspace.lints.clippy]` "
"entries to a crate when that crate declares:"
)
print()
print("[lints]")
print("workspace = true")
print()
print(
"Without that opt-in, `cargo clippy` can miss violations that Bazel clippy "
"catches."
)
print()
print(
"Package-name checks apply to `codex-rs/<crate>/Cargo.toml` and "
"`codex-rs/utils/<crate>/Cargo.toml`."
)
print()
for path, errors in failures:
print(f"{path}:")
for error in errors:
print(f" - {error}")

return 1


def manifest_errors(path: Path) -> list[str]:
manifest = load_manifest(path)
package = manifest.get("package")
if not isinstance(package, dict):
return []

errors = []
for field in WORKSPACE_PACKAGE_FIELDS:
if not is_workspace_reference(package.get(field)):
errors.append(f"set `{field}.workspace = true` in `[package]`")

lints = manifest.get("lints")
if not (isinstance(lints, dict) and lints.get("workspace") is True):
errors.append("add `[lints]` with `workspace = true`")

expected_name = expected_package_name(path)
if expected_name is not None:
actual_name = package.get("name")
if actual_name != expected_name:
errors.append(
f"set `[package].name` to `{expected_name}` (found `{actual_name}`)"
)

return errors


def expected_package_name(path: Path) -> str | None:
parts = path.relative_to(CARGO_RS_ROOT).parts
if len(parts) == 2 and parts[1] == "Cargo.toml":
directory = parts[0]
return TOP_LEVEL_NAME_EXCEPTIONS.get(
directory,
directory if directory.startswith("codex-") else f"codex-{directory}",
)
if len(parts) == 3 and parts[0] == "utils" and parts[2] == "Cargo.toml":
directory = parts[1]
return UTILITY_NAME_EXCEPTIONS.get(directory, f"codex-utils-{directory}")
return None


def is_workspace_reference(value: object) -> bool:
return isinstance(value, dict) and value.get("workspace") is True


def load_manifest(path: Path) -> dict:
return tomllib.loads(path.read_text())


def cargo_manifests() -> list[Path]:
return sorted(
path
for path in CARGO_RS_ROOT.rglob("Cargo.toml")
if path != CARGO_RS_ROOT / "Cargo.toml"
)


if __name__ == "__main__":
sys.exit(main())
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ jobs:
- name: Checkout repository
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6

- name: Verify codex-rs Cargo manifests inherit workspace settings
run: python3 .github/scripts/verify_cargo_workspace_manifests.py

- name: Setup pnpm
uses: pnpm/action-setup@a8198c4bff370c8506180b035930dea56dbd5288 # v5
with:
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/ansi-escape/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ license.workspace = true
name = "codex_ansi_escape"
path = "src/lib.rs"

[lints]
workspace = true

[dependencies]
ansi-to-tui = { workspace = true }
ratatui = { workspace = true, features = [
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/app-server/tests/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ license.workspace = true
[lib]
path = "lib.rs"

[lints]
workspace = true

[dependencies]
anyhow = { workspace = true }
base64 = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/app-server/tests/common/models_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
},
supported_in_api: preset.supported_in_api,
priority,
upgrade: preset.upgrade.as_ref().map(|u| u.into()),
upgrade: preset.upgrade.as_ref().map(Into::into),
base_instructions: "base instructions".to_string(),
model_messages: None,
supports_reasoning_summaries: false,
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/backend-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ publish = false
[lib]
path = "src/lib.rs"

[lints]
workspace = true

[dependencies]
anyhow = "1"
serde = { version = "1", features = ["derive"] }
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/codex-backend-openapi-models/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ license.workspace = true
name = "codex_backend_openapi_models"
path = "src/lib.rs"

[lints]
workspace = true

# Important: generated code often violates our workspace lints.
# Allow unwrap/expect in this crate so the workspace builds cleanly
# after models are regenerated.
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/core/tests/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ license.workspace = true
[lib]
path = "lib.rs"

[lints]
workspace = true

[dependencies]
anyhow = { workspace = true }
assert_cmd = { workspace = true }
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/debug-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ version.workspace = true
edition.workspace = true
license.workspace = true

[lints]
workspace = true

[dependencies]
anyhow.workspace = true
clap = { workspace = true, features = ["derive"] }
Expand Down
1 change: 1 addition & 0 deletions codex-rs/debug-client/src/client.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::expect_used)]
use std::io::BufRead;
use std::io::BufReader;
use std::io::Write;
Expand Down
1 change: 1 addition & 0 deletions codex-rs/debug-client/src/output.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::expect_used)]
use std::io;
use std::io::IsTerminal;
use std::io::Write;
Expand Down
3 changes: 2 additions & 1 deletion codex-rs/debug-client/src/reader.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::expect_used)]
use std::io::BufRead;
use std::io::BufReader;
use std::process::ChildStdout;
Expand Down Expand Up @@ -113,7 +114,7 @@ fn handle_server_request(
stdin: &Arc<Mutex<Option<std::process::ChildStdin>>>,
output: &Output,
) -> anyhow::Result<()> {
let server_request = match ServerRequest::try_from(request.clone()) {
let server_request = match ServerRequest::try_from(request) {
Ok(server_request) => server_request,
Err(_) => return Ok(()),
};
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/feedback/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ version.workspace = true
edition.workspace = true
license.workspace = true

[lints]
workspace = true

[dependencies]
anyhow = { workspace = true }
codex-protocol = { workspace = true }
Expand Down
5 changes: 4 additions & 1 deletion codex-rs/feedback/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,12 @@ impl CodexFeedback {

pub fn snapshot(&self, session_id: Option<ThreadId>) -> FeedbackSnapshot {
let bytes = {
#[allow(clippy::expect_used)]
let guard = self.inner.ring.lock().expect("mutex poisoned");
guard.snapshot_bytes()
};
let tags = {
#[allow(clippy::expect_used)]
let guard = self.inner.tags.lock().expect("mutex poisoned");
guard.clone()
};
Expand Down Expand Up @@ -324,7 +326,7 @@ impl FeedbackSnapshot {
use sentry::protocol::Values;

event.exception = Values::from(vec![Exception {
ty: title.clone(),
ty: title,
value: Some(r.to_string()),
..Default::default()
}]);
Expand Down Expand Up @@ -430,6 +432,7 @@ where
return;
}

#[allow(clippy::expect_used)]
let mut guard = self.inner.tags.lock().expect("mutex poisoned");
for (key, value) in visitor.tags {
if guard.len() >= MAX_FEEDBACK_TAGS && !guard.contains_key(&key) {
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/file-search/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ path = "src/main.rs"
name = "codex_file_search"
path = "src/lib.rs"

[lints]
workspace = true

[dependencies]
anyhow = { workspace = true }
clap = { workspace = true, features = ["derive"] }
Expand Down
14 changes: 10 additions & 4 deletions codex-rs/file-search/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,10 @@ pub fn create_session(
threads: threads.get(),
compute_indices,
respect_gitignore,
cancelled: cancelled.clone(),
cancelled,
shutdown: Arc::new(AtomicBool::new(false)),
reporter,
work_tx: work_tx.clone(),
work_tx,
});

let matcher_inner = inner.clone();
Expand Down Expand Up @@ -611,13 +611,14 @@ struct RunReporter {

impl SessionReporter for RunReporter {
fn on_update(&self, snapshot: &FileSearchSnapshot) {
#[expect(clippy::unwrap_used)]
#[allow(clippy::unwrap_used)]
let mut guard = self.snapshot.write().unwrap();
*guard = snapshot.clone();
}

fn on_complete(&self) {
let (cv, mutex) = &self.completed;
#[allow(clippy::unwrap_used)]
let mut completed = mutex.lock().unwrap();
*completed = true;
cv.notify_all();
Expand All @@ -627,10 +628,15 @@ impl SessionReporter for RunReporter {
impl RunReporter {
fn wait_for_complete(&self) -> FileSearchSnapshot {
let (cv, mutex) = &self.completed;
#[allow(clippy::unwrap_used)]
let mut completed = mutex.lock().unwrap();
while !*completed {
completed = cv.wait(completed).unwrap();
#[allow(clippy::unwrap_used)]
{
completed = cv.wait(completed).unwrap();
}
}
#[allow(clippy::unwrap_used)]
self.snapshot.read().unwrap().clone()
}
}
Expand Down
9 changes: 7 additions & 2 deletions codex-rs/file-search/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ struct StdioReporter {
impl Reporter for StdioReporter {
fn report_match(&self, file_match: &FileMatch) {
if self.write_output_as_json {
println!("{}", serde_json::to_string(&file_match).unwrap());
#[allow(clippy::unwrap_used)]
let json = serde_json::to_string(file_match).unwrap();
println!("{json}");
} else if self.show_indices {
#[allow(clippy::expect_used)]
let indices = file_match
.indices
.as_ref()
Expand Down Expand Up @@ -61,7 +64,9 @@ impl Reporter for StdioReporter {
fn warn_matches_truncated(&self, total_match_count: usize, shown_match_count: usize) {
if self.write_output_as_json {
let value = json!({"matches_truncated": true});
println!("{}", serde_json::to_string(&value).unwrap());
#[allow(clippy::unwrap_used)]
let json = serde_json::to_string(&value).unwrap();
println!("{json}");
} else {
eprintln!(
"Warning: showing {shown_match_count} out of {total_match_count} results. Provide a more specific pattern or increase the --limit.",
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/mcp-server/tests/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ license.workspace = true
[lib]
path = "lib.rs"

[lints]
workspace = true

[dependencies]
anyhow = { workspace = true }
codex-core = { workspace = true }
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/network-proxy/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "codex-network-proxy"
edition = "2024"
version = { workspace = true }
edition.workspace = true
version.workspace = true
license.workspace = true

[lib]
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/shell-escalation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ version.workspace = true
name = "codex-execve-wrapper"
path = "src/bin/main_execve_wrapper.rs"

[lints]
workspace = true

[dependencies]
anyhow = { workspace = true }
async-trait = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/shell-escalation/src/unix/escalate_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ mod tests {
let execve_wrapper_str = execve_wrapper.to_string_lossy().to_string();
let server = EscalateServer::new(
PathBuf::from("/bin/zsh"),
execve_wrapper.clone(),
execve_wrapper,
DeterministicEscalationPolicy {
decision: EscalationDecision::run(),
},
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/shell-escalation/src/unix/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fn extract_fds(control: &[u8]) -> Vec<OwnedFd> {
let fd_count: usize = {
// `cmsghdr::cmsg_len` is not typed consistently across targets, so normalize it
// before doing the size arithmetic.
#[allow(clippy::useless_conversion)]
#[allow(clippy::useless_conversion, clippy::expect_used)]
let cmsg_data_len = usize::try_from(unsafe { (*cmsg).cmsg_len })
.expect("cmsghdr length fits")
- unsafe { libc::CMSG_LEN(0) as usize };
Expand Down
Loading
Loading