Files
clawmates/crates/cm-api/src/phase_config.rs
T
Omar SobhandClaude Opus 5 a33dbdcdc3 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]>
2026-08-06 20:05:28 -07:00

274 lines
10 KiB
Rust

//! Which phase-config keys the platform actually reads.
//!
//! `mission_phases.config` is free-form JSONB written by workflow recipes, the
//! mission wizard and the API. Nothing connected a key to the code that reads
//! it, so a key could be accepted, validated, stored, rendered — and consumed
//! by nobody.
//!
//! `task` was exactly that. Every phase of every mission received identical
//! instructions because the runner selected only the mission description; the
//! per-phase task sat in Postgres unread. Mission `019fc42b` is what surfaced
//! it: two coding phases with different `task` values produced the same two
//! files. There was no error, because there is nothing to fail — an unread key
//! is indistinguishable from a key whose value happens not to matter.
//!
//! This module is the missing link. Every key here names the code that reads
//! it, `unknown_keys` reports anything else, and a test asserts the shipped
//! recipes only write keys that exist. It cannot make a reader appear, but it
//! makes an absent one visible.
/// A phase-config key and where it is consumed.
pub struct KnownKey {
pub key: &'static str,
/// The code path that reads it. Kept as prose so this survives refactors
/// that a symbol reference would not.
pub read_by: &'static str,
}
/// Keys with a reader in the current build.
///
/// Adding a key here without a reader defeats the purpose. The rule is: a key
/// earns its entry when something consumes it, not when something writes it.
pub const KNOWN_KEYS: &[KnownKey] = &[
KnownKey {
key: "done_when",
read_by: "cm_db::repo::missions::create — promoted to the done_when column, \
swept by phase_runner::evaluate_finished_phases",
},
KnownKey {
key: "max_iterations",
read_by: "cm_db::repo::missions::create — promoted to the max_iterations column",
},
KnownKey {
key: "task",
read_by: "phase_runner::start_pending_phases — injected by phase_task_text",
},
KnownKey {
key: "commit_policy",
read_by: "mission_delivery::Gate::parse — selects the delivery gate",
},
KnownKey {
key: "allow_empty",
read_by: "phase_runner::empty_delivery_is_a_failure — when true, a coding \
phase that changes no files still completes; also vm_stop_gate::\
StopGate::for_phase, where it drops the in-loop delivery check",
},
KnownKey {
key: "done_when_check",
read_by: "vm_stop_gate::StopGate::for_phase — a shell command the agent's \
`Stop` hook runs, refusing the stop while it exits non-zero",
},
];
/// Keys a recipe may carry that are deliberately not consumed *yet*.
///
/// Distinguished from unknown keys so the report stays useful: these are known
/// gaps with an owner, not typos. Every one is a feature described in a shipped
/// workflow recipe whose implementation does not exist — which is worth seeing
/// listed, because a recipe promising `loop = "until_done"` reads to an
/// operator like something that loops.
pub const DECLARED_BUT_UNREAD: &[KnownKey] = &[
KnownKey {
key: "loop",
read_by: "NOT IMPLEMENTED — phase iteration uses max_iterations + done_when",
},
KnownKey {
key: "produces",
read_by: "NOT IMPLEMENTED — artifact rendering is not driven by this",
},
KnownKey {
key: "input_from_phase",
read_by: "NOT IMPLEMENTED — phases share a checkout, not declared inputs",
},
KnownKey {
key: "mode",
read_by: "NOT IMPLEMENTED — benchmark/refactor mode selection",
},
KnownKey {
key: "harness",
read_by: "NOT IMPLEMENTED — benchmark harness selection",
},
KnownKey {
key: "tools",
read_by: "NOT IMPLEMENTED — per-phase tool selection",
},
KnownKey {
key: "benchmark",
read_by: "NOT IMPLEMENTED — nested benchmark settings",
},
KnownKey {
key: "mcp_bundles",
read_by: "NOT IMPLEMENTED at phase level — bundles come from the TEAM \
template (mission_orchestrator binds template.mcp_bundles) and \
runtime_provision writes agents.<alias>.mcp_bundles. A recipe \
setting this per phase changes nothing: security_hardening.toml \
asks for gitea_forge + security_scan and its phase gets neither",
},
KnownKey {
key: "test_command",
read_by: "NOT IMPLEMENTED — mission_delivery::discover_test_command infers \
from the repo and does not consult config",
},
];
fn is_listed(key: &str, list: &[KnownKey]) -> bool {
list.iter().any(|k| k.key == key)
}
/// Keys in this config that no code reads and that are not known gaps.
///
/// Almost always a typo or a setting invented for a feature that was never
/// built. Returned rather than rejected: a mission whose config carries an
/// unread key is not *wrong*, it is just doing less than its author believes,
/// and failing the request would break recipes that already ship these.
pub fn unknown_keys(config: &serde_json::Value) -> Vec<String> {
let Some(obj) = config.as_object() else {
return Vec::new();
};
obj.keys()
.filter(|k| !is_listed(k, KNOWN_KEYS) && !is_listed(k, DECLARED_BUT_UNREAD))
.cloned()
.collect()
}
/// Keys that are recognised but that nothing consumes.
pub fn inert_keys(config: &serde_json::Value) -> Vec<String> {
let Some(obj) = config.as_object() else {
return Vec::new();
};
obj.keys()
.filter(|k| is_listed(k, DECLARED_BUT_UNREAD))
.cloned()
.collect()
}
/// Log what a phase's config asked for that will not happen.
///
/// Called once per phase at mission creation. Deliberately not an error: the
/// point is that the author's intent and the platform's behaviour have
/// diverged, and the author should be able to see that without being blocked.
pub fn report(kind: &str, order_idx: i32, config: &serde_json::Value) {
let unknown = unknown_keys(config);
if !unknown.is_empty() {
eprintln!(
"phase_config: phase {order_idx} ({kind}) sets unrecognised key(s) {} — \
nothing reads them; check for a typo",
unknown.join(", ")
);
}
let inert = inert_keys(config);
if !inert.is_empty() {
eprintln!(
"phase_config: phase {order_idx} ({kind}) sets {} — recognised but NOT \
IMPLEMENTED, so it will have no effect on this run",
inert.join(", ")
);
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn a_key_cannot_be_both_read_and_unread() {
for k in KNOWN_KEYS {
assert!(
!is_listed(k.key, DECLARED_BUT_UNREAD),
"{} is listed as both read and unread",
k.key
);
}
}
#[test]
fn every_known_key_names_its_reader() {
for k in KNOWN_KEYS {
assert!(
!k.read_by.is_empty() && !k.read_by.starts_with("NOT IMPLEMENTED"),
"{} claims to be read but names no reader",
k.key
);
}
for k in DECLARED_BUT_UNREAD {
assert!(
k.read_by.starts_with("NOT IMPLEMENTED"),
"{} is listed as unread but names a reader — promote it to KNOWN_KEYS",
k.key
);
}
}
/// The regression that motivated the module: `task` must stay claimed.
#[test]
fn the_per_phase_task_key_has_a_reader() {
assert!(
is_listed("task", KNOWN_KEYS),
"task lost its reader again — every phase will get identical instructions"
);
}
#[test]
fn unknown_and_inert_keys_are_reported_separately() {
let cfg = serde_json::json!({
"done_when": "tests pass",
"loop": "until_done",
"typpo": true,
});
assert_eq!(unknown_keys(&cfg), vec!["typpo".to_string()]);
assert_eq!(inert_keys(&cfg), vec!["loop".to_string()]);
}
/// Every key the shipped workflow recipes write must be accounted for.
///
/// This is the CI-time half: a recipe that invents `comit_policy` should
/// fail here rather than run a mission whose gate silently defaults.
#[test]
fn shipped_recipes_only_write_accounted_keys() {
let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/../../templates/workflows");
let Ok(entries) = std::fs::read_dir(dir) else {
return; // templates not present in this build context
};
// Keys that belong to the recipe/phase envelope rather than to the
// phase config blob itself.
const ENVELOPE: &[&str] = &[
"key",
"name",
"title",
"blurb",
"kind",
"order_idx",
"requires_repo",
"default_team_template",
"default_topology",
"phases",
"description",
];
for entry in entries.flatten() {
let path = entry.path();
if path.extension().and_then(|e| e.to_str()) != Some("toml") {
continue;
}
let body = std::fs::read_to_string(&path).unwrap();
for line in body.lines() {
let line = line.trim();
if line.starts_with('#') || !line.contains('=') {
continue;
}
let key = line.split('=').next().unwrap().trim();
if key.is_empty() || key.contains(' ') || key.contains('[') {
continue;
}
let accounted = ENVELOPE.contains(&key)
|| is_listed(key, KNOWN_KEYS)
|| is_listed(key, DECLARED_BUT_UNREAD);
assert!(
accounted,
"{} writes `{key}`, which no reader claims and no gap declares",
path.display()
);
}
}
}
}