Skip to content

9 near-identical CommandError-wrapping delegate boilerplate blocks in agent_access_impl.rs #6262

Description

@bug-ops

Finding

AgentAccess requires each command handler to return Pin<Box<dyn Future<Output = Result<String, CommandError>> + Send + 'a>> (necessary since async fn cannot appear in dyn-compatible traits). For at least 9 handlers, the entire body is the identical shape — delegate to an inherent *_as_string/*_command_as_string method and map its error to CommandError::new(e.to_string()):

fn X<'a>(&'a mut self, args: &'a str) -> Pin<Box<dyn Future<Output = Result<String, CommandError>> + Send + 'a>> {
    Box::pin(async move {
        self.INNER(args)
            .await
            .map_err(|e| CommandError::new(e.to_string()))
    })
}

Occurrences (all in crates/zeph-core/src/agent/agent_access_impl.rs):

  • lsp_status (1030-1038)
  • the _ match arm of handle_mcp delegating to handle_mcp_command (1262-1266)
  • handle_skill (1272-1282)
  • handle_skills (1286-1296)
  • handle_feedback_command (1300-1310)
  • handle_plan (scheduler feature) (1314-1324)
  • handle_experiment (1336-1345)
  • list_worktrees (1857-1865)
  • clean_worktrees (1867-1876)

Location

crates/zeph-core/src/agent/agent_access_impl.rs — see line ranges above.

Before

Each occurrence repeats the full Pin<Box<dyn Future<...>>> signature and the Box::pin(async move { ... .map_err(...) }) wrapper by hand (9 copies, ~6-10 lines each).

After

A declarative macro collapses each to one line, e.g.:

macro_rules! delegate_str_cmd {
    ($name:ident, $inner:ident) => {
        fn $name<'a>(
            &'a mut self,
            args: &'a str,
        ) -> Pin<Box<dyn Future<Output = Result<String, CommandError>> + Send + 'a>> {
            Box::pin(async move { self.$inner(args).await.map_err(|e| CommandError::new(e.to_string())) })
        }
    };
}

delegate_str_cmd!(lsp_status, handle_lsp_status_as_string);
delegate_str_cmd!(handle_skill, handle_skill_command_as_string);
// ...

(list_worktrees/clean_worktrees return Result<Option<String>, CommandError> rather than Result<String, CommandError>, and handle_plan/handle_experiment take a plain input/args name — a second macro variant or a generic Output type param covers those without forcing an exact match.)

Why

This is boilerplate inherent to implementing a dyn-compatible trait, not copy-pasted domain logic, so it is low urgency — but per the DRY guidance (3+ copies of the same pattern is the extraction threshold) 9 hand-maintained copies of the exact same error-wrapping convention mean any future change to that convention (e.g., adding a tracing span, changing the error message format) requires touching 9 call sites by hand instead of one macro definition. Low risk, mechanical win for maintainability.

Metadata

Metadata

Assignees

Labels

P3Research — medium-high complexityarchitectureArchitecture improvementstech-debtTechnical debt

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions