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]>
113 lines
4.1 KiB
Rust
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);
|
|
}
|
|
}
|