Skip to content

Commit cdfa66e

Browse files
committed
Scope strict validation to the run host
1 parent 73ff1ce commit cdfa66e

5 files changed

Lines changed: 140 additions & 28 deletions

File tree

README.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,15 @@ Gate the declaration before starting anything:
117117

118118
```sh
119119
st2 hooks verify
120-
st2 validate --catalog "$CATALOG"
120+
st2 validate --catalog "$CATALOG" --strict
121121
st2 up --catalog "$CATALOG" --host <host> --materialize-only
122122
```
123123

124+
Validation always checks the whole synced catalog structurally. External workspace and task-path
125+
presence is checked only for the selected run host, which defaults to the local short hostname; use
126+
`--host <host>` to validate another machine's local paths. Supervisors may be declared as either a
127+
bare identity or the fully-qualified `<host>.<identity>` bus id.
128+
124129
Materialization simulates all content operations before writing. It refuses any real change to a
125130
Git-tracked target, including `AGENTS.md`; byte-identical tracked content is accepted. Inspect the
126131
declared targets and keep generated overlays untracked. Detection invokes `git` and fails closed if

src/main.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,10 @@ enum Command {
217217
/// st2 catalog.
218218
#[arg(conflicts_with = "catalog_path")]
219219
root: Option<PathBuf>,
220+
/// Host whose external workspace/task paths should be checked. Structural checks always
221+
/// cover the whole catalog. Defaults to the local hostname.
222+
#[arg(long)]
223+
host: Option<String>,
220224
/// Fail (non-zero exit) on warnings too, not just errors.
221225
#[arg(long)]
222226
strict: bool,
@@ -564,9 +568,14 @@ fn main() -> Result<()> {
564568
}
565569
Command::Pretrust { dirs } => pretrust_cmd(&dirs),
566570
Command::Eval { folder, host, keep } => eval_cmd(&folder, host, keep),
567-
Command::Validate { root, strict, json } => {
571+
Command::Validate {
572+
root,
573+
host,
574+
strict,
575+
json,
576+
} => {
568577
let root = catalog_arg(root)?;
569-
validate_cmd(&root, strict, json)
578+
validate_cmd(&root, host, strict, json)
570579
}
571580
Command::Pty { args } => pty_cmd(&args),
572581
Command::Shell { args } => shell_cmd(&args),
@@ -721,9 +730,10 @@ fn eval_cmd(folder: &Path, host: Option<String>, keep: bool) -> Result<()> {
721730
}
722731
}
723732

724-
fn validate_cmd(root: &Path, strict: bool, json: bool) -> Result<()> {
733+
fn validate_cmd(root: &Path, host: Option<String>, strict: bool, json: bool) -> Result<()> {
725734
let catalog_root = root.canonicalize().unwrap_or_else(|_| root.to_path_buf());
726-
let report = st2::validate::validate(&catalog_root);
735+
let host = host.unwrap_or_else(detect_host);
736+
let report = st2::validate::validate_for_host(&catalog_root, &host);
727737
let (errors, warnings) = (report.errors(), report.warnings());
728738

729739
if json {

src/spec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub struct AgentSpec {
3030
pub job_type: JobType,
3131
/// The repo/worktree; **defaults each task's cwd** (spec.md §2).
3232
pub workspace: Option<String>,
33-
/// Identity of this agent's supervisor — crash-dings/escalations route here.
33+
/// Bare identity or `<host>.<identity>` of this agent's supervisor — crash-dings route here.
3434
pub supervisor: Option<String>,
3535
/// `true` decommissions the agent (an edit, never a file delete) → torn down by reconcile.
3636
pub retired: bool,

src/validate.rs

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77
//! relative path, or a missing **catalog-rooted** path — the renderer's own output). Exits non-zero.
88
//! - **WARN** — advisory; the run still works (identity/host path↔content mismatch — the spec says a
99
//! mismatch is a warning; a dangling supervisor — crash-dings just route nowhere; a missing
10-
//! **external** path such as the workspace repo — the validate host may not be the run host; an
11-
//! overlay `@import` that does not resolve — a *render* concern, not st2 law, since a valid spec
12-
//! may carry no persona). `--strict` promotes every WARN to a failure so a renderer's CI can demand
13-
//! spotless.
10+
//! **external** path for an agent assigned to the selected validation host; an overlay `@import`
11+
//! that does not resolve — a *render* concern, not st2 law, since a valid spec may carry no
12+
//! persona). `--strict` promotes every WARN to a failure so a renderer's CI can demand spotless.
1413
//!
1514
//! st2 stays render-agnostic: render-only fields (`harness`, `model`, `role`, `persona`,
1615
//! `permissions`, …) are never required — their absence is never an issue.
@@ -103,6 +102,18 @@ impl Report {
103102

104103
/// Validate a catalog. Returns every issue found, in a stable order (files sorted by discovery).
105104
pub fn validate(root: &Path) -> Report {
105+
validate_scoped(root, None)
106+
}
107+
108+
/// Validate a whole catalog while checking host-local filesystem facts only for `this_host`.
109+
///
110+
/// Structural checks remain fleet-wide. This scope only prevents a synced multi-host catalog from
111+
/// warning that another machine's external workspace or task cwd is absent locally.
112+
pub fn validate_for_host(root: &Path, this_host: &str) -> Report {
113+
validate_scoped(root, Some(this_host))
114+
}
115+
116+
fn validate_scoped(root: &Path, this_host: Option<&str>) -> Report {
106117
// Canonicalize so `$CATALOG`-rooted paths expand to absolute paths (a relative root would make
107118
// every `$CATALOG/...` look relative). Falls back to the given root if it does not exist yet.
108119
let root = &root.canonicalize().unwrap_or_else(|_| root.to_path_buf());
@@ -155,14 +166,29 @@ pub fn validate(root: &Path) -> Report {
155166
let mut seen: HashMap<String, PathBuf> = HashMap::new();
156167
// Placeholder host for bus-id collision: catalogs carry explicit host, and an empty host still
157168
// makes two unset-host same-identity specs collide (which is the real bug).
158-
let this_host = "";
169+
let collision_host = "";
170+
let addresses: HashSet<String> = d
171+
.specs
172+
.iter()
173+
.flat_map(|s| {
174+
let mut values = vec![s.identity.clone()];
175+
if s.host.is_some() {
176+
values.push(s.bus_id(collision_host));
177+
}
178+
values
179+
})
180+
.collect();
159181

160182
for s in &d.specs {
161183
let rp = rel(root, &s.path);
162184
let ag = Some(s.identity.clone());
185+
let runs_on_selected_host = match this_host {
186+
Some(host) => s.resolved_host(host) == host,
187+
None => true,
188+
};
163189

164190
// Duplicate bus id — the runner cannot run two agents under one <host>.<identity>.
165-
let bid = s.bus_id(this_host);
191+
let bid = s.bus_id(collision_host);
166192
if let Some(prev) = seen.insert(bid.clone(), s.path.clone()) {
167193
issues.push(Issue::error(
168194
"dup-id",
@@ -216,14 +242,16 @@ pub fn validate(root: &Path) -> Report {
216242

217243
// Path fields must be absolute or $CATALOG-rooted, and must exist.
218244
for (field, raw) in path_fields(s) {
219-
if let Some(issue) = check_path(root, &rp, &ag, &field, &raw) {
245+
if let Some(issue) = check_path(root, &rp, &ag, &field, &raw, runs_on_selected_host) {
220246
issues.push(issue);
221247
}
222248
}
223249

224-
// A supervisor that names no agent in this catalog — advisory (may live elsewhere).
250+
// Runtime routing accepts either a bare identity or a fully-qualified <host>.<identity>.
251+
// Validation must index the same address set or it rejects declarations the bus can route.
225252
if let Some(sup) = &s.supervisor
226253
&& !identities.contains(sup.as_str())
254+
&& !addresses.contains(sup)
227255
{
228256
issues.push(Issue::warn(
229257
"dangling-supervisor",
@@ -234,7 +262,9 @@ pub fn validate(root: &Path) -> Report {
234262
}
235263

236264
// Overlay lint: render's persona overlay `@import`s must resolve (WARN — render concern).
237-
issues.extend(overlay_lint(&rp, &ag, s));
265+
if runs_on_selected_host {
266+
issues.extend(overlay_lint(&rp, &ag, s));
267+
}
238268

239269
// Declarative render is a pre-boot gate: malformed directives, unsafe destinations, or a
240270
// missing catalog-owned copy source would prevent this agent from booting.
@@ -273,8 +303,16 @@ fn path_fields(s: &AgentSpec) -> Vec<(String, String)> {
273303

274304
/// Check one path field: `$CATALOG` expands to the catalog root; a path bearing any *other* `$VAR` is
275305
/// skipped (an unset var is a literal token — do not guess). What remains must be absolute (R11:
276-
/// final-spec paths are absolute or $CATALOG-rooted, never relative) and must exist.
277-
fn check_path(root: &Path, rp: &str, ag: &Option<String>, field: &str, raw: &str) -> Option<Issue> {
306+
/// final-spec paths are absolute or $CATALOG-rooted, never relative). Catalog-owned paths must
307+
/// always exist; external paths are checked only for an agent assigned to the selected host.
308+
fn check_path(
309+
root: &Path,
310+
rp: &str,
311+
ag: &Option<String>,
312+
field: &str,
313+
raw: &str,
314+
check_external_presence: bool,
315+
) -> Option<Issue> {
278316
let root_s = root.to_string_lossy();
279317
let expanded = raw
280318
.replace("${CATALOG}", &root_s)
@@ -295,24 +333,25 @@ fn check_path(root: &Path, rp: &str, ag: &Option<String>, field: &str, raw: &str
295333
}
296334
if !p.exists() {
297335
// A **catalog-rooted** path is the renderer's own output — its absence is a real render bug
298-
// (ERROR). An **external** absolute path (e.g. the workspace repo) may simply not be present
299-
// on the host running `validate` — a nix build gate legitimately validates a catalog whose
300-
// workspace is cloned on a different run host — so that is advisory (WARN), not a failure.
301-
return Some(if p.starts_with(root) {
302-
Issue::error(
336+
// (ERROR). An **external** absolute path is checked only for the selected run host; its
337+
// absence there is advisory (WARN), not a structural catalog failure.
338+
return if p.starts_with(root) {
339+
Some(Issue::error(
303340
"bad-path",
304341
rp.to_string(),
305342
ag.clone(),
306343
format!("{field} '{raw}' does not exist"),
307-
)
308-
} else {
309-
Issue::warn(
344+
))
345+
} else if check_external_presence {
346+
Some(Issue::warn(
310347
"bad-path",
311348
rp.to_string(),
312349
ag.clone(),
313350
format!("{field} '{raw}' does not exist (absent on this host? — not the run host)"),
314-
)
315-
});
351+
))
352+
} else {
353+
None
354+
};
316355
}
317356
None
318357
}

tests/validate.rs

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//! it hit the spec. Each test builds a minimal catalog exercising one failure mode and asserts the
33
//! exact issue code + severity; a clean catalog (and our shipped `examples/`) must validate spotless.
44
5-
use st2::validate::{Report, Severity, validate};
5+
use st2::validate::{Report, Severity, validate, validate_for_host};
66

77
/// Write a set of `(relative-path, body)` files into a fresh temp catalog.
88
fn catalog(files: &[(&str, &str)]) -> tempfile::TempDir {
@@ -121,6 +121,22 @@ fn a_missing_external_path_is_only_a_warning() {
121121
);
122122
}
123123

124+
#[test]
125+
fn a_remote_hosts_missing_external_path_is_not_a_local_warning() {
126+
let c = catalog(&[(
127+
"hetz/w/agent.kdl",
128+
r#"agent "w" { host "hetz"; type "service"; workspace "/no/such/dir/xyz123"; pty "agent" { command "x" } }"#,
129+
)]);
130+
let r = validate_for_host(c.path(), "Silber");
131+
assert_eq!(r.errors(), 0, "unexpected errors: {:?}", r.issues);
132+
assert_eq!(
133+
r.warnings(),
134+
0,
135+
"remote-host filesystem facts must not dirty local strict validation: {:?}",
136+
r.issues
137+
);
138+
}
139+
124140
#[test]
125141
fn a_path_bearing_another_var_is_skipped() {
126142
// SD3: an unset var is a literal token — do not guess, do not flag.
@@ -238,6 +254,28 @@ fn a_dangling_supervisor_is_a_warning() {
238254
);
239255
}
240256

257+
#[test]
258+
fn a_fully_qualified_supervisor_in_the_catalog_is_clean() {
259+
let c = catalog(&[
260+
(
261+
"Silber/cos/agent.kdl",
262+
r#"agent "cos" { host "Silber"; command "x" }"#,
263+
),
264+
(
265+
"hetz/w/agent.kdl",
266+
r#"agent "w" { host "hetz"; supervisor "Silber.cos"; command "x" }"#,
267+
),
268+
]);
269+
let r = validate(c.path());
270+
assert_eq!(r.errors(), 0, "unexpected errors: {:?}", r.issues);
271+
assert_eq!(
272+
r.warnings(),
273+
0,
274+
"runtime-routable qualified supervisors must validate: {:?}",
275+
r.issues
276+
);
277+
}
278+
241279
#[test]
242280
fn an_identity_folder_mismatch_is_a_warning() {
243281
let c = catalog(&[(
@@ -300,6 +338,26 @@ fn cli_exits_zero_on_a_clean_catalog() {
300338
);
301339
}
302340

341+
#[test]
342+
fn cli_host_scope_keeps_remote_paths_out_of_strict_validation() {
343+
let c = catalog(&[(
344+
"hetz/w/agent.kdl",
345+
r#"agent "w" { host "hetz"; workspace "/no/such/dir/xyz123"; command "x" }"#,
346+
)]);
347+
let out = run_validate(&[
348+
c.path().as_os_str(),
349+
std::ffi::OsStr::new("--host"),
350+
std::ffi::OsStr::new("Silber"),
351+
std::ffi::OsStr::new("--strict"),
352+
]);
353+
assert!(
354+
out.status.success(),
355+
"stdout:\n{}\nstderr:\n{}",
356+
String::from_utf8_lossy(&out.stdout),
357+
String::from_utf8_lossy(&out.stderr)
358+
);
359+
}
360+
303361
#[test]
304362
fn cli_exits_nonzero_on_an_error_and_strict_promotes_warnings() {
305363
// An error catalog exits non-zero without --strict.

0 commit comments

Comments
 (0)