Files
clawmates/crates/cm-api/src/error.rs
T
Omar SobhandClaude Opus 5 5c2c63f8e8 feat(missions): a human can finally reach the plan/roster review gate
Phase 4 of the plan, plus the PLAN_COMPLETE decision and the gitea_forge
cleanup from Phase 5.

THE REVIEW UI

mission_plan and mission_roster have been complete and reachable by curl
since they shipped, with zero frontend. That matters more than a missing
screen usually would: the decide step is not a convenience, it IS the
safety mechanism. Approving a plan replaces the mission's phases; approving
a roster flips it to the composed engine. A gate nobody can reach is a gate
that is always open or always shut.

MissionProposalDrawer, modelled on LevelUpDrawer which already does
load → review → decide. Reached from a mission's SETUP tab. Verified end to
end against the live backend, not just compiled: a model proposed a roster,
approval flipped the mission to `composed`, and approval on a non-draft
mission was refused.

The plan view shows each phase's done_when, and says plainly when one is
absent — a phase without a completion condition is never judged and reports
completed whatever it did, so its absence is the thing worth seeing.

AND THE DEFECT BUILDING IT FOUND

Every refusal path computed a precise reason — "the mission is running, not
a draft", "no node can boot that backend any more" — logged it to stderr,
and returned a bare {"error":"bad request"}. The person who needed the
sentence was the one clicking Approve; they got two words, and the reason
went to a server log they cannot read.

ApiError::Refused(String) carries it now. Same argument ApiError::Unavailable
was added for ("a 500 with 'internal error' sent them looking for a bug that
was not there"), one status code down. Live: the 400 now reads "this mission
is completed — a roster can only be approved while it is a draft, because
approving one rewrites how the mission will run".

PLAN_COMPLETE, decided

The Skill-Use measurement found that int-xx-marker-protocol documents
PLAN_COMPLETE and task_card_parser never implemented it, so an agent
following the skill exactly was silently ignored. Implemented rather than
removed from the skill: the planner needs a way to say it is done
specifying, and agents already emit it.

Marker ids are now strictly INT-<digits>. `starts_with("INT-")` accepted the
range form `INT-01..02` — observed live — which parsed into an id matching
no real item, so a task card appeared for something that did not exist while
the two items it covered stayed open. Rejecting is right: an ignored marker
is visible, a plausible row is not.

GITEA_FORGE, REMOVED

Named in nine places, defined in none. Harmless while provision_claw ignored
the bundle list; once the list was honoured, an undefined name became a
capability an agent is told it has and does not. Removed from seven team
templates, a workflow recipe, the auto-provision path, and a dropdown a user
could pick it from.

A new test asserts every bundle a template names is defined in the runtime
config — and it immediately found `web_fetch` in two templates I had missed
removing by hand. Same shape as the skill-binding test, one layer up.

Agents reach the forge through git over HTTPS with the ambient GITEA_TOKEN,
which is why nothing ever broke.

Full workspace suite green (106 binaries); frontend builds clean.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-19 11:56:54 -07:00

113 lines
4.1 KiB
Rust

use axum::http::StatusCode;
use axum::response::{IntoResponse, Response};
use axum::Json;
use serde_json::json;
/// API-surface errors with their HTTP mapping. Internal causes are logged
/// server-side, never echoed to clients.
#[derive(Debug, thiserror::Error)]
pub enum ApiError {
#[error("bad request")]
BadRequest,
#[error("unauthorized")]
Unauthorized,
#[error("forbidden")]
Forbidden,
#[error("not found")]
NotFound,
#[error("conflict")]
Conflict,
#[error("{0}")]
Quota(String),
/// A 400 whose REASON the caller needs.
///
/// Same argument as `Unavailable` below, one status code down. The
/// proposal decide handlers each computed a precise refusal — "the mission
/// is running, not a draft", "no node can boot that backend any more" —
/// logged it to stderr, and returned a bare `BadRequest`. The person who
/// needed the sentence was the one clicking Approve, and they got
/// "bad request". `mission_plan::Refusal` exists and is written as
/// human-readable copy; this is how it reaches them.
#[error("{0}")]
Refused(String),
/// A dependency is temporarily refusing work and will accept it later —
/// today, the Claude Code subscription's rate limit. Distinct from
/// `Internal` because the operator's next action is different: wait and
/// press the button again, rather than read a server log. A 500 with
/// "internal error" sent them looking for a bug that was not there.
#[error("{0}")]
Unavailable(String),
#[error("internal error")]
Internal,
}
impl From<cm_db::DbError> for ApiError {
fn from(err: cm_db::DbError) -> Self {
match err {
cm_db::DbError::NotFound => ApiError::NotFound,
_ => ApiError::Internal,
}
}
}
impl From<sqlx::Error> for ApiError {
fn from(err: sqlx::Error) -> Self {
ApiError::from(cm_db::DbError::from(err))
}
}
impl From<cm_auth::AuthError> for ApiError {
fn from(err: cm_auth::AuthError) -> Self {
match err {
cm_auth::AuthError::InvalidCredentials | cm_auth::AuthError::Unauthenticated => {
ApiError::Unauthorized
}
_ => ApiError::Internal,
}
}
}
impl IntoResponse for ApiError {
fn into_response(self) -> Response {
let status = match self {
ApiError::BadRequest | ApiError::Refused(_) => StatusCode::BAD_REQUEST,
ApiError::Unauthorized => StatusCode::UNAUTHORIZED,
ApiError::Forbidden => StatusCode::FORBIDDEN,
ApiError::NotFound => StatusCode::NOT_FOUND,
ApiError::Conflict => StatusCode::CONFLICT,
ApiError::Quota(_) => StatusCode::PAYMENT_REQUIRED,
ApiError::Unavailable(_) => StatusCode::SERVICE_UNAVAILABLE,
ApiError::Internal => StatusCode::INTERNAL_SERVER_ERROR,
};
(status, Json(json!({ "error": self.to_string() }))).into_response()
}
}
#[cfg(test)]
mod tests {
use super::*;
use axum::body::to_bytes;
/// A refusal must carry its reason into the response body.
///
/// The proposal decide handlers each computed a precise sentence and then
/// returned a bare `BadRequest`, so the person clicking Approve saw
/// "bad request" while the reason went to a server log they cannot read.
#[tokio::test]
async fn a_refusal_reaches_the_caller_and_a_bare_bad_request_does_not_pretend_to() {
let refused = ApiError::Refused("this mission is running, not a draft".into());
let response = refused.into_response();
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
let body = to_bytes(response.into_body(), 64 * 1024).await.unwrap();
let text = String::from_utf8_lossy(&body);
assert!(
text.contains("running, not a draft"),
"the reason must be in the body, not only in the server log: {text}"
);
// The bare variant stays as it was — same status, no invented detail.
let bare = ApiError::BadRequest.into_response();
assert_eq!(bare.status(), StatusCode::BAD_REQUEST);
}
}