Skip to content

chore: enable await-holding clippy lints#18698

Merged
bolinfest merged 1 commit into
mainfrom
pr18698
Apr 21, 2026
Merged

chore: enable await-holding clippy lints#18698
bolinfest merged 1 commit into
mainfrom
pr18698

Conversation

@bolinfest

@bolinfest bolinfest commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #18178, where we said the await-holding clippy rule would be enabled separately.

Enable await_holding_lock and await_holding_invalid_type after the preceding commits fixed or explicitly documented the current offenders.

@bolinfest bolinfest requested a review from a team as a code owner April 20, 2026 16:54
@bolinfest bolinfest changed the base branch from main to pr18423 April 20, 2026 16:54
bolinfest added a commit that referenced this pull request Apr 20, 2026
This is the second cleanup in the await-holding lint stack. The
higher-level goal, following #18178
and #18398, is to enable Clippy
coverage for guards held across `.await` points without carrying broad
suppressions.

The stack is working toward enabling Clippy's
[`await_holding_lock`](https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock)
lint and the configurable
[`await_holding_invalid_type`](https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_invalid_type)
lint for Tokio guard types.

Several existing fields used `tokio::sync::Mutex<()>` only as
one-at-a-time async gates. Those guards intentionally lived across
`.await` while an operation was serialized. A mutex over `()` suggests
protected data and trips the await-holding lint shape; a single-permit
`tokio::sync::Semaphore` expresses the intended serialization directly.

## What changed

- Replace `Mutex<()>` serialization gates with `Semaphore::new(1)` for
agent identity ensure, exec policy updates, guardian review session
reuse, plugin remote sync, managed network proxy refresh, auth token
refresh, and RMCP session recovery.
- Update call sites from `lock().await` / `try_lock()` to
`acquire().await` / `try_acquire()`.
- Map closed-semaphore errors into the existing local error types, even
though these semaphores are owned for the lifetime of their managers.
- Update session test builders for the new
`managed_network_proxy_refresh_lock` type.

## Verification

- The split stack was verified at the final lint-enabling head with
`just clippy`.





---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18403).
* #18698
* #18423
* #18418
* __->__ #18403
@bolinfest bolinfest force-pushed the pr18423 branch 2 times, most recently from 0e2580d to feaccc4 Compare April 20, 2026 20:17
@bolinfest bolinfest force-pushed the pr18698 branch 2 times, most recently from 2dd173f to c20e6c3 Compare April 21, 2026 00:00
@bolinfest bolinfest force-pushed the pr18423 branch 2 times, most recently from e029db9 to a7cecda Compare April 21, 2026 00:13
@bolinfest bolinfest force-pushed the pr18698 branch 2 times, most recently from 5b5a9bc to ff04162 Compare April 21, 2026 00:17
bolinfest added a commit that referenced this pull request Apr 21, 2026
## Why

This is part of the follow-up work from #18178 to make Codex ready for
Clippy's
[`await_holding_lock`](https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock)
/
[`await_holding_invalid_type`](https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_invalid_type)
lints.

This bottom PR keeps the scope intentionally small:
`NetworkProxyState::record_blocked()` only needs the state write lock
while it mutates the blocked-request ring buffer and counters. The debug
log payload and `BlockedRequestObserver` callback can be produced after
that lock is released.

## What changed

- Copies the blocked-request snapshot values needed for logging while
updating the state.
- Releases the `RwLockWriteGuard` before logging or notifying the
observer.

## Verification

- `cargo test -p codex-network-proxy`


---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18418).
* #18698
* #18423
* __->__ #18418
bolinfest added a commit that referenced this pull request Apr 21, 2026
## Why

This PR prepares the stack to enable Clippy await-holding lints that
were left disabled in #18178. The mechanical lock-scope cleanup is
handled separately; this PR is the documentation/configuration layer for
the remaining await-across-guard sites.

Without explicit annotations, reviewers and future maintainers cannot
tell whether an await-holding warning is a real concurrency smell or an
intentional serialization boundary.

## What changed

- Configures `clippy.toml` so `await_holding_invalid_type` also covers
`tokio::sync::{MutexGuard,RwLockReadGuard,RwLockWriteGuard}`.
- Adds targeted `#[expect(clippy::await_holding_invalid_type, reason =
...)]` annotations for intentional async guard lifetimes.
- Documents the main categories of intentional cases: active-turn state
transitions that must remain atomic, session-owned MCP manager accesses,
remote-control websocket serialization, JS REPL kernel/process
serialization, OAuth persistence, external bearer token refresh
serialization, and tests that intentionally serialize shared global or
session-owned state.
- For external bearer token refresh, documents the existing
serialization boundary: holding `cached_token` across the provider
command prevents concurrent cache misses from starting duplicate refresh
commands, and the current behavior is small enough that an explicit
expectation is easier to maintain than adding another synchronization
primitive.

## Verification

- `cargo clippy -p codex-login --all-targets`
- `cargo clippy -p codex-connectors --all-targets`
- `cargo clippy -p codex-core --all-targets`
- The follow-up PR #18698 enables `await_holding_invalid_type` and
`await_holding_lock` as workspace `deny` lints, so any undocumented
remaining offender will fail Clippy.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18423).
* #18698
* __->__ #18423
Base automatically changed from pr18423 to main April 21, 2026 05:41
Follow-up to #18178, where we said the await-holding clippy rule would be enabled separately.

Enable `await_holding_lock` and `await_holding_invalid_type` after the preceding commits fixed or explicitly documented the current offenders.
@bolinfest bolinfest enabled auto-merge (squash) April 21, 2026 05:46
@bolinfest bolinfest merged commit 1dcea72 into main Apr 21, 2026
43 of 69 checks passed
@bolinfest bolinfest deleted the pr18698 branch April 21, 2026 06:06
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants