Skip to content

Commit 3a4dced

Browse files
authored
config phase-set + full --json error chain (rc.13, TASK-390) (#324)
* feat(workflow-config): full anyhow chain in --json errors + `config phase-set` Fix A: emit_cli_error now builds the --json error.message via anyhow's alternate Display (`{:#}`) instead of `err.to_string()`, so the message carries the full context chain (outer .context() + every underlying source) matching the human path. Previously only the top-level context surfaced, dropping the real cause (e.g. the "references unknown phase" detail behind "the config to write is invalid..."). Fix B: add `animus workflow config phase-set --id <id> --input-json <json>`, mirroring agent-set/workflow-set. It read-modify-writes the RAW config_source base's `WorkflowConfig.phase_definitions` (NOT the agent-runtime overlay that `workflow phases upsert` writes), so a subsequently-set workflow that references the phase resolves at post-pack-merge validation instead of failing "references unknown phase". Case-insensitive upsert to match phase-id resolution elsewhere. Tests: json_error_message chain assertion; phase-set JSON-error handler; phase-authored-on-base lets a referencing workflow validate (+ control). Docs: docs/reference/cli/index.md. * release: bump orchestrator-cli to v0.7.0-rc.13 (config phase-set + full --json error chain, TASK-390)
1 parent f033853 commit 3a4dced

10 files changed

Lines changed: 163 additions & 8 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/orchestrator-cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "orchestrator-cli"
3-
version = "0.7.0-rc.12"
3+
version = "0.7.0-rc.13"
44
edition = "2021"
55
license = "Elastic-2.0"
66
default-run = "animus"

crates/orchestrator-cli/src/cli_types/workflow_types.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,11 @@ pub(crate) enum WorkflowConfigCommand {
170170
AgentSet(WorkflowConfigAgentSetArgs),
171171
/// Remove one agent definition (read-modify-write the full config).
172172
AgentRemove(WorkflowConfigEntityIdArgs),
173+
/// Create or replace one phase definition on the config_source base
174+
/// (read-modify-write). Writes `WorkflowConfig.phase_definitions`, NOT the
175+
/// agent-runtime overlay that `workflow phases upsert` writes — so a
176+
/// subsequently-set workflow that references the phase validates.
177+
PhaseSet(WorkflowConfigPhaseSetArgs),
173178
/// Create or replace one workflow definition (read-modify-write).
174179
WorkflowSet(WorkflowConfigWorkflowSetArgs),
175180
/// Remove one workflow definition (read-modify-write).
@@ -204,6 +209,18 @@ pub(crate) struct WorkflowConfigAgentSetArgs {
204209
pub(crate) input_json: String,
205210
}
206211

212+
#[derive(Debug, Args)]
213+
pub(crate) struct WorkflowConfigPhaseSetArgs {
214+
#[arg(long, value_name = "PHASE_ID", help = "Phase definition id to create or replace.")]
215+
pub(crate) id: String,
216+
#[arg(
217+
long,
218+
value_name = "JSON",
219+
help = "Phase execution definition JSON payload (the value of phase_definitions.<id>)."
220+
)]
221+
pub(crate) input_json: String,
222+
}
223+
207224
#[derive(Debug, Args)]
208225
pub(crate) struct WorkflowConfigWorkflowSetArgs {
209226
#[arg(long, value_name = "JSON", help = "Workflow definition JSON payload (must include an 'id' field).")]

crates/orchestrator-cli/src/services/operations/ops_workflow/config.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,17 @@ pub(crate) fn set_config_agent_payload(project_root: &str, id: &str, input_json:
557557
write_back_payload(project_root, &config)
558558
}
559559

560+
/// `animus workflow config phase-set` — upsert one phase definition on the
561+
/// config_source base (`WorkflowConfig.phase_definitions`), NOT the agent-runtime
562+
/// overlay that `workflow phases upsert` writes. A workflow set afterwards that
563+
/// references this phase then resolves during post-pack-merge validation.
564+
pub(crate) fn set_config_phase_payload(project_root: &str, id: &str, input_json: &str) -> Result<Value> {
565+
let definition: orchestrator_config::PhaseExecutionDefinition = serde_json::from_str(input_json)
566+
.context("invalid phase execution definition JSON for workflow config phase-set")?;
567+
let config = orchestrator_core::set_phase_definition(Path::new(project_root), id, definition)?;
568+
write_back_payload(project_root, &config)
569+
}
570+
560571
/// `animus workflow config agent-remove` — remove one agent definition.
561572
pub(crate) fn remove_config_agent_payload(project_root: &str, id: &str) -> Result<Value> {
562573
let config = orchestrator_core::remove_agent_profile(Path::new(project_root), id)?;

crates/orchestrator-cli/src/services/operations/ops_workflow/mod.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,9 @@ pub(crate) async fn handle_workflow(
756756
WorkflowConfigCommand::AgentRemove(args) => {
757757
print_value(config::remove_config_agent_payload(project_root, &args.id)?, json)
758758
}
759+
WorkflowConfigCommand::PhaseSet(args) => {
760+
print_value(config::set_config_phase_payload(project_root, &args.id, &args.input_json)?, json)
761+
}
759762
WorkflowConfigCommand::WorkflowSet(args) => {
760763
print_value(config::set_config_workflow_payload(project_root, &args.input_json)?, json)
761764
}
@@ -1023,6 +1026,17 @@ mod tests {
10231026
assert!(message.contains("workflow agent-runtime set --help"));
10241027
}
10251028

1029+
#[test]
1030+
fn set_config_phase_payload_reports_actionable_json_error() {
1031+
let error =
1032+
set_config_phase_payload("/tmp/unused", "my-phase", "{invalid").expect_err("invalid payload should fail");
1033+
let message = format!("{error:#}");
1034+
assert!(
1035+
message.contains("invalid phase execution definition JSON for workflow config phase-set"),
1036+
"expected actionable phase-set JSON error, got: {message}"
1037+
);
1038+
}
1039+
10261040
#[test]
10271041
fn resolve_workflow_run_dispatch_builds_custom_title_dispatch() {
10281042
let dispatch = resolve_workflow_run_dispatch(

crates/orchestrator-cli/src/shared/output.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ pub(crate) fn emit_cli_error(err: &anyhow::Error, json: bool) {
8080
let envelope = CliErrorEnvelope {
8181
schema: CLI_SCHEMA,
8282
ok: false,
83-
error: CliErrorBody { code: code.to_string(), message: err.to_string(), exit_code, details },
83+
error: CliErrorBody { code: code.to_string(), message: json_error_message(err), exit_code, details },
8484
};
8585
eprintln!("{}", serialize_compact_json(&envelope).unwrap_or_else(|_| {
8686
format!("{{\"schema\":\"{}\",\"ok\":false,\"error\":{{\"code\":\"internal\",\"message\":\"serialization failure\",\"exit_code\":1}}}}", CLI_SCHEMA_ID)
@@ -93,6 +93,17 @@ pub(crate) fn emit_cli_error(err: &anyhow::Error, json: bool) {
9393
}
9494
}
9595

96+
/// Build the `--json` error message from an anyhow error.
97+
///
98+
/// Uses anyhow's alternate Display (`{:#}`) so the message carries the FULL
99+
/// context chain — the outer `.context(...)` plus every underlying source,
100+
/// joined by ": " — matching what the human (`error: {:#}`) path prints. Plain
101+
/// `err.to_string()` yields ONLY the top-level context and silently drops the
102+
/// real cause, which made `--json` consumers blind to the underlying failure.
103+
fn json_error_message(err: &anyhow::Error) -> String {
104+
format!("{err:#}")
105+
}
106+
96107
fn should_emit_help_hint(message: &str) -> bool {
97108
!message.to_ascii_lowercase().contains("--help")
98109
}
@@ -216,6 +227,30 @@ mod tests {
216227
assert_eq!(classify_exit_code(&long), 2);
217228
}
218229

230+
#[test]
231+
fn json_error_message_includes_full_context_chain() {
232+
use anyhow::Context;
233+
// Mirror the real failure shape: an inner cause wrapped by an outer
234+
// `.context(...)`. The --json message must surface BOTH — the outer
235+
// context and the inner source — not just the top-level context.
236+
let err = Err::<(), anyhow::Error>(anyhow!(
237+
"workflow 'X' references unknown phase 'Y'; add it to phase_catalog or define it under phases"
238+
))
239+
.context("the config to write is invalid once pack overlays are merged; refusing to persist")
240+
.unwrap_err();
241+
let message = json_error_message(&err);
242+
assert!(
243+
message.contains("the config to write is invalid once pack overlays are merged"),
244+
"message must contain the outer context, got: {message}"
245+
);
246+
assert!(
247+
message.contains("references unknown phase 'Y'"),
248+
"message must contain the inner source cause, got: {message}"
249+
);
250+
// Plain Display would drop the cause; assert we improved on it.
251+
assert_ne!(message, err.to_string(), "alternate Display must add the source chain");
252+
}
253+
219254
#[test]
220255
fn should_emit_help_hint_is_case_insensitive() {
221256
assert!(!should_emit_help_hint("Run with --HELP for usage"));

crates/orchestrator-config/src/workflow_config/config_write.rs

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::path::Path;
1212

1313
use anyhow::{anyhow, Context, Result};
1414

15-
use animus_config_protocol::agent_types::AgentProfileOverlay;
15+
use animus_config_protocol::agent_types::{AgentProfileOverlay, PhaseExecutionDefinition};
1616

1717
use super::config_source_client::{resolve_plugin_base, write_plugin_config};
1818
use super::loading::compile_workflow_config_onto_base;
@@ -98,6 +98,34 @@ pub fn remove_agent_profile(project_root: &Path, agent_id: &str) -> Result<Workf
9898
})
9999
}
100100

101+
/// Upsert (create or replace) a phase definition keyed by `phase_id` on the RAW
102+
/// config_source base's `phase_definitions`. This is the config-source authoring
103+
/// path — distinct from the agent-runtime overlay that `animus workflow phases
104+
/// upsert` writes. Writing here means a subsequently-set workflow that references
105+
/// the phase resolves during the post-pack-merge validation, instead of failing
106+
/// with "references unknown phase". The kernel validates the resulting full
107+
/// config before it is written.
108+
pub fn set_phase_definition(
109+
project_root: &Path,
110+
phase_id: &str,
111+
definition: PhaseExecutionDefinition,
112+
) -> Result<WorkflowConfig> {
113+
let phase_id = phase_id.trim();
114+
if phase_id.is_empty() {
115+
return Err(anyhow!("phase id must not be empty"));
116+
}
117+
read_modify_write(project_root, |config| {
118+
// Phase ids resolve case-insensitively everywhere else (validation's
119+
// reference check and the runtime `phase_execution` lookup both use
120+
// `eq_ignore_ascii_case`). Drop any existing key that differs only by
121+
// case before inserting, so an upsert REPLACES the phase instead of
122+
// leaving a stale duplicate that could win by map order.
123+
config.phase_definitions.retain(|existing, _| !existing.eq_ignore_ascii_case(phase_id));
124+
config.phase_definitions.insert(phase_id.to_string(), definition);
125+
Ok(())
126+
})
127+
}
128+
101129
/// Upsert (create or replace) a workflow definition by its `id`. The definition
102130
/// replaces any existing entry with the same id; otherwise it is appended.
103131
pub fn upsert_workflow_definition(project_root: &Path, definition: WorkflowDefinition) -> Result<WorkflowConfig> {
@@ -223,6 +251,46 @@ mod tests {
223251
assert!(msg.contains("invalid"), "error must flag the invalid config, got: {msg}");
224252
}
225253

254+
#[test]
255+
fn phase_authored_on_base_lets_a_referencing_workflow_validate() {
256+
// The whole point of `set_phase_definition`: a phase written to the
257+
// config_source base's `phase_definitions` must resolve when a
258+
// subsequently-set workflow references it — no "references unknown
259+
// phase". We drive the validate gate directly (write_full_workflow_config
260+
// validates BEFORE it attempts the plugin write), so phase-reference
261+
// resolution is exercised without a live writable plugin.
262+
let dir = tempfile::tempdir().expect("tempdir");
263+
let mut config = builtin_workflow_config();
264+
// Minimal valid phase definition (agent mode needs no agent_id).
265+
let phase: PhaseExecutionDefinition = serde_json::from_str(r#"{"mode":"agent"}"#).expect("valid phase json");
266+
config.phase_definitions.insert("authored-phase".to_string(), phase);
267+
let mut wf = sample_workflow("uses-authored");
268+
wf.phases =
269+
vec![animus_config_protocol::workflow_types::WorkflowPhaseEntry::Simple("authored-phase".to_string())];
270+
config.workflows = vec![wf];
271+
272+
// Validation PASSES: the only remaining failure is the write step (no
273+
// writable config_source plugin under test), never an "unknown phase".
274+
let err = write_full_workflow_config(dir.path(), &config).expect_err("no writable source under test");
275+
let msg = format!("{err:#}");
276+
assert!(!msg.contains("unknown phase"), "phase authored on the base must resolve; got: {msg}");
277+
assert!(
278+
msg.contains("no config_source plugin") || msg.contains("does not support writes"),
279+
"should fail only at the write step, got: {msg}"
280+
);
281+
282+
// Control: WITHOUT the phase definition the same workflow reference is
283+
// rejected as unknown — proving it is the authored phase that unblocks it.
284+
let mut missing = builtin_workflow_config();
285+
let mut wf2 = sample_workflow("uses-authored");
286+
wf2.phases =
287+
vec![animus_config_protocol::workflow_types::WorkflowPhaseEntry::Simple("authored-phase".to_string())];
288+
missing.workflows = vec![wf2];
289+
let err2 = write_full_workflow_config(dir.path(), &missing).expect_err("unknown phase must be rejected");
290+
let msg2 = format!("{err2:#}");
291+
assert!(msg2.contains("unknown phase"), "control must fail as unknown phase; got: {msg2}");
292+
}
293+
226294
#[test]
227295
fn write_to_non_writable_or_absent_source_is_actionable() {
228296
// A VALID config that passes the validate gate then hits the write path.

crates/orchestrator-config/src/workflow_config/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ pub use animus_config_protocol::overlay::{
6161
// ---------------------------------------------------------------------------
6262

6363
pub use config_write::{
64-
remove_agent_profile, remove_workflow_definition, upsert_agent_profile, upsert_workflow_definition,
65-
write_full_workflow_config,
64+
remove_agent_profile, remove_workflow_definition, set_phase_definition, upsert_agent_profile,
65+
upsert_workflow_definition, write_full_workflow_config,
6666
};
6767
pub use environment_routing::resolve_environment;
6868
pub use loading::{

crates/orchestrator-core/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ pub use workflow_config::{
170170
load_workflow_config_with_metadata, merge_yaml_into_config, missing_project_skill_reference_warnings,
171171
missing_skill_reference_warnings_for_sources, missing_skill_yaml_warnings, parse_yaml_workflow_config,
172172
remove_agent_profile, remove_generated_workflow_phase, remove_workflow_definition, resolve_workflow_phase_plan,
173-
resolve_workflow_rework_attempts, resolve_workflow_skip_guards, resolve_workflow_variables,
173+
resolve_workflow_rework_attempts, resolve_workflow_skip_guards, resolve_workflow_variables, set_phase_definition,
174174
resolve_workflow_verdict_routing, unenforced_project_yaml_warnings, unenforced_yaml_field_warnings,
175175
upsert_agent_profile, upsert_generated_workflow_phase, upsert_generated_workflow_pipeline,
176176
upsert_workflow_definition, validate_and_compile_yaml_workflows, validate_workflow_and_runtime_configs,

docs/reference/cli/index.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ animus
186186
│ │ ├── set Replace the full config via the writable config_source plugin (validates first; rejected on read-only sources)
187187
│ │ ├── agent-set Create or replace one agent definition (read-modify-write the full config)
188188
│ │ ├── agent-remove Remove one agent definition (read-modify-write the full config)
189+
│ │ ├── phase-set Create or replace one phase definition on the config_source base (read-modify-write; writes phase_definitions, not the agent-runtime overlay)
189190
│ │ ├── workflow-set Create or replace one workflow definition (read-modify-write)
190191
│ │ └── workflow-remove Remove one workflow definition (read-modify-write)
191192
│ ├── state-machine
@@ -1925,6 +1926,15 @@ the plugin — nothing is partially written.
19251926
model back. This is the **definition**-management verb; it does not collide
19261927
with the runtime `animus agent {list,get,run,...}` surface.
19271928
- `animus workflow config agent-remove --id <id>` — remove one agent.
1929+
- `animus workflow config phase-set --id <phase_id> --input-json <json>` — upsert
1930+
one phase definition on the RAW config_source base
1931+
(`WorkflowConfig.phase_definitions`). Read-modify-write, same validate-before-write
1932+
path as the other entity verbs. This writes the **config_source base**, NOT the
1933+
agent-runtime overlay that `animus workflow phases upsert` writes — so a phase
1934+
authored here resolves when a subsequently-set workflow references it, instead of
1935+
failing `workflow-set` with "references unknown phase". The JSON is a
1936+
`PhaseExecutionDefinition` (the value of `phase_definitions.<id>`), e.g.
1937+
`--input-json '{"mode":"agent","agent_id":"builder"}'`.
19281938
- `animus workflow config workflow-set --input-json <json>` — upsert one
19291939
workflow definition (the JSON must include an `id`).
19301940
- `animus workflow config workflow-remove --id <id>` — remove one workflow.
@@ -1941,7 +1951,7 @@ watch (gated on the `config_watch` capability).
19411951
| Metric | Count |
19421952
|---|---|
19431953
| Top-level commands | 28 |
1944-
| Nested command entries (all levels) | 214 |
1954+
| Nested command entries (all levels) | 215 |
19451955

19461956
Counting basis: counts are derived from the command tree above. "Top-level
19471957
commands" counts the column-0 tree roots (`animus <command>`); "Nested

0 commit comments

Comments
 (0)