feat(missions): wire the workflow registry so phase config reaches the database
workflow_registry.rs had zero call sites -- lib.rs declared the module and
nothing ever called load() or get(). So templates/workflows/*.toml was never
read, and because the client's TEMPLATE_PRESETS carries only {kind, order_idx}
with no config, PhaseSpec.config defaulted to Value::Null and every
wizard-created mission stored a null mission_phases.config.
Every per-phase setting was therefore inert. `loop = "until_no_more_int_items"`
and `commit_policy = "on_green_tests"` described a scheduler that does not
exist AND had no path to the database. benchmark_runner and security_scan
already read phase_config(); they were reading from null.
- Mission create derives phases from the recipe when none are sent, and
backfills config per phase (matched on kind+order_idx, then kind) when the
caller sends shape without config. An explicit config always wins.
- phases_for_create takes Option<&WorkflowRecipe> rather than reaching for the
global, because the registry resolves its directory relative to the process
cwd -- which under cargo test is the crate root, not the repo root.
- GET /api/workflows serves the catalog; the wizard fetches it and falls back
to TEMPLATE_PRESETS. Adding a TOML now adds a template with no FE change.
- load() runs at boot so a malformed recipe appears in the boot log instead of
silently producing a mission with no phase config.
Also fixes a latent bug in all five recipes: `default_team_template` was
written below the first [[phases]] block, and TOML scopes a bare key after a
table header INTO that table -- so it parsed as
phases[last].config.default_team_template and the real field was always None.
Invisible while the registry was dead code. Moved above the phases, with a
test asserting it neither returns None nor leaks into a phase config.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
Omar Sobh
co-authored by
Claude Opus 5
Claude Opus 5
parent
d94487d3ba
commit
49bcf53b84
@@ -148,6 +148,72 @@ pub async fn list(
|
||||
))
|
||||
}
|
||||
|
||||
/// Resolve the phase list for a new mission, filling in each phase's `config`
|
||||
/// from the workflow recipe when the caller didn't supply one.
|
||||
///
|
||||
/// `mission_phases.config` is where per-phase settings live (`done_when`,
|
||||
/// `max_iterations`, `harness`, `tools`). The client's phase list historically
|
||||
/// carried only `{kind, order_idx}`, so every wizard-created mission landed
|
||||
/// with a null config and every recipe setting was silently inert. Callers can
|
||||
/// still override by sending a non-null config per phase.
|
||||
fn phases_for_create(
|
||||
recipe: Option<&crate::workflow_registry::WorkflowRecipe>,
|
||||
requested: Vec<PhaseSpec>,
|
||||
) -> Vec<NewMissionPhase> {
|
||||
// No phases requested: take the recipe's wholesale.
|
||||
if requested.is_empty() {
|
||||
return recipe
|
||||
.map(|r| {
|
||||
r.phases
|
||||
.iter()
|
||||
.map(|p| NewMissionPhase {
|
||||
kind: p.kind.clone(),
|
||||
order_idx: p.order_idx,
|
||||
config: p.config.clone(),
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
}
|
||||
|
||||
// Phases requested: honour them, but backfill config from the matching
|
||||
// recipe phase (by kind + order_idx, falling back to kind alone) so a
|
||||
// client that only knows the shape still gets the recipe's settings.
|
||||
requested
|
||||
.into_iter()
|
||||
.map(|p| {
|
||||
let config = if p.config.is_null() {
|
||||
recipe
|
||||
.and_then(|r| {
|
||||
r.phases
|
||||
.iter()
|
||||
.find(|rp| rp.kind == p.kind && rp.order_idx == p.order_idx)
|
||||
.or_else(|| r.phases.iter().find(|rp| rp.kind == p.kind))
|
||||
})
|
||||
.map(|rp| rp.config.clone())
|
||||
.unwrap_or(Value::Null)
|
||||
} else {
|
||||
p.config
|
||||
};
|
||||
NewMissionPhase {
|
||||
kind: p.kind,
|
||||
order_idx: p.order_idx,
|
||||
config,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// `GET /api/workflows` — the workflow recipe catalog.
|
||||
///
|
||||
/// Serves `templates/workflows/*.toml` so the client can drop its inline
|
||||
/// mirror of the phase composition table.
|
||||
pub async fn list_workflows(
|
||||
Authed(_user): Authed,
|
||||
) -> Json<&'static [crate::workflow_registry::WorkflowRecipe]> {
|
||||
Json(crate::workflow_registry::load())
|
||||
}
|
||||
|
||||
pub async fn create(
|
||||
State(state): State<AppState>,
|
||||
Authed(user): Authed,
|
||||
@@ -180,15 +246,10 @@ pub async fn create(
|
||||
config: body.config,
|
||||
runtime_kind: Some(runtime_kind),
|
||||
target_node_id: body.target_node_id,
|
||||
phases: body
|
||||
.phases
|
||||
.into_iter()
|
||||
.map(|p| NewMissionPhase {
|
||||
kind: p.kind,
|
||||
order_idx: p.order_idx,
|
||||
config: p.config,
|
||||
})
|
||||
.collect(),
|
||||
phases: phases_for_create(
|
||||
crate::workflow_registry::get(body.template_kind.trim()),
|
||||
body.phases,
|
||||
),
|
||||
};
|
||||
let id = cm_db::repo::missions::insert(&state.pool, new).await?;
|
||||
let mission = cm_db::repo::missions::get(&state.pool, id, user.workspace_id.as_uuid())
|
||||
@@ -918,6 +979,99 @@ pub async fn get_document(
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The whole point of wiring the registry: a client that sends only the
|
||||
/// phase shape must still get the recipe's config, because that is where
|
||||
/// per-phase settings are read from at run time. Before this, every
|
||||
/// wizard-created mission stored a null config and every recipe setting
|
||||
/// was inert.
|
||||
#[test]
|
||||
fn phase_config_is_backfilled_from_the_recipe() {
|
||||
let recipe = test_recipe();
|
||||
let requested = vec![
|
||||
PhaseSpec {
|
||||
kind: "research".into(),
|
||||
order_idx: 0,
|
||||
config: Value::Null,
|
||||
},
|
||||
PhaseSpec {
|
||||
kind: "coding".into(),
|
||||
order_idx: 1,
|
||||
config: Value::Null,
|
||||
},
|
||||
];
|
||||
let phases = phases_for_create(Some(&recipe), requested);
|
||||
assert_eq!(phases.len(), 2);
|
||||
assert!(
|
||||
phases.iter().all(|p| !p.config.is_null()),
|
||||
"recipe config was not backfilled: {phases:?}"
|
||||
);
|
||||
// The coding phase's loop policy is the setting the loop work depends on.
|
||||
let coding = phases.iter().find(|p| p.kind == "coding").expect("coding");
|
||||
assert_eq!(
|
||||
coding.config.get("loop").and_then(|v| v.as_str()),
|
||||
Some("until_no_more_int_items")
|
||||
);
|
||||
}
|
||||
|
||||
/// Omitting phases entirely takes the recipe's list wholesale.
|
||||
#[test]
|
||||
fn phases_default_to_the_recipe() {
|
||||
let phases = phases_for_create(Some(&test_recipe()), vec![]);
|
||||
assert_eq!(phases.len(), 2);
|
||||
assert_eq!(phases[0].kind, "research");
|
||||
assert_eq!(phases[1].kind, "coding");
|
||||
}
|
||||
|
||||
/// An explicit config always wins over the recipe's.
|
||||
#[test]
|
||||
fn explicit_phase_config_is_not_overwritten() {
|
||||
let requested = vec![PhaseSpec {
|
||||
kind: "coding".into(),
|
||||
order_idx: 1,
|
||||
config: serde_json::json!({"loop": "single_pass"}),
|
||||
}];
|
||||
let phases = phases_for_create(Some(&test_recipe()), requested);
|
||||
assert_eq!(
|
||||
phases[0].config.get("loop").and_then(|v| v.as_str()),
|
||||
Some("single_pass")
|
||||
);
|
||||
}
|
||||
|
||||
/// An unknown template must not fabricate phases or panic.
|
||||
#[test]
|
||||
fn unknown_template_yields_no_phases() {
|
||||
assert!(phases_for_create(None, vec![]).is_empty());
|
||||
}
|
||||
|
||||
/// Mirrors `templates/workflows/research_and_code.toml`. Built inline
|
||||
/// rather than loaded from disk because the registry resolves its
|
||||
/// directory relative to the process cwd, which under `cargo test` is the
|
||||
/// crate root, not the repo root.
|
||||
fn test_recipe() -> crate::workflow_registry::WorkflowRecipe {
|
||||
crate::workflow_registry::WorkflowRecipe {
|
||||
key: "research_and_code".into(),
|
||||
title: "Research + Coding Loop".into(),
|
||||
blurb: String::new(),
|
||||
requires_repo: true,
|
||||
default_team_template: Some("rust_sdlc".into()),
|
||||
phases: vec![
|
||||
crate::workflow_registry::WorkflowPhase {
|
||||
kind: "research".into(),
|
||||
order_idx: 0,
|
||||
config: serde_json::json!({"produces": ["md", "pdf"]}),
|
||||
},
|
||||
crate::workflow_registry::WorkflowPhase {
|
||||
kind: "coding".into(),
|
||||
order_idx: 1,
|
||||
config: serde_json::json!({
|
||||
"loop": "until_no_more_int_items",
|
||||
"commit_policy": "on_green_tests"
|
||||
}),
|
||||
},
|
||||
],
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn title_prefers_first_markdown_heading() {
|
||||
let body = "I'll start by exploring.\n\n# ClawHDF5 Research Report\n\ntext";
|
||||
|
||||
Reference in New Issue
Block a user