feat(missions): W1/#13 — let a model author the mission's phases
The last unstarted item from the missions-as-workflows plan, and the other half
of Slice 5: that one lets a model size the TEAM, this lets it decide what the
work IS.
Every mission's phases come from one of five hand-written recipes in
`templates/workflows/*.toml`, chosen by `template_kind` before anyone saw the
mission. That is the "do it this way: 1, 2, 3" over-specification that makes a
capable model follow a worse plan than it would have chosen. The recipes stay —
they are still the default for a mission nobody proposes a plan for, and the
fallback when a proposal is refused.
Same three verbs and the same review gate as the roster, deliberately: propose
and decide are separate because only the second changes a mission, and a second
shape would be a second thing to get right. Approving REPLACES the phases (a
plan is an answer to "what is this mission", not an addition to one), draft-only.
GROUNDED IN WHAT THE PLATFORM ACTUALLY READS, which is the part that makes this
more than a copy. `phase_config::KNOWN_KEYS` already names every phase-config key
and the code that reads it — the registry built after `task` sat unread through
every mission. A plan is validated against it, so a model cannot propose a phase
whose settings nothing will act on: the failure that registry exists to EXPOSE is
one this path cannot create. Phase kinds are checked the same way, because an
unknown kind does not error — it falls through to the catch-all purpose and runs
as a generic phase that looks like it worked.
TWO THINGS THE WORK ITSELF FOUND, both the same shape:
- `done_when_check` — the stop-gate key added earlier today — was never
registered in `phase_config`, so every mission that set it has been logging
it as an unknown key. Found by a test written for a different purpose, which
is the registry doing exactly its job. Now registered with its reader.
- `done_when` and `max_iterations` are COLUMNS promoted out of config by
`missions::create`; the evaluator sweep filters on the column in SQL every
tick. My first insert wrote the config blob alone, which would have stored a
plan's completion condition where nothing judges it. NEGATIVE CONTROL run:
binding NULL instead of the promoted value fails
`an_approved_plan_replaces_the_missions_phases`.
`order_idx` comes from the array's own order rather than a field the model sets:
two sources for one fact is how a plan ends up with two phase 0s, and order_idx
is what `start_pending_phases` sequences on.
MAX_PHASES is 4 and the prompt argues for one. Each phase is a full agent run in
sequence, and splitting one change into plan → implement → test is the documented
anti-pattern — a single agent doing all three keeps the context that makes the
later steps good.
543 tests pass, clippy clean. Migration 0072.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
Omar Sobh
co-authored by
Claude Opus 5
Claude Opus 5
parent
a48d78f8eb
commit
a33dbdcdc3
@@ -0,0 +1,296 @@
|
||||
//! A model-authored execution plan for one mission — W1 / #13.
|
||||
//!
|
||||
//! Every mission's phases come from one of five hand-written recipes in
|
||||
//! `templates/workflows/*.toml`, chosen by `template_kind`. A recipe is a fixed
|
||||
//! answer to "what phases does this kind of mission have", written before anyone
|
||||
//! saw the mission — the "do it this way: 1, 2, 3" over-specification that makes
|
||||
//! a capable model follow a worse plan than it would have chosen for the actual
|
||||
//! task.
|
||||
//!
|
||||
//! This is the other half of [`crate::mission_roster`]: that one lets a model
|
||||
//! size the team, this one lets it decide what the work IS. Same shape on
|
||||
//! purpose — propose, review, approve, apply — because the review gate is what
|
||||
//! makes model-authored structure safe to run, and a second shape would be a
|
||||
//! second thing to get right.
|
||||
//!
|
||||
//! # Grounded in what the platform actually reads
|
||||
//!
|
||||
//! The interesting constraint is not "is this JSON valid" but "will anything
|
||||
//! consume it". `phase_config::KNOWN_KEYS` already names every phase-config key
|
||||
//! and the code that reads it, with eleven marked NOT IMPLEMENTED — the registry
|
||||
//! built after `task` sat unread through every mission. A plan is validated
|
||||
//! against that registry, so a model cannot propose a phase whose settings
|
||||
//! nothing will act on. The failure that registry exists to EXPOSE is one this
|
||||
//! path cannot create.
|
||||
//!
|
||||
//! Phase kinds are checked the same way, against the kinds `phase_runner`
|
||||
//! actually dispatches. A model asked to plan work will happily invent
|
||||
//! `kind: "review"`, and an unknown kind does not fail — it falls to the
|
||||
//! catch-all purpose and runs as a generic phase, which looks like it worked.
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
/// Phase kinds `phase_runner` dispatches on.
|
||||
///
|
||||
/// Not an enum, because `mission_phases.kind` is a free-form column shared with
|
||||
/// hand-written recipes and the wizard; this is the subset a MODEL may propose.
|
||||
/// An unrecognised kind is the dangerous case: it does not error, it falls
|
||||
/// through to the generic `mission` purpose and runs anyway.
|
||||
pub const PLANNABLE_KINDS: &[&str] = &["research", "coding", "benchmark", "security_scan"];
|
||||
|
||||
/// Ceiling on a proposed plan.
|
||||
///
|
||||
/// Each phase is a full agent run — a VM boot, a checkout, a turn, a capture —
|
||||
/// executed in sequence. Anthropic's own guidance warns against decomposing work
|
||||
/// into sequential phases at all ("a handoff loses context at every step"), so
|
||||
/// this bound is deliberately tight: a model that wants eight phases is
|
||||
/// describing a to-do list, not a plan.
|
||||
pub const MAX_PHASES: usize = 4;
|
||||
|
||||
/// One phase of a proposed plan.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
pub struct PlannedPhase {
|
||||
/// One of [`PLANNABLE_KINDS`].
|
||||
pub kind: String,
|
||||
/// What this phase does. Lands in `config.task`, which
|
||||
/// `phase_task_text` injects — the key that sat unread through every
|
||||
/// mission until two phases with different tasks produced identical output.
|
||||
pub task: String,
|
||||
/// Optional completion condition, judged post-hoc by the evaluator.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub done_when: Option<String>,
|
||||
/// Optional deterministic check, enforced IN the agent's loop by the stop
|
||||
/// gate ([`crate::vm_stop_gate`]).
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub done_when_check: Option<String>,
|
||||
/// This phase is allowed to change nothing (a verification pass).
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub allow_empty: Option<bool>,
|
||||
}
|
||||
|
||||
/// A proposed sequence of phases.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
pub struct Plan {
|
||||
pub phases: Vec<PlannedPhase>,
|
||||
}
|
||||
|
||||
/// Why a plan was refused.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum Refusal {
|
||||
Empty,
|
||||
TooMany(usize),
|
||||
UnknownKind { index: usize, kind: String },
|
||||
BlankTask(usize),
|
||||
/// A config key with no reader in this build — named, with the ones that
|
||||
/// would have been consumed.
|
||||
InertKey { index: usize, key: String },
|
||||
}
|
||||
|
||||
impl std::fmt::Display for Refusal {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
Refusal::Empty => write!(f, "the plan has no phases, so the mission would do nothing"),
|
||||
Refusal::TooMany(n) => write!(
|
||||
f,
|
||||
"the plan has {n} phases and the ceiling is {MAX_PHASES} — each one is a full \
|
||||
agent run, and a handoff loses context at every step"
|
||||
),
|
||||
Refusal::UnknownKind { index, kind } => write!(
|
||||
f,
|
||||
"phase {index} has kind {kind:?}, which nothing dispatches on; use one of: {}",
|
||||
PLANNABLE_KINDS.join(", ")
|
||||
),
|
||||
Refusal::BlankTask(i) => write!(
|
||||
f,
|
||||
"phase {i} has no task, so its agent would receive the mission description and \
|
||||
nothing telling it which part is its own"
|
||||
),
|
||||
Refusal::InertKey { index, key } => write!(
|
||||
f,
|
||||
"phase {index} sets {key:?}, which nothing in this build reads — it would be \
|
||||
stored, rendered, and consumed by nobody"
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Plan {
|
||||
/// Check a plan against what the platform can actually execute.
|
||||
pub fn validate(&self) -> Result<(), Refusal> {
|
||||
if self.phases.is_empty() {
|
||||
return Err(Refusal::Empty);
|
||||
}
|
||||
if self.phases.len() > MAX_PHASES {
|
||||
return Err(Refusal::TooMany(self.phases.len()));
|
||||
}
|
||||
for (i, p) in self.phases.iter().enumerate() {
|
||||
if !PLANNABLE_KINDS.contains(&p.kind.as_str()) {
|
||||
return Err(Refusal::UnknownKind {
|
||||
index: i,
|
||||
kind: p.kind.clone(),
|
||||
});
|
||||
}
|
||||
if p.task.trim().is_empty() {
|
||||
return Err(Refusal::BlankTask(i));
|
||||
}
|
||||
// Every key this phase would write must have a reader. The plan is
|
||||
// built from typed fields, so this can only fail if a field is added
|
||||
// here without a corresponding entry in the registry — which is
|
||||
// exactly the drift worth failing on.
|
||||
if let Some(key) = crate::phase_config::inert_keys(&p.config()).into_iter().next() {
|
||||
return Err(Refusal::InertKey { index: i, key });
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The phases as `(kind, order_idx, config)`, ready for mission creation.
|
||||
///
|
||||
/// `order_idx` is the array position rather than a field the model sets:
|
||||
/// two sources for one fact is how a plan ends up with two phase 0s.
|
||||
pub fn phases(&self) -> Vec<(String, i32, serde_json::Value)> {
|
||||
self.phases
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, p)| (p.kind.clone(), i as i32, p.config()))
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
impl PlannedPhase {
|
||||
/// This phase's `mission_phases.config`.
|
||||
fn config(&self) -> serde_json::Value {
|
||||
let mut o = serde_json::Map::new();
|
||||
o.insert("task".into(), serde_json::Value::String(self.task.clone()));
|
||||
if let Some(d) = self.done_when.as_deref().map(str::trim).filter(|s| !s.is_empty()) {
|
||||
o.insert("done_when".into(), serde_json::Value::String(d.to_string()));
|
||||
}
|
||||
if let Some(c) = self
|
||||
.done_when_check
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty())
|
||||
{
|
||||
o.insert(
|
||||
"done_when_check".into(),
|
||||
serde_json::Value::String(c.to_string()),
|
||||
);
|
||||
}
|
||||
if let Some(e) = self.allow_empty {
|
||||
o.insert("allow_empty".into(), serde_json::Value::Bool(e));
|
||||
}
|
||||
serde_json::Value::Object(o)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn phase(kind: &str, task: &str) -> PlannedPhase {
|
||||
PlannedPhase {
|
||||
kind: kind.into(),
|
||||
task: task.into(),
|
||||
done_when: None,
|
||||
done_when_check: None,
|
||||
allow_empty: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// A kind nothing dispatches on is the dangerous one: it does not error, it
|
||||
/// falls through to the generic purpose and runs as a nondescript phase that
|
||||
/// looks like it worked.
|
||||
#[test]
|
||||
fn an_invented_phase_kind_is_refused_naming_the_real_ones() {
|
||||
let p = Plan {
|
||||
phases: vec![phase("coding", "do it"), phase("review", "check it")],
|
||||
};
|
||||
let err = p.validate().unwrap_err();
|
||||
assert_eq!(
|
||||
err,
|
||||
Refusal::UnknownKind {
|
||||
index: 1,
|
||||
kind: "review".into()
|
||||
}
|
||||
);
|
||||
let msg = err.to_string();
|
||||
for kind in PLANNABLE_KINDS {
|
||||
assert!(msg.contains(kind), "the message must name {kind}: {msg}");
|
||||
}
|
||||
// And every kind the runner dispatches on is accepted, so this cannot
|
||||
// drift from what `phase_runner` can actually execute.
|
||||
for kind in PLANNABLE_KINDS {
|
||||
assert!(Plan { phases: vec![phase(kind, "work")] }.validate().is_ok(), "{kind}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Every key a planned phase writes must have a reader. This is the whole
|
||||
/// reason `phase_config` exists — a key nothing consumes is stored,
|
||||
/// rendered, and silently inert.
|
||||
#[test]
|
||||
fn every_key_a_plan_writes_is_one_something_reads() {
|
||||
let p = PlannedPhase {
|
||||
kind: "coding".into(),
|
||||
task: "add a module".into(),
|
||||
done_when: Some("the suite passes".into()),
|
||||
done_when_check: Some("cargo test".into()),
|
||||
allow_empty: Some(false),
|
||||
};
|
||||
let cfg = p.config();
|
||||
assert!(
|
||||
crate::phase_config::inert_keys(&cfg).is_empty(),
|
||||
"a planned phase must write only keys with readers: {:?}",
|
||||
crate::phase_config::inert_keys(&cfg)
|
||||
);
|
||||
assert!(
|
||||
crate::phase_config::unknown_keys(&cfg).is_empty(),
|
||||
"and only keys the registry knows: {:?}",
|
||||
crate::phase_config::unknown_keys(&cfg)
|
||||
);
|
||||
assert!(Plan { phases: vec![p] }.validate().is_ok());
|
||||
}
|
||||
|
||||
/// A blank task is the failure that produced identical output from two
|
||||
/// different phases — the agent gets the mission description and nothing
|
||||
/// saying which part is its own.
|
||||
#[test]
|
||||
fn a_phase_without_a_task_is_refused() {
|
||||
let p = Plan {
|
||||
phases: vec![phase("coding", " ")],
|
||||
};
|
||||
assert_eq!(p.validate(), Err(Refusal::BlankTask(0)));
|
||||
}
|
||||
|
||||
/// Bounded and non-empty. Each phase is a full agent run in sequence, and
|
||||
/// splitting one change into stages loses context at every handoff.
|
||||
#[test]
|
||||
fn a_plan_is_bounded_and_non_empty() {
|
||||
assert_eq!(Plan { phases: vec![] }.validate(), Err(Refusal::Empty));
|
||||
let many: Vec<_> = (0..MAX_PHASES + 1).map(|_| phase("coding", "work")).collect();
|
||||
assert_eq!(
|
||||
Plan { phases: many }.validate(),
|
||||
Err(Refusal::TooMany(MAX_PHASES + 1))
|
||||
);
|
||||
}
|
||||
|
||||
/// Order comes from the array, not from a field the model sets. Two sources
|
||||
/// for one fact is how a plan ends up with two phase 0s — and `order_idx`
|
||||
/// is what `start_pending_phases` sequences on.
|
||||
#[test]
|
||||
fn order_comes_from_the_arrays_own_order() {
|
||||
let p = Plan {
|
||||
phases: vec![
|
||||
phase("research", "read the code"),
|
||||
phase("coding", "change it"),
|
||||
phase("coding", "then this"),
|
||||
],
|
||||
};
|
||||
let out = p.phases();
|
||||
assert_eq!(
|
||||
out.iter().map(|(_, i, _)| *i).collect::<Vec<_>>(),
|
||||
vec![0, 1, 2]
|
||||
);
|
||||
assert_eq!(out[0].0, "research");
|
||||
assert_eq!(out[1].2["task"], "change it");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user