Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/orchestrator-cli/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "orchestrator-cli"
version = "0.7.0-rc.12"
version = "0.7.0-rc.13"
edition = "2021"
license = "Elastic-2.0"
default-run = "animus"
Expand Down
17 changes: 17 additions & 0 deletions crates/orchestrator-cli/src/cli_types/workflow_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ pub(crate) enum WorkflowConfigCommand {
AgentSet(WorkflowConfigAgentSetArgs),
/// Remove one agent definition (read-modify-write the full config).
AgentRemove(WorkflowConfigEntityIdArgs),
/// Create or replace one phase definition on the config_source base
/// (read-modify-write). Writes `WorkflowConfig.phase_definitions`, NOT the
/// agent-runtime overlay that `workflow phases upsert` writes — so a
/// subsequently-set workflow that references the phase validates.
PhaseSet(WorkflowConfigPhaseSetArgs),
/// Create or replace one workflow definition (read-modify-write).
WorkflowSet(WorkflowConfigWorkflowSetArgs),
/// Remove one workflow definition (read-modify-write).
Expand Down Expand Up @@ -204,6 +209,18 @@ pub(crate) struct WorkflowConfigAgentSetArgs {
pub(crate) input_json: String,
}

#[derive(Debug, Args)]
pub(crate) struct WorkflowConfigPhaseSetArgs {
#[arg(long, value_name = "PHASE_ID", help = "Phase definition id to create or replace.")]
pub(crate) id: String,
#[arg(
long,
value_name = "JSON",
help = "Phase execution definition JSON payload (the value of phase_definitions.<id>)."
)]
pub(crate) input_json: String,
}

#[derive(Debug, Args)]
pub(crate) struct WorkflowConfigWorkflowSetArgs {
#[arg(long, value_name = "JSON", help = "Workflow definition JSON payload (must include an 'id' field).")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,17 @@ pub(crate) fn set_config_agent_payload(project_root: &str, id: &str, input_json:
write_back_payload(project_root, &config)
}

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

/// `animus workflow config agent-remove` — remove one agent definition.
pub(crate) fn remove_config_agent_payload(project_root: &str, id: &str) -> Result<Value> {
let config = orchestrator_core::remove_agent_profile(Path::new(project_root), id)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,9 @@ pub(crate) async fn handle_workflow(
WorkflowConfigCommand::AgentRemove(args) => {
print_value(config::remove_config_agent_payload(project_root, &args.id)?, json)
}
WorkflowConfigCommand::PhaseSet(args) => {
print_value(config::set_config_phase_payload(project_root, &args.id, &args.input_json)?, json)
}
WorkflowConfigCommand::WorkflowSet(args) => {
print_value(config::set_config_workflow_payload(project_root, &args.input_json)?, json)
}
Expand Down Expand Up @@ -1023,6 +1026,17 @@ mod tests {
assert!(message.contains("workflow agent-runtime set --help"));
}

#[test]
fn set_config_phase_payload_reports_actionable_json_error() {
let error =
set_config_phase_payload("/tmp/unused", "my-phase", "{invalid").expect_err("invalid payload should fail");
let message = format!("{error:#}");
assert!(
message.contains("invalid phase execution definition JSON for workflow config phase-set"),
"expected actionable phase-set JSON error, got: {message}"
);
}

#[test]
fn resolve_workflow_run_dispatch_builds_custom_title_dispatch() {
let dispatch = resolve_workflow_run_dispatch(
Expand Down
37 changes: 36 additions & 1 deletion crates/orchestrator-cli/src/shared/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub(crate) fn emit_cli_error(err: &anyhow::Error, json: bool) {
let envelope = CliErrorEnvelope {
schema: CLI_SCHEMA,
ok: false,
error: CliErrorBody { code: code.to_string(), message: err.to_string(), exit_code, details },
error: CliErrorBody { code: code.to_string(), message: json_error_message(err), exit_code, details },
};
eprintln!("{}", serialize_compact_json(&envelope).unwrap_or_else(|_| {
format!("{{\"schema\":\"{}\",\"ok\":false,\"error\":{{\"code\":\"internal\",\"message\":\"serialization failure\",\"exit_code\":1}}}}", CLI_SCHEMA_ID)
Expand All @@ -93,6 +93,17 @@ pub(crate) fn emit_cli_error(err: &anyhow::Error, json: bool) {
}
}

/// Build the `--json` error message from an anyhow error.
///
/// Uses anyhow's alternate Display (`{:#}`) so the message carries the FULL
/// context chain — the outer `.context(...)` plus every underlying source,
/// joined by ": " — matching what the human (`error: {:#}`) path prints. Plain
/// `err.to_string()` yields ONLY the top-level context and silently drops the
/// real cause, which made `--json` consumers blind to the underlying failure.
fn json_error_message(err: &anyhow::Error) -> String {
format!("{err:#}")
}

fn should_emit_help_hint(message: &str) -> bool {
!message.to_ascii_lowercase().contains("--help")
}
Expand Down Expand Up @@ -216,6 +227,30 @@ mod tests {
assert_eq!(classify_exit_code(&long), 2);
}

#[test]
fn json_error_message_includes_full_context_chain() {
use anyhow::Context;
// Mirror the real failure shape: an inner cause wrapped by an outer
// `.context(...)`. The --json message must surface BOTH — the outer
// context and the inner source — not just the top-level context.
let err = Err::<(), anyhow::Error>(anyhow!(
"workflow 'X' references unknown phase 'Y'; add it to phase_catalog or define it under phases"
))
.context("the config to write is invalid once pack overlays are merged; refusing to persist")
.unwrap_err();
let message = json_error_message(&err);
assert!(
message.contains("the config to write is invalid once pack overlays are merged"),
"message must contain the outer context, got: {message}"
);
assert!(
message.contains("references unknown phase 'Y'"),
"message must contain the inner source cause, got: {message}"
);
// Plain Display would drop the cause; assert we improved on it.
assert_ne!(message, err.to_string(), "alternate Display must add the source chain");
}

#[test]
fn should_emit_help_hint_is_case_insensitive() {
assert!(!should_emit_help_hint("Run with --HELP for usage"));
Expand Down
70 changes: 69 additions & 1 deletion crates/orchestrator-config/src/workflow_config/config_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::path::Path;

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

use animus_config_protocol::agent_types::AgentProfileOverlay;
use animus_config_protocol::agent_types::{AgentProfileOverlay, PhaseExecutionDefinition};

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

/// Upsert (create or replace) a phase definition keyed by `phase_id` on the RAW
/// config_source base's `phase_definitions`. This is the config-source authoring
/// path — distinct from the agent-runtime overlay that `animus workflow phases
/// upsert` writes. Writing here means a subsequently-set workflow that references
/// the phase resolves during the post-pack-merge validation, instead of failing
/// with "references unknown phase". The kernel validates the resulting full
/// config before it is written.
pub fn set_phase_definition(
project_root: &Path,
phase_id: &str,
definition: PhaseExecutionDefinition,
) -> Result<WorkflowConfig> {
let phase_id = phase_id.trim();
if phase_id.is_empty() {
return Err(anyhow!("phase id must not be empty"));
}
read_modify_write(project_root, |config| {
// Phase ids resolve case-insensitively everywhere else (validation's
// reference check and the runtime `phase_execution` lookup both use
// `eq_ignore_ascii_case`). Drop any existing key that differs only by
// case before inserting, so an upsert REPLACES the phase instead of
// leaving a stale duplicate that could win by map order.
config.phase_definitions.retain(|existing, _| !existing.eq_ignore_ascii_case(phase_id));
config.phase_definitions.insert(phase_id.to_string(), definition);
Ok(())
})
}

/// Upsert (create or replace) a workflow definition by its `id`. The definition
/// replaces any existing entry with the same id; otherwise it is appended.
pub fn upsert_workflow_definition(project_root: &Path, definition: WorkflowDefinition) -> Result<WorkflowConfig> {
Expand Down Expand Up @@ -223,6 +251,46 @@ mod tests {
assert!(msg.contains("invalid"), "error must flag the invalid config, got: {msg}");
}

#[test]
fn phase_authored_on_base_lets_a_referencing_workflow_validate() {
// The whole point of `set_phase_definition`: a phase written to the
// config_source base's `phase_definitions` must resolve when a
// subsequently-set workflow references it — no "references unknown
// phase". We drive the validate gate directly (write_full_workflow_config
// validates BEFORE it attempts the plugin write), so phase-reference
// resolution is exercised without a live writable plugin.
let dir = tempfile::tempdir().expect("tempdir");
let mut config = builtin_workflow_config();
// Minimal valid phase definition (agent mode needs no agent_id).
let phase: PhaseExecutionDefinition = serde_json::from_str(r#"{"mode":"agent"}"#).expect("valid phase json");
config.phase_definitions.insert("authored-phase".to_string(), phase);
let mut wf = sample_workflow("uses-authored");
wf.phases =
vec![animus_config_protocol::workflow_types::WorkflowPhaseEntry::Simple("authored-phase".to_string())];
config.workflows = vec![wf];

// Validation PASSES: the only remaining failure is the write step (no
// writable config_source plugin under test), never an "unknown phase".
let err = write_full_workflow_config(dir.path(), &config).expect_err("no writable source under test");
let msg = format!("{err:#}");
assert!(!msg.contains("unknown phase"), "phase authored on the base must resolve; got: {msg}");
assert!(
msg.contains("no config_source plugin") || msg.contains("does not support writes"),
"should fail only at the write step, got: {msg}"
);

// Control: WITHOUT the phase definition the same workflow reference is
// rejected as unknown — proving it is the authored phase that unblocks it.
let mut missing = builtin_workflow_config();
let mut wf2 = sample_workflow("uses-authored");
wf2.phases =
vec![animus_config_protocol::workflow_types::WorkflowPhaseEntry::Simple("authored-phase".to_string())];
missing.workflows = vec![wf2];
let err2 = write_full_workflow_config(dir.path(), &missing).expect_err("unknown phase must be rejected");
let msg2 = format!("{err2:#}");
assert!(msg2.contains("unknown phase"), "control must fail as unknown phase; got: {msg2}");
}

#[test]
fn write_to_non_writable_or_absent_source_is_actionable() {
// A VALID config that passes the validate gate then hits the write path.
Expand Down
4 changes: 2 additions & 2 deletions crates/orchestrator-config/src/workflow_config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ pub use animus_config_protocol::overlay::{
// ---------------------------------------------------------------------------

pub use config_write::{
remove_agent_profile, remove_workflow_definition, upsert_agent_profile, upsert_workflow_definition,
write_full_workflow_config,
remove_agent_profile, remove_workflow_definition, set_phase_definition, upsert_agent_profile,
upsert_workflow_definition, write_full_workflow_config,
};
pub use environment_routing::resolve_environment;
pub use loading::{
Expand Down
2 changes: 1 addition & 1 deletion crates/orchestrator-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ pub use workflow_config::{
load_workflow_config_with_metadata, merge_yaml_into_config, missing_project_skill_reference_warnings,
missing_skill_reference_warnings_for_sources, missing_skill_yaml_warnings, parse_yaml_workflow_config,
remove_agent_profile, remove_generated_workflow_phase, remove_workflow_definition, resolve_workflow_phase_plan,
resolve_workflow_rework_attempts, resolve_workflow_skip_guards, resolve_workflow_variables,
resolve_workflow_rework_attempts, resolve_workflow_skip_guards, resolve_workflow_variables, set_phase_definition,
resolve_workflow_verdict_routing, unenforced_project_yaml_warnings, unenforced_yaml_field_warnings,
upsert_agent_profile, upsert_generated_workflow_phase, upsert_generated_workflow_pipeline,
upsert_workflow_definition, validate_and_compile_yaml_workflows, validate_workflow_and_runtime_configs,
Expand Down
12 changes: 11 additions & 1 deletion docs/reference/cli/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ animus
│ │ ├── set Replace the full config via the writable config_source plugin (validates first; rejected on read-only sources)
│ │ ├── agent-set Create or replace one agent definition (read-modify-write the full config)
│ │ ├── agent-remove Remove one agent definition (read-modify-write the full config)
│ │ ├── phase-set Create or replace one phase definition on the config_source base (read-modify-write; writes phase_definitions, not the agent-runtime overlay)
│ │ ├── workflow-set Create or replace one workflow definition (read-modify-write)
│ │ └── workflow-remove Remove one workflow definition (read-modify-write)
│ ├── state-machine
Expand Down Expand Up @@ -1925,6 +1926,15 @@ the plugin — nothing is partially written.
model back. This is the **definition**-management verb; it does not collide
with the runtime `animus agent {list,get,run,...}` surface.
- `animus workflow config agent-remove --id <id>` — remove one agent.
- `animus workflow config phase-set --id <phase_id> --input-json <json>` — upsert
one phase definition on the RAW config_source base
(`WorkflowConfig.phase_definitions`). Read-modify-write, same validate-before-write
path as the other entity verbs. This writes the **config_source base**, NOT the
agent-runtime overlay that `animus workflow phases upsert` writes — so a phase
authored here resolves when a subsequently-set workflow references it, instead of
failing `workflow-set` with "references unknown phase". The JSON is a
`PhaseExecutionDefinition` (the value of `phase_definitions.<id>`), e.g.
`--input-json '{"mode":"agent","agent_id":"builder"}'`.
- `animus workflow config workflow-set --input-json <json>` — upsert one
workflow definition (the JSON must include an `id`).
- `animus workflow config workflow-remove --id <id>` — remove one workflow.
Expand All @@ -1941,7 +1951,7 @@ watch (gated on the `config_watch` capability).
| Metric | Count |
|---|---|
| Top-level commands | 28 |
| Nested command entries (all levels) | 214 |
| Nested command entries (all levels) | 215 |

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