Skip to content
Closed
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
17 changes: 17 additions & 0 deletions crates/agent-spec/src/kdl_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,23 @@ fn agent_node_to_raw(node: &DeclaredNode) -> anyhow::Result<RawSpec> {
"command" => raw.command = arg_string(child),
"argv" => raw.argv = Some(argv(child)?),
"ding" => raw.ding = true,
"deliver" => {
anyhow::ensure!(
raw.deliver.is_none(),
"agent declares `deliver` more than once"
);
anyhow::ensure!(
child.type_name.is_none()
&& child.children.is_empty()
&& child.entries.len() == 1
&& child.entries[0].name.is_none(),
"agent `deliver` must contain exactly one positional string"
);
raw.deliver = Some(Some(
arg_string(child)
.ok_or_else(|| anyhow::anyhow!("agent `deliver` value must be a string"))?,
));
}
"env" => {}
"pty" => {
if let Some(name) = arg_string(child) {
Expand Down
4 changes: 2 additions & 2 deletions crates/agent-spec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ pub use discovery::{
path_defaults,
};
pub use spec::{
AgentDesiredState, AgentSpec, JobType, Resource, Restart, RestartMode, Task, TaskKind,
TaskLifecycle, parse_duration, validate_desired_state_reason,
AgentDesiredState, AgentSpec, DeliveryTransport, JobType, Resource, Restart, RestartMode, Task,
TaskKind, TaskLifecycle, parse_duration, validate_desired_state_reason,
};
58 changes: 55 additions & 3 deletions crates/agent-spec/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
//! stage's script; must NOT allocate a terminal, R09). st2 reads only the runner-normative subset:
//! `identity`, presentation (`name`, `description`), `host`, `role` (metadata only), `type`,
//! `workspace`, whole-agent desired state (plus legacy `retired`), `keep`, `supervisor`,
//! `restart{}`, task lifecycle, Resource bindings (declaration metadata), and the tasks. Everything render-only
//! (`harness`, `model`, `persona`, `permissions`, `transport`, `strategy`, `meta{}`) is baked into
//! the tasks/commands by the render layer and ignored here.
//! `restart{}`, `deliver`, task lifecycle, Resource bindings (declaration metadata), and the tasks.
//! Everything render-only (`harness`, `model`, `persona`, `permissions`, legacy `transport`
//! metadata, `strategy`, `meta{}`) is baked into the tasks/commands by the render layer and ignored
//! here.
//!
//! Three on-disk formats lower to this model: KDL (canonical, parsed by hand in `kdl_format`), and
//! TOML/JSON (serde). Every spec is a `service` — `type = batch` is retired; evals run through the
Expand Down Expand Up @@ -36,6 +37,32 @@ pub enum AgentDesiredState {
Retired { reason: Option<String> },
}

/// One provider-native message delivery transport declared by an agent.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum DeliveryTransport {
Mcp,
AppServer,
}

impl DeliveryTransport {
pub fn as_str(self) -> &'static str {
match self {
Self::Mcp => "mcp",
Self::AppServer => "app-server",
}
}

fn parse(value: &str) -> anyhow::Result<Self> {
match value {
"mcp" => Ok(Self::Mcp),
"app-server" => Ok(Self::AppServer),
_ => anyhow::bail!(
"unsupported `deliver` value '{value}' (expected `mcp` or `app-server`)"
),
}
}
}

impl AgentDesiredState {
pub fn as_str(&self) -> &'static str {
match self {
Expand Down Expand Up @@ -91,6 +118,8 @@ pub struct AgentSpec {
pub keep: bool,
/// Crash/restart policy (§4). `None` → the runner's default policy.
pub restart: Option<Restart>,
/// Provider-native delivery selected by `deliver`; `None` means legacy `ding` or no delivery.
pub delivery: Option<DeliveryTransport>,
/// Named typed references used by the agent. st2 preserves these for readers but does not
/// resolve them or assign launch, readiness, access, or lifecycle semantics.
pub resources: Vec<Resource>,
Expand Down Expand Up @@ -327,6 +356,15 @@ impl AgentSpec {
.any(|task| !task.derived && (task.command.is_some() || task.argv.is_some()))
}

/// True when the declaration selected legacy screen delivery or one native transport.
pub fn has_delivery_transport(&self) -> bool {
self.delivery.is_some()
|| self
.tasks
.iter()
.any(|task| task.derived && task.kind == TaskKind::Exec && task.name == "ding")
}

/// The restart policy in effect (declared, else the runner default).
pub fn restart_policy(&self) -> Restart {
self.restart.clone().unwrap_or_default()
Expand Down Expand Up @@ -399,6 +437,9 @@ pub(crate) struct RawSpec {
/// Compact catalog form: include the built-in `st2 ding` sidecar.
#[serde(default)]
pub ding: bool,
/// Compact catalog form: select one provider-native delivery transport.
#[serde(default, deserialize_with = "deserialize_explicit_optional")]
pub deliver: Option<Option<String>>,
/// Compact catalog form: reconciliation policy for the generated agent PTY.
pub lifecycle: Option<String>,
/// `pty "<name>" {}` / `[pty.<name>]` — interactive tasks.
Expand Down Expand Up @@ -750,6 +791,7 @@ impl RawSpec {
|| self.command.is_some()
|| self.argv.is_some()
|| self.ding
|| self.deliver.is_some()
|| !self.resource.0.is_empty()
|| !self.pty.is_empty()
|| !self.exec.is_empty()
Expand Down Expand Up @@ -778,6 +820,15 @@ impl RawSpec {
desired_state_value.as_deref(),
desired_state_reason,
)?;
let deliver = reject_explicit_null("deliver", self.deliver)?;
let delivery = deliver
.as_deref()
.map(DeliveryTransport::parse)
.transpose()?;
anyhow::ensure!(
!(self.ding && delivery.is_some()),
"agent '{identity}' declares both `ding` and `deliver`; choose one transport"
);
validate_launch(
&identity,
self.command.as_ref(),
Expand Down Expand Up @@ -850,6 +901,7 @@ impl RawSpec {
desired_state,
keep: self.keep,
restart: self.restart.map(RawRestart::lower),
delivery,
resources,
tasks,
path,
Expand Down
87 changes: 85 additions & 2 deletions crates/agent-spec/tests/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::fs;
use std::path::Path;
use std::time::Duration;

use agent_spec::spec::{TaskKind, TaskLifecycle};
use agent_spec::spec::{DeliveryTransport, TaskKind, TaskLifecycle};
use agent_spec::{
AgentDesiredState, AgentSpec, JobType, Resource, Task, discover, discover_strict,
};
Expand Down Expand Up @@ -139,10 +139,11 @@ fn lifecycle_fields_make_path_placed_files_agent_candidates() {
}

#[test]
fn explicit_json_null_lifecycle_fields_are_rejected_instead_of_granting_running_intent() {
fn explicit_json_null_fields_are_rejected_instead_of_granting_default_behavior() {
for (name, lifecycle) in [
("null-retired", r#""retired":null"#),
("null-state", r#""desired_state":null"#),
("null-deliver", r#""deliver":null"#),
(
"null-reason",
r#""desired_state":"suspended","desired_state_reason":null"#,
Expand Down Expand Up @@ -328,6 +329,88 @@ agent "cos" {
ding.env.get("ST_AGENT").map(String::as_str),
Some("Silber.cos")
);
assert!(spec.delivery.is_none());
assert!(spec.has_delivery_transport());
}

#[test]
fn deliver_is_typed_without_lowering_to_the_legacy_ding_task() {
let tmp = tempfile::tempdir().unwrap();
write(
tmp.path(),
"agents/h/claude/agent.kdl",
r#"agent "claude" { host "h"; command "claude"; deliver "mcp" }"#,
);
write(
tmp.path(),
"agents/h/codex/agent.kdl",
r#"agent "codex" { host "h"; command "codex"; deliver "app-server" }"#,
);

let found = discover(tmp.path());
assert!(found.errors.is_empty(), "{:?}", found.errors);
let claude = find(&found.specs, "claude");
let codex = find(&found.specs, "codex");
assert_eq!(claude.delivery, Some(DeliveryTransport::Mcp));
assert_eq!(codex.delivery, Some(DeliveryTransport::AppServer));
assert_eq!(claude.delivery.unwrap().as_str(), "mcp");
assert_eq!(codex.delivery.unwrap().as_str(), "app-server");
for spec in [claude, codex] {
assert!(spec.has_delivery_transport());
assert_eq!(spec.tasks.len(), 1);
assert!(spec.tasks.iter().all(|task| !task.derived));
}
}

#[test]
fn deliver_rejects_unknown_duplicate_mixed_and_malformed_declarations() {
for (name, declaration, expected) in [
(
"unknown",
r#"agent "worker" { command "true"; deliver "socket" }"#,
"unsupported `deliver` value 'socket'",
),
(
"duplicate",
r#"agent "worker" { command "true"; deliver "mcp"; deliver "app-server" }"#,
"declares `deliver` more than once",
),
(
"mixed",
r#"agent "worker" { command "true"; ding; deliver "mcp" }"#,
"declares both `ding` and `deliver`",
),
(
"missing",
r#"agent "worker" { command "true"; deliver }"#,
"must contain exactly one positional string",
),
(
"non-string",
r#"agent "worker" { command "true"; deliver #true }"#,
"value must be a string",
),
(
"property",
r#"agent "worker" { command "true"; deliver "mcp" mode="extra" }"#,
"must contain exactly one positional string",
),
] {
let tmp = tempfile::tempdir().unwrap();
write(
tmp.path(),
&format!("agents/h/{name}/agent.kdl"),
declaration,
);
let found = discover(tmp.path());
assert!(found.specs.is_empty(), "{name}: {:?}", found.specs);
assert_eq!(found.errors.len(), 1, "{name}: {:?}", found.errors);
assert!(
found.errors[0].message.contains(expected),
"{name}: expected {expected:?}, got {:?}",
found.errors[0]
);
}
}

#[test]
Expand Down
6 changes: 6 additions & 0 deletions src/catalog_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,12 @@ fn normalize_agent(spec: &agent_spec::AgentSpec) -> Result<BTreeMap<String, Sema
spec.desired_state.reason(),
);
insert_default_bool(&mut fields, &format!("{base}/keep"), spec.keep, false);
insert_optional(
&mut fields,
&format!("{base}/delivery"),
SemanticType::String,
spec.delivery.map(|delivery| delivery.as_str()),
);

let restart = spec.restart_policy();
let default_restart = Restart::default();
Expand Down
1 change: 1 addition & 0 deletions src/eval_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ pub fn spec_to_agent_specs(agents: &[SpecAgent], host: &str, root: &Path) -> Vec
desired_state: AgentDesiredState::Running,
keep: false,
restart: None,
delivery: None,
resources: Vec::new(),
tasks,
path: path.clone(),
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ pub use agent_spec::{discovery, spec};

pub use agent_spec::discovery::{Discovered, SpecError, discover, discover_strict};
pub use agent_spec::spec::{
AgentDesiredState, AgentSpec, JobType, Resource, Restart, RestartMode, Task, TaskKind,
TaskLifecycle, parse_duration,
AgentDesiredState, AgentSpec, DeliveryTransport, JobType, Resource, Restart, RestartMode, Task,
TaskKind, TaskLifecycle, parse_duration,
};
pub use catalog_lock::CatalogLock;
pub use exec_backend::ExecBackend;
Expand Down
10 changes: 10 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1433,6 +1433,12 @@ fn doctor_cmd(root: &Path, host: Option<String>, require_supervisor: bool) -> Re
);
continue;
}
if !spec.has_delivery_transport() {
report_advisory(
&format!("{bus_id} delivery transport missing"),
"declare `ding` or `deliver`; agent receives no DING",
);
}
for task in &spec.tasks {
let id = task
.id
Expand Down Expand Up @@ -1546,6 +1552,10 @@ fn report_check(problems: &mut usize, ok: bool, label: &str, detail: &str) {
}
}

fn report_advisory(label: &str, detail: &str) {
println!(" ⚠ {label} — {detail}");
}

fn presentation_cmd(
field: st2::agent_author::PresentationField,
args: PresentationArgs,
Expand Down
3 changes: 3 additions & 0 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2164,6 +2164,7 @@ mod tests {
desired_state: crate::AgentDesiredState::Running,
keep: false,
restart: None,
delivery: None,
resources: vec![],
tasks: vec![Task {
kind: TaskKind::Pty,
Expand Down Expand Up @@ -2213,6 +2214,7 @@ mod tests {
desired_state: crate::AgentDesiredState::Running,
keep: false,
restart: None,
delivery: None,
resources: vec![],
tasks: vec![Task {
kind: TaskKind::Pty,
Expand Down Expand Up @@ -2469,6 +2471,7 @@ mod tests {
desired_state: crate::AgentDesiredState::Running,
keep: false,
restart: None,
delivery: None,
resources: vec![],
tasks: vec![],
path: std::path::PathBuf::from("/x"),
Expand Down
25 changes: 25 additions & 0 deletions tests/catalog_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,31 @@ fn desired_state_and_reason_have_distinct_secret_safe_semantic_addresses() {
assert!(!rendered.contains(reason));
}

#[test]
fn declared_delivery_has_one_exact_semantic_address() {
let (_temp, catalog, prepared, root) = fixture();
fs::write(
prepared.join("agents/host/worker/agent.kdl"),
r#"agent "worker" {
host "host"
deliver "app-server"
argv "tool" "arg"
}
"#,
)
.unwrap();

let receipt = parsed(&diff(&catalog, &prepared, &root));
let fields = agent_fields(&receipt);
assert_eq!(
fields
.iter()
.filter(|field| field.as_str() == "/agents/host/worker/delivery")
.count(),
1
);
}

#[test]
fn effective_task_id_and_cwd_defaults_normalize_to_explicit_values() {
let (_temp, catalog, prepared, _root) = fixture();
Expand Down
Loading
Loading