Skip to content

refactor(macros): create agentd-macros proc-macro crate with ApiIntoResponse derive #768

@geoffjay

Description

@geoffjay

Overview

Every service crate manually implements IntoResponse for its ApiError enum using an identical structural pattern: match each variant to a (StatusCode, message) pair and return (status, Json(json!({ \"error\": message }))).into_response(). A #[derive(ApiIntoResponse)] proc-macro would eliminate this boilerplate, enforce the consistent JSON shape, and make the status-code mapping declarative rather than imperative.

This issue tracks both creating the agentd-macros crate and implementing the first derive macro within it. It is complementary to #126 (which consolidates error types) — the derive macro makes the remaining per-service IntoResponse impls trivially thin regardless of whether types are fully unified.

Current State

The following four files each contain an identical ~10-line impl IntoResponse block:

// crates/hook/src/error.rs:31-41
impl IntoResponse for ApiError {
    fn into_response(self) -> Response {
        let (status, message) = match &self {
            ApiError::InvalidEvent(msg) => (StatusCode::BAD_REQUEST, msg.clone()),
            ApiError::NotFound(msg)     => (StatusCode::NOT_FOUND, msg.clone()),
            ApiError::InvalidShell(msg) => (StatusCode::BAD_REQUEST, msg.clone()),
            ApiError::Internal(msg)     => (StatusCode::INTERNAL_SERVER_ERROR, msg.clone()),
        };
        (status, Json(json!({ "error": message }))).into_response()
    }
}
// crates/monitor/src/error.rs:27-37  (identical shape, different variants)
// crates/common/src/error.rs:67-80   (identical shape, different variants)
// crates/ask/src/error.rs:200-215    (identical shape, wraps sub-errors via .to_string())

Desired State

A new crates/macros proc-macro crate exposes a #[derive(ApiIntoResponse)] derive macro. Error variants are annotated with their HTTP status code:

// crates/hook/src/error.rs — after
#[derive(Debug, Error, ApiIntoResponse)]
pub enum ApiError {
    #[error("Invalid event: {0}")]
    #[status(StatusCode::BAD_REQUEST)]
    InvalidEvent(String),

    #[error("Not found: {0}")]
    #[status(StatusCode::NOT_FOUND)]
    NotFound(String),

    #[error("Invalid shell: {0}")]
    #[status(StatusCode::BAD_REQUEST)]
    InvalidShell(String),

    #[error("Internal error: {0}")]
    #[status(StatusCode::INTERNAL_SERVER_ERROR)]
    Internal(String),
}
// No manual `impl IntoResponse` block needed.

The generated impl always returns (status, Json(json!({ "error": self.to_string() }))).into_response(), delegating the message formatting to thiserror's Display impl.

New Crate Structure

crates/macros/
├── Cargo.toml         # proc-macro = true; deps: proc-macro2, syn, quote
└── src/
    └── lib.rs         # #[proc_macro_derive(ApiIntoResponse, attributes(status))]

Cargo.toml workspace member entry and agentd-macros added as a dependency to common, hook, monitor, ask.

Affected Files

  • crates/macros/ — new crate (create)
  • Cargo.toml — add workspace member
  • crates/common/src/error.rs — apply derive, remove manual impl (~14 lines)
  • crates/hook/src/error.rs — apply derive, remove manual impl (~12 lines)
  • crates/monitor/src/error.rs — apply derive, remove manual impl (~11 lines)
  • crates/ask/src/error.rs — apply derive, remove manual impl (~18 lines)

Acceptance Criteria

  • crates/macros crate scaffolded with proc-macro = true and proc-macro2, syn, quote deps
  • #[derive(ApiIntoResponse)] derive macro implemented and tested in the macros crate
  • #[status(StatusCode::X)] helper attribute accepted on each enum variant
  • impl IntoResponse removed from common, hook, monitor, and ask error files
  • JSON response shape is { "error": "<Display output>" } — identical to current behavior
  • All existing error response tests pass unchanged
  • cargo clippy produces no new warnings
  • No change to public API behavior

Notes

  • The proc-macro crate should depend on syn with full features and quote/proc-macro2 — these are already transitive deps via serde, thiserror, and sea-orm, so compile overhead is minimal.
  • Variants that wrap sub-errors (e.g., ask::ApiError::TmuxError(#[from] TmuxError)) work naturally since the generated impl calls self.to_string() which delegates to thiserror's Display.
  • ask/src/error.rs has a manual impl From<anyhow::Error> that is unrelated and should be preserved.
  • Pairs with Consolidate duplicated error types into agentd-common #126 (error type consolidation) but is independently mergeable — apply the derive before or after type consolidation.
  • Consider macro_rules! as a lighter-weight alternative if proc-macro complexity is not justified; revisit after Consolidate duplicated error types into agentd-common #126 lands.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorCode improvement without changing behavior

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions