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
7 changes: 6 additions & 1 deletion cmux-tui/crates/acpmux/src/hub/steer_end_race_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ async fn unsent_steer_after_completion(steer_only: bool) {
use crate::config::{Config, StoreMode};
let fake = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/fake_agent.py");
let mut cfg: Config = serde_json::from_value(json!({
"harnesses": {"fake": {"argv": ["/usr/bin/env", "FAKE_CODEX_STEER=1", "python3", fake]}},
// At normal CPU class (`cpu_class.rs`), as in the hermetic integration tests: at
// background class a loaded fleet Mac did not end the stand-in's steer turn in 20 s.
"harnesses": {"fake": {
"argv": ["/usr/bin/env", "FAKE_CODEX_STEER=1", "python3", fake],
"env": {"ACPMUX_SESSION_CPU_CLASS": "normal"},
}},
"defaultHarness": "fake",
}))
.unwrap();
Expand Down
10 changes: 8 additions & 2 deletions cmux-tui/crates/acpmux/tests/cpu_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@ mod daemon_env;
// agent hosts this process starts in background class (`qos.rs`), so a hosted harness would
// inherit it: the hosted test checks that its harness clears it at its exec. The in-process hubs
// start their harnesses directly and read no `ACPMUX_QOS`.
const HERMETIC: daemon_env::Process = daemon_env::Process::stand_in()
.with_env(&[("ACPMUX_TEST_CPU_CLASS_REF", "normal"), ("ACPMUX_QOS", "background")]);
// The hermetic environment keeps stand-ins at normal class (`daemon_env::vars`); these tests
// check the product's background class, so this process and its daemons ask for it.
const HERMETIC: daemon_env::Process = daemon_env::Process::stand_in().with_env(&[
("ACPMUX_TEST_CPU_CLASS_REF", "normal"),
("ACPMUX_QOS", "background"),
("ACPMUX_SESSION_CPU_CLASS", "background"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Use the acpmux::cpu_class::ENV constant here, as the new RealDaemon::cmd line does. A literal would go stale silently if the variable is renamed.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/acpmux/tests/cpu_class.rs, line 28:

<comment>Use the `acpmux::cpu_class::ENV` constant here, as the new `RealDaemon::cmd` line does. A literal would go stale silently if the variable is renamed.</comment>

<file context>
@@ -20,8 +20,13 @@ mod daemon_env;
+const HERMETIC: daemon_env::Process = daemon_env::Process::stand_in().with_env(&[
+    ("ACPMUX_TEST_CPU_CLASS_REF", "normal"),
+    ("ACPMUX_QOS", "background"),
+    ("ACPMUX_SESSION_CPU_CLASS", "background"),
+]);
 
</file context>
Suggested change
("ACPMUX_SESSION_CPU_CLASS", "background"),
(acpmux::cpu_class::ENV, "background"),

]);

use acpmux::agent::{Attached, ChildAgent};
use acpmux::agent_host::link::{self, HostLauncher};
Expand Down Expand Up @@ -427,6 +432,7 @@ impl RealDaemon {
.env("ACPMUX_HEALTH_FETCH", "0")
.env("ACPMUX_CATALOG_FETCH", "0")
.env("ACPMUX_PROBE_HARNESSES", "fake")
.env(acpmux::cpu_class::ENV, "background")
.env_remove("ACPMUX_SESSION_ID")
.stdin(std::process::Stdio::null());
cmd
Expand Down
12 changes: 12 additions & 0 deletions cmux-tui/crates/acpmux/tests/hub_integration/stop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,3 +705,15 @@ async fn a_long_replayed_history_stays_out_of_the_resumed_sessions_last_message(
let summary = hub.session_summary(&hub.resolve(&id).unwrap());
assert_eq!(summary["lastMessage"], json!({"role": "agent", "text": "echo: second"}));
}

/// A test's stand-in harness runs at normal CPU class once its start is over, not at the
/// product's background default (`cpu_class.rs`). The stand-ins time their turns with short
/// sleeps (a Codex-steer turn polls 300 x 10 ms for a joined prompt), and on a loaded fleet Mac
/// (load 13 to 21 on 10 performance cores) a background-class stand-in did not end that turn in
/// 18 to 30 s: this test, the forum's steered rule and the steer race failed only on macOS.
/// `tests/cpu_class.rs` asks for background itself and checks the product's class.
#[cfg(target_os = "macos")]
#[test]
fn a_stand_in_harness_runs_at_normal_class_in_tests() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This test checks only the resolved CPU class for the test process's env, not the class of a running stand-in harness. Rename it to describe the resolution (for example the_test_environment_resolves_cpu_class_to_normal), or add a real priority check.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/acpmux/tests/hub_integration/stop.rs, line 717:

<comment>This test checks only the resolved CPU class for the test process's env, not the class of a running stand-in harness. Rename it to describe the resolution (for example `the_test_environment_resolves_cpu_class_to_normal`), or add a real priority check.</comment>

<file context>
@@ -705,3 +705,15 @@ async fn a_long_replayed_history_stays_out_of_the_resumed_sessions_last_message(
+/// `tests/cpu_class.rs` asks for background itself and checks the product's class.
+#[cfg(target_os = "macos")]
+#[test]
+fn a_stand_in_harness_runs_at_normal_class_in_tests() {
+    assert_eq!(acpmux::cpu_class::CpuClass::for_session([]), acpmux::cpu_class::CpuClass::Normal);
+}
</file context>
Suggested change
fn a_stand_in_harness_runs_at_normal_class_in_tests() {
fn the_test_environment_resolves_cpu_class_to_normal() {

assert_eq!(acpmux::cpu_class::CpuClass::for_session([]), acpmux::cpu_class::CpuClass::Normal);
}
7 changes: 7 additions & 0 deletions cmux-tui/crates/acpmux/tests/support/daemon_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
//! daemon's home or a chat's folder, and the daemon refuses to start an
//! agent in the user's home folder unless a person picks it;
//! - `TMPDIR`: the test process's temp directory (macOS: the per-user one);
//! - `ACPMUX_SESSION_CPU_CLASS=normal`: a stand-in harness keeps its normal CPU class (see
//! [`vars`]);
//! - `ACPMUX_AGENT_LAUNCH=deny`: no agent starts at all (cx-9ettq). A test
//! that drives a stand-in agent (fake_agent.py, fake_claude.py) opts out
//! with [`Agents::StandIn`]; it keeps the bare PATH, so only its stand-in
Expand Down Expand Up @@ -293,6 +295,11 @@ pub fn vars(scratch: &Path, agents: Agents, tools: &[&str]) -> Vec<(String, Stri
if agents == Agents::Deny {
env.push(("ACPMUX_AGENT_LAUNCH".into(), "deny".into()));
}
// Stand-in harnesses at normal CPU class once their start is over, not the product's
// background default (`cpu_class.rs`): they time their turns with short sleeps, and at
// background class on a loaded fleet Mac (load 13 to 21 on 10 performance cores) a Codex-steer
// turn did not end in 18 to 30 s. `tests/cpu_class.rs` sets background itself.
env.push((acpmux::cpu_class::ENV.into(), "normal".into()));
env
}

Expand Down
Loading