feat: pu bench / pu play — suspend and resume agents

[2witstudios]

Summary

• Adds pu bench <agent_id> and pu bench --all to suspend (bench) agents
• Adds pu play <agent_id> to resume a benched agent
• Enhances pu status to show "benched" (yellow) for suspended agents instead of generic "waiting"
• Uses PurePoint's basketball metaphor: bench a player, put them back in play

The suspend/resume protocol (Request::Suspend/Request::Resume) and engine handlers were already fully implemented but had no CLI commands. This wires them up with the right terminology.

What it looks like

$ pu bench ag-abc123
Benched 1 agent(s)
  ag-abc123

$ pu status
Worktree wt-xyz (pu/auth) — Active
  ID             NAME             STATUS
  ag-abc123      claude           benched

$ pu play ag-abc123
Back in play: ag-abc123 (streaming)

Test plan

• All 410 existing tests pass
• 6 new tests for benched status display
• cargo clippy clean
• cargo fmt clean
• Manual: pu bench --help / pu play --help show correct usage
• Manual: bench a running agent, verify status shows "benched"
• Manual: play a benched agent, verify it resumes

Review feedback addressed

• Self-protection: refuse to bench own agent (single target), warn on --all self-inclusion (7b8acac)
• conflicts_with for mutual exclusivity of <agent_id> and --all (953fec0)
• Docstrings and long_about help text documenting --all behavior (c7a1b9f)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR introduces bench and play CLI commands for suspending and resuming agent execution, plus command dispatch wiring, a new command module, and output/status updates to display bench/resume results.

Changes

Cohort / File(s)	Summary
Bench Command Implementation crates/pu-cli/src/commands/bench.rs	Adds run_bench() and run_play() handlers: build/validate target (All or Agent), ensure daemon, send Suspend / Resume requests, handle responses, print results, and warn if self-agent is included.
Command Module Registration crates/pu-cli/src/commands/mod.rs	Adds pub mod bench; to expose the new commands module.
Command Dispatch and Enum crates/pu-cli/src/main.rs	Adds Bench and Play variants to Commands enum and dispatches them to bench::run_bench / bench::run_play.
Display and Status Output crates/pu-cli/src/output.rs	Introduces status_colored_with_suspended() and updates status_colored() to delegate; updates Suspend/Resume result messages and prints; adds/updates tests for benched/resume behavior and empty suspend results.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as "CLI Parser"
    participant Handler as "bench::run_bench / run_play"
    participant Daemon
    participant Output as "output formatter"

    User->>CLI: invoke bench/play command
    CLI->>Handler: call run_bench(socket, agent?, all, json) / run_play(socket, agent_id, json)
    Handler->>Handler: determine target & validate (All or Agent)
    Handler->>Daemon: send Suspend / Resume request
    Daemon-->>Handler: return SuspendResult / ResumeResult
    Handler->>Output: format result (status_colored_with_suspended)
    Output-->>Handler: formatted message
    Handler-->>User: display results
    alt self-agent included
        Handler-->>User: emit self-bench warning
    end

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I thumped my paw and gave a wink,

Agents paused upon my blink,
Bench for tests, then play — hooray!
Quiet fields, then work and play,
— A rabbit cheers the code today.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'feat: pu bench / pu play — suspend and resume agents' directly and clearly summarizes the main changes: adding two new CLI commands (bench and play) for suspending and resuming agents.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pu/feature-3

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[2witstudios]

Code review

Found 1 issue:

1. Missing self-protection: kill.rs reads PU_AGENT_ID from the environment to prevent an agent from killing itself, and auto-excludes self from bulk kills (--all). The new bench command performs an analogous destructive operation (the engine's handle_suspend calls self.kill_agents() internally) but has no self-protection. An agent running pu bench --all or pu bench <own-id> would suspend itself.

PurePoint/crates/pu-cli/src/commands/bench.rs

Lines 9 to 33 in eace188

	socket: &Path,
	agent: Option<String>,
	all: bool,
	json: bool,
	) -> Result<(), CliError> {
	let target = if all {
	SuspendTarget::All
	} else if let Some(id) = agent {
	SuspendTarget::Agent(id)
	} else {
	return Err(CliError::Other(
	"bench target required — use <AGENT_ID> or --all".into(),
	));
	};
	daemon_ctrl::ensure_daemon(socket).await?;
	let project_root = crate::commands::cwd_string()?;
	let resp = client::send_request(
	socket,
	&Request::Suspend {
	project_root,
	target,
	},
	)

Compare with the self-protection pattern in kill.rs:

PurePoint/crates/pu-cli/src/commands/kill.rs

Lines 16 to 44 in eace188

	let self_agent_id = std::env::var("PU_AGENT_ID").ok();
	let target = if all {
	if include_root {
	KillTarget::All
	} else {
	KillTarget::AllWorktrees
	}
	} else if let Some(a) = agent {
	// Self-protection: refuse to kill own agent
	if let Some(ref self_id) = self_agent_id {
	if &a == self_id {
	return Err(CliError::Other("cannot kill self".into()));
	}
	}
	KillTarget::Agent(a)
	} else if let Some(wt) = worktree {
	KillTarget::Worktree(wt)
	} else {
	return Err(CliError::Other(
	"kill target required — use --agent, --worktree, or --all".into(),
	));
	};
	// Auto-exclude self from bulk kills
	let exclude = match self_agent_id {
	Some(id) if !matches!(target, KillTarget::Agent(_)) => vec![id],
	_ => vec![],
	};

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[2witstudios]

Fixed in 7b8acac:

• Single-agent bench (pu bench <id>) now reads PU_AGENT_ID from env and refuses to bench self, matching kill.rs lines 16/26-30.
• Bulk bench (pu bench --all) emits a warning if the calling agent was included in the suspend results, since SuspendTarget lacks an exclude field (unlike KillTarget). A protocol-level exclude would be a separate enhancement.

Changed file: crates/pu-cli/src/commands/bench.rs lines 16-52.

[coderabbitai]

🧹 Nitpick comments (2)

crates/pu-cli/src/commands/bench.rs (1)

45-54: Post-hoc warning is a reasonable workaround for protocol limitation.

Since SuspendTarget lacks an exclude field (unlike KillTarget), the code cannot prevent self-inclusion in --all operations at the request level. The warning after the fact, with guidance to resume via pu play, is a pragmatic solution. Consider documenting this behavior in the CLI help text.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/pu-cli/src/commands/bench.rs` around lines 45 - 54, The post-hoc
warning handles a protocol limitation where SuspendTarget cannot exclude self,
so detect self-inclusion as currently done by checking self_agent_id and
matching pu_core::protocol::Response::SuspendResult { suspended } and, in
addition to the existing eprintln warning, add a short help-text note to the
bench command's CLI usage explaining that `--all` may include the invoking agent
and how to resume with `pu play <agent_id>`; update the help/usage string for
the bench subcommand (where the command description/long_about is defined) to
document this behavior so users see it before running the command.

crates/pu-cli/src/main.rs (1)

70-79: Consider adding conflicts_with for mutual exclusivity.

The Kill command (lines 91-92) uses #[arg(long, conflicts_with_all = ["worktree", "all"])] to enforce that --agent and --all are mutually exclusive at the CLI level. The Bench command allows both agent_id and --all to be provided simultaneously, relying on bench.rs to prefer --all. While functionally correct, adding conflicts_with would provide clearer error messages at parse time.

♻️ Optional: Add conflicts_with for clearer CLI errors

     /// Bench (suspend) agents — pull them off the court
     Bench {
         /// Agent ID to bench
+        #[arg(conflicts_with = "all")]
         agent_id: Option<String>,
         /// Bench all active agents
         #[arg(long)]
         all: bool,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/pu-cli/src/main.rs` around lines 70 - 79, The Bench variant allows
both agent_id and --all simultaneously; update the CLI arg attributes on the
Bench fields to declare mutual exclusivity so clap reports a parse-time error:
add an attribute to the agent_id field (e.g. #[arg(long, conflicts_with =
"all")] or #[arg(conflicts_with = "all")]) and/or to the all field (e.g.
#[arg(long, conflicts_with = "agent_id")]) so the Bench struct's agent_id and
all flags conflict at parse time (refer to the Bench variant and its agent_id
and all fields).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/pu-cli/src/commands/bench.rs`:
- Around line 45-54: The post-hoc warning handles a protocol limitation where
SuspendTarget cannot exclude self, so detect self-inclusion as currently done by
checking self_agent_id and matching pu_core::protocol::Response::SuspendResult {
suspended } and, in addition to the existing eprintln warning, add a short
help-text note to the bench command's CLI usage explaining that `--all` may
include the invoking agent and how to resume with `pu play <agent_id>`; update
the help/usage string for the bench subcommand (where the command
description/long_about is defined) to document this behavior so users see it
before running the command.

In `@crates/pu-cli/src/main.rs`:
- Around line 70-79: The Bench variant allows both agent_id and --all
simultaneously; update the CLI arg attributes on the Bench fields to declare
mutual exclusivity so clap reports a parse-time error: add an attribute to the
agent_id field (e.g. #[arg(long, conflicts_with = "all")] or
#[arg(conflicts_with = "all")]) and/or to the all field (e.g. #[arg(long,
conflicts_with = "agent_id")]) so the Bench struct's agent_id and all flags
conflict at parse time (refer to the Bench variant and its agent_id and all
fields).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aedae928-786a-4cf9-a59d-355eff61b50d

📥 Commits

Reviewing files that changed from the base of the PR and between 55aa882 and 7b8acac.

📒 Files selected for processing (4)

• crates/pu-cli/src/commands/bench.rs
• crates/pu-cli/src/commands/mod.rs
• crates/pu-cli/src/main.rs
• crates/pu-cli/src/output.rs

[2witstudios]

Addressed the remaining CodeRabbit feedback in c7a1b9f:

Nitpick #1 (bench.rs:45-54 — document --all self-inclusion): Added long_about to the Bench command in main.rs explaining that --all may suspend the invoking agent and how to resume with pu play. Also added doc comments to run_bench documenting this behavior.

Nitpick #2 (main.rs:70-79 — conflicts_with): Already addressed in 953fec0.

Docstring coverage warning: Added doc comments to run_bench, run_play, status_colored, and status_colored_with_suspended.

All CI checks were green before this commit; local validation (tests, clippy, fmt) passes.