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]>
280 lines
9.6 KiB
Rust
280 lines
9.6 KiB
Rust
//! Task-card parser — Slice 5.
|
||
//!
|
||
//! Watches `run_events` payloads for the INT-XX marker protocol
|
||
//! (see skills/foundation/int-xx-marker-protocol.md) and materializes
|
||
//! `mission_tasks` rows with typed state so the mission canvas Tasks
|
||
//! tab can render a status timeline.
|
||
//!
|
||
//! Recognized markers (case-sensitive, on their own line):
|
||
//! TASK: INT-NN — <title> → status='created'
|
||
//! WORK: INT-NN → status='working'
|
||
//! HANDOFF: INT-NN → status='validating'
|
||
//! TEST_PASS: INT-NN → status='validating' (unchanged if already validating+)
|
||
//! TEST_FAIL: INT-NN — <reason> → status='failed'
|
||
//! REVIEW_APPROVE: INT-NN → status='validating'
|
||
//! REVIEW_BLOCK: INT-NN — <reason> → status='failed'
|
||
//! COMPLETED: INT-NN → status='complete'
|
||
//!
|
||
//! Uses UPSERT keyed on (phase_id, external_id) so the same INT
|
||
//! progressing through states updates a single row; state transitions
|
||
//! are monotonic where reasonable (once complete, only failed can
|
||
//! demote — but the loop-once semantics of INT items mean this rarely
|
||
//! matters in practice).
|
||
|
||
use serde_json::Value;
|
||
use sqlx::PgPool;
|
||
use sqlx::Row;
|
||
use uuid::Uuid;
|
||
|
||
use cm_db::repo::missions::UpsertTask;
|
||
|
||
/// A parsed marker from a single run-event payload.
|
||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||
pub struct Marker {
|
||
pub int_id: String, // "INT-05"
|
||
pub kind: MarkerKind,
|
||
pub title: Option<String>,
|
||
}
|
||
|
||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||
pub enum MarkerKind {
|
||
Task,
|
||
PlanComplete,
|
||
Work,
|
||
Handoff,
|
||
TestPass,
|
||
TestFail,
|
||
ReviewApprove,
|
||
ReviewBlock,
|
||
Completed,
|
||
}
|
||
|
||
impl MarkerKind {
|
||
/// The status this marker implies. Preferences monotonic forward
|
||
/// motion — the UPSERT layer may still overwrite prior states.
|
||
pub fn status(&self) -> &'static str {
|
||
match self {
|
||
// The planner finished specifying; no work has started, so the item
|
||
// is in the same state a fresh TASK leaves it in. A distinct status
|
||
// would need a column value the UI does not render, and inventing
|
||
// one to look complete is how a status stops meaning anything.
|
||
MarkerKind::Task | MarkerKind::PlanComplete => "created",
|
||
MarkerKind::Work => "working",
|
||
MarkerKind::Handoff | MarkerKind::TestPass | MarkerKind::ReviewApprove => "validating",
|
||
MarkerKind::TestFail | MarkerKind::ReviewBlock => "failed",
|
||
MarkerKind::Completed => "complete",
|
||
}
|
||
}
|
||
}
|
||
|
||
/// Line-scanner over the raw text of a run's output. Cheap enough to
|
||
/// re-run per event; the UPSERT layer collapses duplicates.
|
||
pub fn parse(text: &str) -> Vec<Marker> {
|
||
let mut out = Vec::new();
|
||
for line in text.lines() {
|
||
let trimmed = line.trim();
|
||
if let Some(m) = parse_line(trimmed) {
|
||
out.push(m);
|
||
}
|
||
}
|
||
out
|
||
}
|
||
|
||
/// `INT-` followed by at least one digit and nothing else.
|
||
fn is_int_id(id: &str) -> bool {
|
||
match id.strip_prefix("INT-") {
|
||
Some(rest) => !rest.is_empty() && rest.chars().all(|c| c.is_ascii_digit()),
|
||
None => false,
|
||
}
|
||
}
|
||
|
||
fn parse_line(line: &str) -> Option<Marker> {
|
||
// Match `<KIND>: INT-NN` (rest optional). Strict on the colon and
|
||
// the INT- prefix — anything laxer starts matching prose.
|
||
let (kind_str, rest) = line.split_once(':')?;
|
||
let kind = match kind_str.trim() {
|
||
"TASK" => MarkerKind::Task,
|
||
// Documented in `skills/foundation/int-xx-marker-protocol.md` since the
|
||
// skill was written, and never implemented here. Agents that followed
|
||
// the skill exactly emitted it and were silently ignored — observed on
|
||
// a live mission, found by the Skill-Use measurement. Implemented
|
||
// rather than removed from the skill: the planner needs a way to say
|
||
// it is done specifying, and agents already emit this one.
|
||
"PLAN_COMPLETE" => MarkerKind::PlanComplete,
|
||
"WORK" => MarkerKind::Work,
|
||
"HANDOFF" => MarkerKind::Handoff,
|
||
"TEST_PASS" => MarkerKind::TestPass,
|
||
"TEST_FAIL" => MarkerKind::TestFail,
|
||
"REVIEW_APPROVE" => MarkerKind::ReviewApprove,
|
||
"REVIEW_BLOCK" => MarkerKind::ReviewBlock,
|
||
"COMPLETED" => MarkerKind::Completed,
|
||
_ => return None,
|
||
};
|
||
let rest = rest.trim();
|
||
let (id_tok, tail) = match rest.split_once(char::is_whitespace) {
|
||
Some((a, b)) => (a, Some(b.trim())),
|
||
None => (rest, None),
|
||
};
|
||
let int_id = id_tok.trim_end_matches(&[',', ';', '.'][..]).to_string();
|
||
// Strictly `INT-<digits>`. `starts_with("INT-")` alone accepted range forms
|
||
// like `INT-01..02`, which parse into an id matching no real item — so a
|
||
// task card appeared for something that did not exist while the two items
|
||
// it was meant to cover stayed open. Observed live. Rejecting is right:
|
||
// the marker is ignored, which is visible, instead of creating a plausible
|
||
// row, which is not.
|
||
if !is_int_id(&int_id) {
|
||
return None;
|
||
}
|
||
// Title: after the id + any of ` — / – / - ` separators
|
||
let title = tail.and_then(|t| {
|
||
let t = t.trim_start_matches(['—', '–', '-', ':'].as_slice()).trim();
|
||
if t.is_empty() {
|
||
None
|
||
} else {
|
||
Some(t.to_string())
|
||
}
|
||
});
|
||
Some(Marker {
|
||
int_id,
|
||
kind,
|
||
title,
|
||
})
|
||
}
|
||
|
||
/// Scan every event of a topology_run that has a mission_id set;
|
||
/// parse markers out of the event payload's `text` / `output` fields;
|
||
/// UPSERT each into `mission_tasks`.
|
||
///
|
||
/// Idempotent: re-running against the same run's events collapses
|
||
/// into the current-truth state (last marker per INT wins per pass).
|
||
pub async fn apply_for_run(pool: &PgPool, run_id: Uuid) -> Result<usize, String> {
|
||
// Load the run + its mission binding. Bail cheaply if no mission.
|
||
let run = sqlx::query(
|
||
"SELECT mission_id, mission_phase_id
|
||
FROM topology_runs WHERE id = $1",
|
||
)
|
||
.bind(run_id)
|
||
.fetch_optional(pool)
|
||
.await
|
||
.map_err(|e| format!("load run: {e}"))?;
|
||
let Some(run) = run else {
|
||
return Ok(0);
|
||
};
|
||
let mission_id: Option<Uuid> = run.try_get("mission_id").ok().flatten();
|
||
let phase_id: Option<Uuid> = run.try_get("mission_phase_id").ok().flatten();
|
||
let (Some(mission_id), Some(phase_id)) = (mission_id, phase_id) else {
|
||
return Ok(0);
|
||
};
|
||
|
||
// Read event payloads in order.
|
||
let rows = sqlx::query(
|
||
"SELECT payload
|
||
FROM run_events WHERE run_id = $1
|
||
ORDER BY seq ASC",
|
||
)
|
||
.bind(run_id)
|
||
.fetch_all(pool)
|
||
.await
|
||
.map_err(|e| format!("load events: {e}"))?;
|
||
|
||
let mut all_markers: Vec<Marker> = Vec::new();
|
||
for r in rows {
|
||
let payload: Value = r.get("payload");
|
||
for text in extract_text_fields(&payload) {
|
||
all_markers.extend(parse(&text));
|
||
}
|
||
}
|
||
|
||
let mut applied = 0usize;
|
||
for m in &all_markers {
|
||
let title = m.title.clone().unwrap_or_else(|| m.int_id.clone());
|
||
cm_db::repo::missions::upsert_task(
|
||
pool,
|
||
UpsertTask {
|
||
mission_id,
|
||
phase_id,
|
||
external_id: &m.int_id,
|
||
title: &title,
|
||
assigned_agent_id: None,
|
||
status: m.kind.status(),
|
||
run_id: Some(run_id),
|
||
},
|
||
)
|
||
.await
|
||
.map_err(|e| format!("upsert_task {}: {e}", m.int_id))?;
|
||
applied += 1;
|
||
}
|
||
Ok(applied)
|
||
}
|
||
|
||
/// Walk a payload's JSON tree and yield every string field named
|
||
/// `text`, `output`, `content`, or `message`. Handles the shapes we
|
||
/// see across ZeroClaw events (chunk / tool_result / done / final).
|
||
fn extract_text_fields(v: &Value) -> Vec<String> {
|
||
fn walk(v: &Value, out: &mut Vec<String>) {
|
||
match v {
|
||
Value::Object(map) => {
|
||
for (k, child) in map {
|
||
if matches!(k.as_str(), "text" | "output" | "content" | "message") {
|
||
if let Value::String(s) = child {
|
||
out.push(s.clone());
|
||
}
|
||
}
|
||
walk(child, out);
|
||
}
|
||
}
|
||
Value::Array(items) => {
|
||
for item in items {
|
||
walk(item, out);
|
||
}
|
||
}
|
||
_ => {}
|
||
}
|
||
}
|
||
let mut out = Vec::new();
|
||
walk(v, &mut out);
|
||
out
|
||
}
|
||
|
||
#[cfg(test)]
|
||
mod tests {
|
||
use super::*;
|
||
|
||
#[test]
|
||
fn parses_completed_marker() {
|
||
let out = parse("COMPLETED: INT-05");
|
||
assert_eq!(out.len(), 1);
|
||
assert_eq!(out[0].int_id, "INT-05");
|
||
assert_eq!(out[0].kind, MarkerKind::Completed);
|
||
}
|
||
|
||
#[test]
|
||
fn parses_task_with_title() {
|
||
let out = parse("TASK: INT-12 — wire the loader");
|
||
assert_eq!(out[0].kind, MarkerKind::Task);
|
||
assert_eq!(out[0].title.as_deref(), Some("wire the loader"));
|
||
}
|
||
|
||
#[test]
|
||
fn parses_test_fail_with_reason() {
|
||
let out = parse("TEST_FAIL: INT-03 — flake in async_setup");
|
||
assert_eq!(out[0].kind, MarkerKind::TestFail);
|
||
assert_eq!(out[0].title.as_deref(), Some("flake in async_setup"));
|
||
}
|
||
|
||
#[test]
|
||
fn ignores_marker_inside_prose() {
|
||
// Not on its own line + not starting with the exact kind:
|
||
let out = parse("we should not TASK: INT-05 like this");
|
||
assert!(out.is_empty());
|
||
}
|
||
|
||
#[test]
|
||
fn scans_multiline_batch() {
|
||
let src = "TASK: INT-01 — foo\nWORK: INT-01\nother text\nCOMPLETED: INT-01\n";
|
||
let out = parse(src);
|
||
assert_eq!(out.len(), 3);
|
||
assert_eq!(out[2].kind, MarkerKind::Completed);
|
||
}
|
||
}
|