From 3062345125cd439a5b76811ff0c74e7ccfbcb037 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 30 Mar 2026 08:50:31 -0700 Subject: [PATCH] fix: address coverage gap in argument-comment-lint --- .bazelrc | 1 + codex-rs/core/src/shell_snapshot_tests.rs | 12 ++++++-- codex-rs/linux-sandbox/src/landlock.rs | 32 ++++++++++++++++----- codex-rs/linux-sandbox/src/proxy_routing.rs | 3 +- tools/argument-comment-lint/README.md | 4 ++- 5 files changed, 40 insertions(+), 12 deletions(-) diff --git a/.bazelrc b/.bazelrc index b9275426fa..91ae7915e7 100644 --- a/.bazelrc +++ b/.bazelrc @@ -80,6 +80,7 @@ build:clippy --@rules_rust//rust/settings:clippy.toml=//codex-rs:clippy.toml # Shared config for Bazel-backed argument-comment-lint. build:argument-comment-lint --aspects=//tools/argument-comment-lint:lint_aspect.bzl%rust_argument_comment_lint_aspect build:argument-comment-lint --output_groups=argument_comment_lint_checks +build:argument-comment-lint --build_manual_tests build:argument-comment-lint --@rules_rust//rust/toolchain/channel=nightly # Rearrange caches on Windows so they're on the same volume as the checkout. diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index 90288300ff..ff700ff7a6 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -313,9 +313,15 @@ async fn timed_out_snapshot_shell_is_terminated() -> Result<()> { shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; - let err = run_script_with_timeout(&shell, &script, Duration::from_secs(1), true, dir.path()) - .await - .expect_err("snapshot shell should time out"); + let err = run_script_with_timeout( + &shell, + &script, + Duration::from_secs(1), + /*use_login_shell*/ true, + dir.path(), + ) + .await + .expect_err("snapshot shell should time out"); assert!( err.to_string().contains("timed out"), "expected timeout error, got {err:?}" diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index 307f956a4d..d257edf933 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -274,7 +274,10 @@ mod tests { #[test] fn managed_network_enforces_seccomp_even_for_full_network_policy() { assert_eq!( - should_install_network_seccomp(NetworkSandboxPolicy::Enabled, true), + should_install_network_seccomp( + NetworkSandboxPolicy::Enabled, + /*allow_network_for_proxy*/ true + ), true ); } @@ -282,7 +285,10 @@ mod tests { #[test] fn full_network_policy_without_managed_network_skips_seccomp() { assert_eq!( - should_install_network_seccomp(NetworkSandboxPolicy::Enabled, false), + should_install_network_seccomp( + NetworkSandboxPolicy::Enabled, + /*allow_network_for_proxy*/ false + ), false ); } @@ -291,18 +297,22 @@ mod tests { fn restricted_network_policy_always_installs_seccomp() { assert!(should_install_network_seccomp( NetworkSandboxPolicy::Restricted, - false + /*allow_network_for_proxy*/ false )); assert!(should_install_network_seccomp( NetworkSandboxPolicy::Restricted, - true + /*allow_network_for_proxy*/ true )); } #[test] fn managed_proxy_routes_use_proxy_routed_seccomp_mode() { assert_eq!( - network_seccomp_mode(NetworkSandboxPolicy::Enabled, true, true), + network_seccomp_mode( + NetworkSandboxPolicy::Enabled, + /*allow_network_for_proxy*/ true, + /*proxy_routed_network*/ true + ), Some(NetworkSeccompMode::ProxyRouted) ); } @@ -310,7 +320,11 @@ mod tests { #[test] fn restricted_network_without_proxy_routing_uses_restricted_mode() { assert_eq!( - network_seccomp_mode(NetworkSandboxPolicy::Restricted, false, false), + network_seccomp_mode( + NetworkSandboxPolicy::Restricted, + /*allow_network_for_proxy*/ false, + /*proxy_routed_network*/ false + ), Some(NetworkSeccompMode::Restricted) ); } @@ -318,7 +332,11 @@ mod tests { #[test] fn full_network_without_managed_proxy_skips_network_seccomp_mode() { assert_eq!( - network_seccomp_mode(NetworkSandboxPolicy::Enabled, false, false), + network_seccomp_mode( + NetworkSandboxPolicy::Enabled, + /*allow_network_for_proxy*/ false, + /*proxy_routed_network*/ false + ), None ); } diff --git a/codex-rs/linux-sandbox/src/proxy_routing.rs b/codex-rs/linux-sandbox/src/proxy_routing.rs index f57472c8a1..07e1893ee3 100644 --- a/codex-rs/linux-sandbox/src/proxy_routing.rs +++ b/codex-rs/linux-sandbox/src/proxy_routing.rs @@ -718,7 +718,8 @@ mod tests { #[test] fn rewrites_proxy_url_to_local_loopback_port() { let rewritten = - rewrite_proxy_env_value("socks5h://127.0.0.1:8081", 43210).expect("rewritten value"); + rewrite_proxy_env_value("socks5h://127.0.0.1:8081", /*local_port*/ 43210) + .expect("rewritten value"); assert_eq!(rewritten, "socks5h://127.0.0.1:43210"); } diff --git a/tools/argument-comment-lint/README.md b/tools/argument-comment-lint/README.md index 78e28f308f..49efd344e1 100644 --- a/tools/argument-comment-lint/README.md +++ b/tools/argument-comment-lint/README.md @@ -138,7 +138,9 @@ If no package selection is provided, `just argument-comment-lint` now defaults to the Bazel aspect path over `//codex-rs/...`. The Python wrappers remain the package-scoped escape hatch and still default the underlying Cargo invocation to `--all-targets` unless you explicitly narrow the target set, so targeted -wrapper runs cover test-only call sites by default. +wrapper runs cover test-only call sites by default. The Bazel config also sets +`--build_manual_tests` so wildcard lint runs include the internal +`*-unit-tests-bin` Rust targets and catch inline `#[cfg(test)]` call sites. Repo runs also promote `uncommented_anonymous_literal_argument` to an error by default: