Skip to content

Simplify pu-cli: deferred refactors from code review #65

@2witstudios

Description

@2witstudios

Context

During a simplify review of crates/pu-cli/src/, several structural improvements were identified but deferred as they require broader changes across multiple crates or touch many files.

Deferred Refactors

1. Extract daemon_rpc() helper to eliminate request-response boilerplate

Impact: High — 16+ call sites

Nearly every command handler repeats this identical 4-line sequence:

daemon_ctrl::ensure_daemon(socket).await?;
let project_root = commands::cwd_string()?;
let resp = client::send_request(socket, &Request::...).await?;
let resp = output::check_response(resp, json)?;
output::print_response(&resp, json);
Ok(())

A shared daemon_rpc(socket, request, json) -> Result<(), CliError> would reduce each simple command to request construction + one function call.

Affected files: status.rs, health.rs, logs.rs, kill.rs, agent_def.rs (4 fns), swarm.rs (5 fns), prompt.rs (3 fns), spawn.rs.

2. Group function parameters into structs to eliminate #[allow(clippy::too_many_arguments)]

Impact: High — 4 functions

  • spawn.rs:run() — 11 parameters
  • swarm.rs:run_create() — 8 parameters
  • agent_def.rs:run_create() — 8 parameters
  • prompt.rs:run_create() — 7 parameters

Each suppresses the clippy lint. Extracting domain parameters into structs (e.g., SpawnOptions, SwarmCreateParams) or passing the clap-parsed struct directly would fix this.

3. Introduce Scope enum in pu_core to replace stringly-typed "local" / "global"

Impact: High — 12+ locations across pu-cli and pu-core

The scope parameter flows as a raw String through CLI args, IPC requests, and storage with zero validation. A user can pass --scope banana with no error. A Scope enum with clap ValueEnum derive would add compile-time safety.

Locations: main.rs (6 subcommand variants), pu_core::protocol (6 Request variants), pu_core::agent_def, pu_core::swarm_def.

4. Compute skill content hash at compile time

Impact: Medium — runs on every CLI invocation

skill.rs::skill_hash() hashes the entire SKILL_CONTENT (an include_str! constant) at runtime on every pu command. Since the content is baked in at compile time, the hash is deterministic and could be precomputed via build.rs or a const-evaluable hash, turning the hot-path check into a simple string comparison.

5. Consolidate duplicate serde default functions in pu_core

Impact: Low — 8 duplicated functions

These serde #[serde(default = "...")] helpers are duplicated across files:

  • default_true() — in types.rs, protocol.rs, agent_def.rs
  • default_agent_type() — in protocol.rs, agent_def.rs
  • default_quantity() — in protocol.rs, swarm_def.rs
  • default_worktree_count() — in protocol.rs, swarm_def.rs

Should be consolidated into a shared defaults module.

6. Add axis and direction enums for grid commands

Impact: Medium — no validation currently

  • axis in GridAction::Split accepts any string (should be "v" or "h")
  • direction in GridAction::Focus accepts any string (should be "up", "down", "left", "right")

Both should use clap ValueEnum enums, defined in pu_core::protocol.

7. Move GridAction enum from main.rs to commands/grid.rs

Impact: Low — leaky abstraction

GridAction is defined in main.rs but consumed exclusively by grid.rs. The enum should live in the command module and be re-exported.

8. Unify prompt list output between daemon and local-only paths

Impact: Low — inconsistent UX

prompt.rs::run_list() falls back to run_list_local() when the daemon is unavailable, which formats template lists differently from the daemon path's output::print_response(). Users get inconsistent output depending on daemon state.


Identified during /simplify review of crates/pu-cli/src/. Items 1-3 are the highest-impact improvements.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions