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
104 changes: 83 additions & 21 deletions crates/agent-spec/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
//! The folder is *both* the catalog and the inboxes: one directory per agent holding its spec plus
//! its `inbox/`/`archive/`. Discovery walks `<root>` recursively, parses every `*.{toml,json,kdl}`
//! that looks like a spec, and resolves each spec's `identity`/`host` with the gist's precedence:
//! **content wins, the path supplies defaults, a mismatch is a warning** (a convention, not a
//! constraint). Malformed files are collected as errors rather than halting the walk — one bad edit
//! must not wedge the whole reconcile.
//! an explicit identity+host pair is path-independent; otherwise **content wins, the path supplies
//! defaults, and a mismatch is a warning**. Malformed files are collected as errors rather than
//! halting the walk — one bad edit must not wedge the whole reconcile.

use std::fs;
use std::path::{Component, Path, PathBuf};
Expand Down Expand Up @@ -54,34 +54,91 @@ pub fn discover(root: &Path) -> Discovered {
out
}

/// Recursively gather candidate spec files, skipping dotfiles/dotdirs (`.git`, hidden config), the
/// top-level `pty/` runtime registry, and anything that isn't one of [`SPEC_EXTS`]. `pty` session
/// metadata includes JSON that can resemble an agent spec; it is runner state, never catalog input.
/// Unreadable directories are skipped, not fatal.
/// Whether `path` is in catalog declaration space rather than a known control/runtime namespace.
///
/// A leading dot has no generic meaning: organizational directories such as `.managed` and
/// `.retired` remain visible. `.git` and `.st2` control directories at any depth, the catalog root's
/// `pty` child, and an actual declaration parent's `resources`, `archive`, and `inbox` children have
/// explicit non-catalog meaning.
pub fn is_catalog_path(root: &Path, path: &Path) -> bool {
let Ok(rel) = path.strip_prefix(root) else {
return false;
};
let components: Vec<_> = rel
.components()
.filter_map(|component| match component {
Component::Normal(name) => Some(name),
_ => None,
})
.collect();

if components
.iter()
.any(|name| matches!(name.to_str(), Some(".git" | ".st2")))
|| components.first().and_then(|name| name.to_str()) == Some("pty")
{
return false;
}

let mut parent = root.to_path_buf();
for name in components {
if matches!(name.to_str(), Some("resources" | "archive" | "inbox"))
&& is_declaration_parent(&parent)
{
return false;
}
parent.push(name);
}
true
}

/// Whether `dir` anchors at least one declaration whose adjacent state directories are not
/// recursively discoverable catalog input.
///
/// Generic `agent.*` filenames reserve the namespace even while malformed so a broken declaration
/// cannot suddenly expose its inbox as candidate specs. Named declaration files are recognized
/// only when they parse as an agent spec, which keeps ordinary project JSON/TOML/KDL from claiming
/// an unrelated `resources` directory.
fn is_declaration_parent(dir: &Path) -> bool {
let Ok(entries) = fs::read_dir(dir) else {
return false;
};
entries.flatten().any(|entry| {
let path = entry.path();
if !entry.file_type().is_ok_and(|kind| kind.is_file())
|| !path
.extension()
.and_then(|extension| extension.to_str())
.is_some_and(|extension| SPEC_EXTS.contains(&extension))
{
return false;
}
if path.file_stem().and_then(|stem| stem.to_str()) == Some("agent") {
return true;
}
parse_raw_file(&path).is_ok_and(|raws| raws.iter().any(RawSpec::looks_like_spec))
})
}

/// Recursively gather candidate spec files, skipping only explicit control/runtime namespaces and
/// anything that isn't one of [`SPEC_EXTS`]. `pty` session metadata includes JSON that can resemble
/// an agent spec; it is runner state, never catalog input. Unreadable directories are skipped, not
/// fatal.
fn collect_spec_files(root: &Path, dir: &Path, acc: &mut Vec<PathBuf>) {
let entries = match fs::read_dir(dir) {
Ok(e) => e,
Err(_) => return,
};
for entry in entries.flatten() {
let path = entry.path();
let name = entry.file_name();
let name = name.to_string_lossy();
if name.starts_with('.') {
continue; // skip hidden files and directories
if !is_catalog_path(root, &path) {
continue;
}
let ft = match entry.file_type() {
Ok(ft) => ft,
Err(_) => continue,
};
if ft.is_dir() {
if path == root.join("pty")
|| name == "resources"
|| name == "archive"
|| name == "inbox"
{
continue;
}
collect_spec_files(root, &path, acc);
} else if ft.is_file()
&& let Some(ext) = path.extension().and_then(|e| e.to_str())
Expand All @@ -103,6 +160,8 @@ fn collect_spec_files(root: &Path, dir: &Path, acc: &mut Vec<PathBuf>) {
pub struct Declared {
/// `identity` as written in the file. `None` when the file relies on [`path_defaults`].
pub identity: Option<String>,
/// `host` as written in the file. `None` when the file relies on [`path_defaults`].
pub host: Option<String>,
/// `type` as written, before it is normalized to `JobType::Service`. `None` when unset.
pub job_type: Option<String>,
}
Expand All @@ -117,6 +176,7 @@ pub fn parse_declared(path: &Path) -> anyhow::Result<Vec<Declared>> {
.into_iter()
.map(|raw| Declared {
identity: raw.identity,
host: raw.host,
job_type: raw.job_type,
})
.collect())
Expand Down Expand Up @@ -156,7 +216,8 @@ fn load_specs(root: &Path, path: &Path) -> anyhow::Result<(Vec<AgentSpec>, Vec<S
Ok((specs, warnings))
}

/// Apply the gist's identity/host precedence to one raw spec: content wins, path supplies defaults, a
/// Apply identity/host precedence to one raw spec. An explicit pair is authoritative and
/// path-independent. When either is omitted, content still wins over path-derived defaults and a
/// mismatch warns. Returns `None` for a non-spec (no agent signal); `Err` when it looks like a spec
/// but no identity can be resolved from content or path.
fn resolve_spec(
Expand All @@ -169,10 +230,11 @@ fn resolve_spec(
}

let (path_identity, path_host) = path_defaults(root, path);
let explicit_placement = raw.identity.is_some() && raw.host.is_some();
let mut warnings = Vec::new();

let identity = match (raw.identity.clone(), path_identity) {
(Some(c), Some(p)) if c != p => {
(Some(c), Some(p)) if c != p && !explicit_placement => {
warnings.push(format!(
"{}: identity mismatch — content '{c}' vs path '{p}'; using content",
path.display()
Expand All @@ -190,7 +252,7 @@ fn resolve_spec(
};

let host = match (raw.host.clone(), path_host) {
(Some(c), Some(p)) if c != p => {
(Some(c), Some(p)) if c != p && !explicit_placement => {
warnings.push(format!(
"{}: host mismatch — content '{c}' vs path '{p}'; using content",
path.display()
Expand Down
4 changes: 3 additions & 1 deletion crates/agent-spec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ pub mod discovery;
mod kdl_format;
pub mod spec;

pub use discovery::{Declared, Discovered, SpecError, discover, parse_declared, path_defaults};
pub use discovery::{
Declared, Discovered, SpecError, discover, is_catalog_path, parse_declared, path_defaults,
};
pub use spec::{
AgentSpec, JobType, Resource, Restart, RestartMode, Task, TaskKind, parse_duration,
};
135 changes: 107 additions & 28 deletions crates/agent-spec/tests/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,10 +496,7 @@ fn empty_or_ambiguous_argv_is_rejected_in_every_task_shape() {
argv "true""#,
),
("empty-explicit", r#"pty "agent" { argv }"#),
(
"empty-program-explicit",
r#"pty "agent" { argv "" }"#,
),
("empty-program-explicit", r#"pty "agent" { argv "" }"#),
(
"both-explicit",
r#"pty "agent" { command "true"; argv "true" }"#,
Expand Down Expand Up @@ -657,11 +654,10 @@ fn path_supplies_identity_and_host_when_content_omits_them() {
}

#[test]
fn content_wins_over_path_and_mismatch_warns() {
fn an_explicit_identity_with_an_implicit_host_keeps_the_path_warning() {
let tmp = tempfile::tempdir().unwrap();
let spec = r#"
identity = "real-name"
host = "real-host"
[pty.agent]
command = "exec claude 'boot'"
"#;
Expand All @@ -671,17 +667,63 @@ command = "exec claude 'boot'"
assert!(found.errors.is_empty());
let s = &found.specs[0];
assert_eq!(s.identity, "real-name");
assert_eq!(s.host.as_deref(), Some("real-host"));
assert_eq!(found.warnings.len(), 2);
assert_eq!(s.host.as_deref(), Some("wrong-host"));
assert_eq!(found.warnings.len(), 1);
assert!(
found
.warnings
.iter()
.any(|w| w.contains("identity mismatch"))
);
}

#[test]
fn an_explicit_host_with_an_implicit_identity_keeps_the_path_warning() {
let tmp = tempfile::tempdir().unwrap();
let spec = r#"
host = "real-host"
[pty.agent]
command = "exec claude 'boot'"
"#;
write(tmp.path(), "agents/wrong-host/path-name/agent.toml", spec);

let found = discover(tmp.path());
assert!(found.errors.is_empty());
let s = &found.specs[0];
assert_eq!(s.identity, "path-name");
assert_eq!(s.host.as_deref(), Some("real-host"));
assert_eq!(found.warnings.len(), 1);
assert!(found.warnings.iter().any(|w| w.contains("host mismatch")));
}

#[test]
fn an_explicit_identity_and_host_are_path_independent() {
let tmp = tempfile::tempdir().unwrap();
let rel = "teams/.managed/groups/archive/project/declaration/agent.kdl";
write(
tmp.path(),
rel,
r#"agent "stable-agent" { host "stable-host"; command "exec codex" }"#,
);

let found = discover(tmp.path());
assert!(found.errors.is_empty(), "errors: {:?}", found.errors);
assert!(found.warnings.is_empty(), "warnings: {:?}", found.warnings);
assert_eq!(found.specs.len(), 1);
let spec = &found.specs[0];
assert_eq!(spec.identity, "stable-agent");
assert_eq!(spec.host.as_deref(), Some("stable-host"));
assert_eq!(
spec.path.parent(),
Some(
tmp.path()
.join("teams/.managed/groups/archive/project/declaration")
.as_path()
),
"the declaration parent remains the state/resource anchor"
);
}

#[test]
fn malformed_file_is_collected_as_error_and_does_not_halt_the_walk() {
let tmp = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -822,29 +864,66 @@ fn nonexistent_root_yields_empty_not_error() {
}

#[test]
fn hidden_runner_state_and_resources_are_ignored() {
fn only_contextually_reserved_namespaces_are_ignored() {
let tmp = tempfile::tempdir().unwrap();
write(
tmp.path(),
"agents/hetz/a/agent.toml",
"identity=\"a\"\n[pty.agent]\ncommand=\"x\"\n",
);
// dot-prefixed runner state (R03) + a resource message — neither is a spec.
write(tmp.path(), ".st2.hetz.lock", "12345");
write(
tmp.path(),
"agents/hetz/a/resources/inbox/1784-abc.md",
"a message",
);
// The canonical PTY_ROOT is `<catalog>/pty`. Its session JSON contains command/cwd fields and
// must never be mistaken for a catalog agent declaration.
write(
tmp.path(),
"pty/hetz.a.json",
r#"{"name":"hetz.a","status":"running","command":"sh -c x","cwd":"/tmp"}"#,
);
"agents/hetz/live/agent.kdl",
r#"agent "live" { host "hetz"; command "x" }"#,
);
for (path, identity) in [
(".managed/team/agent.kdl", "managed"),
(".retired/team/agent.kdl", "dot-retired"),
("agents/archive/project/agent.kdl", "archive-project"),
("agents/resources/project/agent.kdl", "resources-project"),
("agents/inbox/project/agent.kdl", "inbox-project"),
] {
write(
tmp.path(),
path,
&format!(r#"agent "{identity}" {{ host "h"; command "x" }}"#),
);
}

for path in [
".git/project/agent.kdl",
".st2/project/agent.kdl",
"organizations/project/.git/nested/agent.kdl",
"organizations/project/.st2/nested/agent.kdl",
"pty/project/agent.kdl",
"agents/hetz/live/resources/project/agent.kdl",
"agents/hetz/live/archive/project/agent.kdl",
"agents/hetz/live/inbox/project/agent.kdl",
] {
write(
tmp.path(),
path,
r#"agent "excluded" { host "h"; command "x" }"#,
);
}

let found = discover(tmp.path());
assert_eq!(found.specs.len(), 1);
assert_eq!(found.specs[0].identity, "a");
assert!(found.errors.is_empty(), "errors: {:?}", found.errors);
assert_eq!(found.specs.len(), 6, "specs: {:?}", found.specs);
for identity in [
"live",
"managed",
"dot-retired",
"archive-project",
"resources-project",
"inbox-project",
] {
assert!(
found.specs.iter().any(|spec| spec.identity == identity),
"{identity} must remain discoverable"
);
}
assert!(
!find(&found.specs, "dot-retired").retired,
"a .retired folder has no lifecycle meaning"
);
assert!(
found.specs.iter().all(|spec| spec.identity != "excluded"),
"reserved control/state namespaces must not become declarations"
);
}
12 changes: 10 additions & 2 deletions docs/vrs/requirements.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,16 @@ accepted.

- **R01 Agent-spec compliance:** st2 validates and implements every agent-spec
capability it claims to support, and identifies unsupported capabilities.
- **R02 Canonical KDL:** Hand-authored KDL is the canonical declaration; any
generator is optional and its output is inspectable before reconciliation.
- **R02 Canonical KDL and declaration identity:** Hand-authored KDL is the
canonical declaration; any generator is optional and its output is
inspectable before reconciliation. Declarations are discovered recursively.
An explicit `identity` and `host` pair is authoritative independent of its
folder path, while either omitted field retains path-derived defaults and
mismatch diagnostics. The declaration's parent remains its state/resource
anchor. Dot-prefixed and other organizational folders have no implicit
lifecycle meaning; discovery excludes `.git` and `.st2` control directories
at any depth, the catalog-root `pty` runtime directory, and state namespaces
directly owned by a declaration.
- **R03 Host-pinned placement:** Every runnable agent or task resolves to its
declared host; host-local roots own reconciliation.

Expand Down
Loading
Loading