Receipt · 62a6fd05-0
Comment misrepresents safety of mutating ALLOWED_ORIGINS_ENV during parallel test runs
The finding
- src/core/jsonrpc_cors_tests.rs:80-104
- src/core/jsonrpc_cors_tests.rs:21-47
The comment claims parallel runs are safe because no other test reads ALLOWED_ORIGINS_ENV. However, the other tests in this file (`allows_tauri_webview_origins`, `allows_loopback_with_any_port`, `rejects_disallowed_origins`, `always_sets_methods_headers_and_max_age`) all call `is_origin_allowed(...)`, which by its very name and the existence of the ALLOWED_ORIGINS_ENV constant almost certainly consults that env var when computing the allowlist. Cargo's default test harness runs tests in parallel threads within the same process, and `std::env::set_var` mutates process-global state. While `env_override_allows_extra_origins` is running with the env var set to `https://debug.internal, http://harness:9000`, a concurrently executing test like `allows_tauri_webview_origins` could observe that allowlist (or its restoration race) and either fail or, worse, pass for the wrong reason. The comment's claim of safety is therefore deceptive and the test is genuinely flaky. Additionally, on Rust 2024 `std::env::set_var` is `unsafe` precisely because it is not thread-safe on Unix (races with getenv in other threads / libc), so the `SAFETY` comment is technically incorrect about the soundness invariant being upheld.
Fix
Either (a) serialize env-mutating tests using a `Mutex`/`serial_test` crate and document that all tests touching ALLOWED_ORIGINS_ENV must take the lock, (b) gate this test behind `#[ignore]` + a dedicated single-threaded test binary, or (c) refactor `is_origin_allowed` to take the allowlist as a parameter so tests don't need to mutate process-global state. Fix the misleading SAFETY comment so it doesn't assert thread-safety that doesn't hold.
Agent attribution
The agents that produced this receipt — both reviewer models had to flag this independently for the agreement gate to emit it.
anthropic
gpt-5
51.9s · error
openai
claude-opus-4-7
205.4s · error
Total
wall-clock review time · est. inference cost
205.4s · $0.40
Sweeper
closed at SHA
still open
internal review id · 62a6fd05
Third-party witnesses
Everything below lives on GitHub's event log, not ours. Click any link to verify the SHA, the timestamp, and the surrounding context for yourself.
Original review comment
https://github.com/AntFleet/agent-openhuman-bench/pull/1#issuecomment-4494694855