Files
codex/codex-rs/core/src/agent/guards.rs
T
Michael Bolin b77fe8fefe Apply argument comment lint across codex-rs (#14652)
## Why

Once the repo-local lint exists, `codex-rs` needs to follow the
checked-in convention and CI needs to keep it from drifting. This commit
applies the fallback `/*param*/` style consistently across existing
positional literal call sites without changing those APIs.

The longer-term preference is still to avoid APIs that require comments
by choosing clearer parameter types and call shapes. This PR is
intentionally the mechanical follow-through for the places where the
existing signatures stay in place.

After rebasing onto newer `main`, the rollout also had to cover newly
introduced `tui_app_server` call sites. That made it clear the first cut
of the CI job was too expensive for the common path: it was spending
almost as much time installing `cargo-dylint` and re-testing the lint
crate as a representative test job spends running product tests. The CI
update keeps the full workspace enforcement but trims that extra
overhead from ordinary `codex-rs` PRs.

## What changed

- keep a dedicated `argument_comment_lint` job in `rust-ci`
- mechanically annotate remaining opaque positional literals across
`codex-rs` with exact `/*param*/` comments, including the rebased
`tui_app_server` call sites that now fall under the lint
- keep the checked-in style aligned with the lint policy by using
`/*param*/` and leaving string and char literals uncommented
- cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
registry/git metadata in the lint job
- split changed-path detection so the lint crate's own `cargo test` step
runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
- continue to run the repo wrapper over the `codex-rs` workspace, so
product-code enforcement is unchanged

Most of the code changes in this commit are intentionally mechanical
comment rewrites or insertions driven by the lint itself.

## Verification

- `./tools/argument-comment-lint/run.sh --workspace`
- `cargo test -p codex-tui-app-server -p codex-tui`
- parsed `.github/workflows/rust-ci.yml` locally with PyYAML

---

* -> #14652
* #14651
2026-03-16 16:48:15 -07:00

231 lines
7.3 KiB
Rust

use crate::error::CodexErr;
use crate::error::Result;
use codex_protocol::ThreadId;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use rand::prelude::IndexedRandom;
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering;
/// This structure is used to add some limits on the multi-agent capabilities for Codex. In
/// the current implementation, it limits:
/// * Total number of sub-agents (i.e. threads) per user session
///
/// This structure is shared by all agents in the same user session (because the `AgentControl`
/// is).
#[derive(Default)]
pub(crate) struct Guards {
active_agents: Mutex<ActiveAgents>,
total_count: AtomicUsize,
}
#[derive(Default)]
struct ActiveAgents {
threads_set: HashSet<ThreadId>,
thread_agent_nicknames: HashMap<ThreadId, String>,
used_agent_nicknames: HashSet<String>,
nickname_reset_count: usize,
}
fn format_agent_nickname(name: &str, nickname_reset_count: usize) -> String {
match nickname_reset_count {
0 => name.to_string(),
reset_count => {
let value = reset_count + 1;
let suffix = match value % 100 {
11..=13 => "th",
_ => match value % 10 {
1 => "st", // codespell:ignore
2 => "nd", // codespell:ignore
3 => "rd", // codespell:ignore
_ => "th", // codespell:ignore
},
};
format!("{name} the {value}{suffix}")
}
}
}
fn session_depth(session_source: &SessionSource) -> i32 {
match session_source {
SessionSource::SubAgent(SubAgentSource::ThreadSpawn { depth, .. }) => *depth,
SessionSource::SubAgent(_) => 0,
_ => 0,
}
}
pub(crate) fn next_thread_spawn_depth(session_source: &SessionSource) -> i32 {
session_depth(session_source).saturating_add(1)
}
pub(crate) fn exceeds_thread_spawn_depth_limit(depth: i32, max_depth: i32) -> bool {
depth > max_depth
}
impl Guards {
pub(crate) fn reserve_spawn_slot(
self: &Arc<Self>,
max_threads: Option<usize>,
) -> Result<SpawnReservation> {
if let Some(max_threads) = max_threads {
if !self.try_increment_spawned(max_threads) {
return Err(CodexErr::AgentLimitReached { max_threads });
}
} else {
self.total_count.fetch_add(1, Ordering::AcqRel);
}
Ok(SpawnReservation {
state: Arc::clone(self),
active: true,
reserved_agent_nickname: None,
})
}
pub(crate) fn release_spawned_thread(&self, thread_id: ThreadId) {
let removed = {
let mut active_agents = self
.active_agents
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let removed = active_agents.threads_set.remove(&thread_id);
active_agents.thread_agent_nicknames.remove(&thread_id);
removed
};
if removed {
self.total_count.fetch_sub(1, Ordering::AcqRel);
}
}
fn register_spawned_thread(&self, thread_id: ThreadId, agent_nickname: Option<String>) {
let mut active_agents = self
.active_agents
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
active_agents.threads_set.insert(thread_id);
if let Some(agent_nickname) = agent_nickname {
active_agents
.used_agent_nicknames
.insert(agent_nickname.clone());
active_agents
.thread_agent_nicknames
.insert(thread_id, agent_nickname);
}
}
fn reserve_agent_nickname(&self, names: &[&str], preferred: Option<&str>) -> Option<String> {
let mut active_agents = self
.active_agents
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let agent_nickname = if let Some(preferred) = preferred {
preferred.to_string()
} else {
if names.is_empty() {
return None;
}
let available_names: Vec<String> = names
.iter()
.map(|name| format_agent_nickname(name, active_agents.nickname_reset_count))
.filter(|name| !active_agents.used_agent_nicknames.contains(name))
.collect();
if let Some(name) = available_names.choose(&mut rand::rng()) {
name.clone()
} else {
active_agents.used_agent_nicknames.clear();
active_agents.nickname_reset_count += 1;
if let Some(metrics) = codex_otel::metrics::global() {
let _ = metrics.counter(
"codex.multi_agent.nickname_pool_reset",
/*inc*/ 1,
&[],
);
}
format_agent_nickname(
names.choose(&mut rand::rng())?,
active_agents.nickname_reset_count,
)
}
};
active_agents
.used_agent_nicknames
.insert(agent_nickname.clone());
Some(agent_nickname)
}
fn try_increment_spawned(&self, max_threads: usize) -> bool {
let mut current = self.total_count.load(Ordering::Acquire);
loop {
if current >= max_threads {
return false;
}
match self.total_count.compare_exchange_weak(
current,
current + 1,
Ordering::AcqRel,
Ordering::Acquire,
) {
Ok(_) => return true,
Err(updated) => current = updated,
}
}
}
}
pub(crate) struct SpawnReservation {
state: Arc<Guards>,
active: bool,
reserved_agent_nickname: Option<String>,
}
impl SpawnReservation {
pub(crate) fn reserve_agent_nickname(&mut self, names: &[&str]) -> Result<String> {
self.reserve_agent_nickname_with_preference(names, /*preferred*/ None)
}
pub(crate) fn reserve_agent_nickname_with_preference(
&mut self,
names: &[&str],
preferred: Option<&str>,
) -> Result<String> {
let agent_nickname = self
.state
.reserve_agent_nickname(names, preferred)
.ok_or_else(|| {
CodexErr::UnsupportedOperation("no available agent nicknames".to_string())
})?;
self.reserved_agent_nickname = Some(agent_nickname.clone());
Ok(agent_nickname)
}
pub(crate) fn commit(self, thread_id: ThreadId) {
self.commit_with_agent_nickname(thread_id, /*agent_nickname*/ None);
}
pub(crate) fn commit_with_agent_nickname(
mut self,
thread_id: ThreadId,
agent_nickname: Option<String>,
) {
let agent_nickname = self.reserved_agent_nickname.take().or(agent_nickname);
self.state
.register_spawned_thread(thread_id, agent_nickname);
self.active = false;
}
}
impl Drop for SpawnReservation {
fn drop(&mut self) {
if self.active {
self.state.total_count.fetch_sub(1, Ordering::AcqRel);
}
}
}
#[cfg(test)]
#[path = "guards_tests.rs"]
mod tests;