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]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
91a6b4e304
commit
5c2c63f8e8
@@ -253,8 +253,11 @@ pub async fn decide(
|
||||
.map_err(|_| ApiError::Internal)?
|
||||
.ok_or(ApiError::NotFound)?;
|
||||
if mission.status != "draft" {
|
||||
eprintln!("mission {id}: roster approval refused — mission is {}", mission.status);
|
||||
return Err(ApiError::BadRequest);
|
||||
return Err(ApiError::Refused(format!(
|
||||
"this mission is {} — a {} can only be approved while it is a draft, \
|
||||
because approving one rewrites how the mission will run",
|
||||
mission.status, "roster"
|
||||
)));
|
||||
}
|
||||
|
||||
let roster: Roster = serde_json::from_value(proposal.roster.clone()).map_err(|e| {
|
||||
@@ -269,6 +272,7 @@ pub async fn decide(
|
||||
.map_err(|_| ApiError::Internal)?;
|
||||
if let Err(why) = roster.validate(&available) {
|
||||
eprintln!("mission {id}: roster {pid} is no longer applicable: {why}");
|
||||
let reason = why.to_string();
|
||||
let _ = cm_db::repo::mission_team_proposals::decide(
|
||||
&state.pool,
|
||||
pid,
|
||||
@@ -278,7 +282,13 @@ pub async fn decide(
|
||||
Some(user.user_id.as_uuid().to_owned()),
|
||||
)
|
||||
.await;
|
||||
return Err(ApiError::BadRequest);
|
||||
// The proposal has just been auto-rejected, so the caller is about to
|
||||
// re-read a list where it says "rejected" with no visible cause. The
|
||||
// reason is the whole content of this response.
|
||||
return Err(ApiError::Refused(format!(
|
||||
"this roster no longer applies to the fleet as it is now, so it was \
|
||||
rejected: {reason}"
|
||||
)));
|
||||
}
|
||||
|
||||
let graph = roster.graph().map_err(|e| {
|
||||
|
||||
Reference in New Issue
Block a user