From 6084b4ca78998475b93697df90a61afebe1354fd Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Thu, 30 Jul 2026 20:42:42 +0200 Subject: [PATCH 1/4] feat(eval): boot canonical agent specs agent-session-id: dev3.dotfiles-cos-misc-agent-runtime-simplification agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /home/schickling/.config/coding-agents/profile.json agent-skills-manifest: /nix/store/nk9iml2841l1yjjg0f6f0d3y60zkg1nn-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@4a0515f --- README.md | 21 +++ docs/vrs/spec.md | 28 +++ src/eval_run.rs | 418 +++++++++++++++++++++++++++++++++++++----- src/eval_spec.rs | 55 +++++- tests/eval_run_e2e.rs | 318 ++++++++++++++++++++++++++++++++ 5 files changed, 787 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index d51d07ad..12f9d66c 100644 --- a/README.md +++ b/README.md @@ -392,5 +392,26 @@ The sole canonical agent contract is repository. Eval corpus definitions, execution/readiness evidence, authorization, and results belong to that repository; st2 does not duplicate or pin its ledgers here. +An eval that exercises production Agent Specs opts in explicitly: + +```kdl +eval { + copy "./fixture" + run "publish-canonical-team" { command "..." } + canonical-agents + message { from "requester"; to "evalhost.supervisor"; content "./task.md" } + max-timeout "300s" + judges { /* held-out checks */ } +} +``` + +`copy` and deterministic `run` steps populate the hermetic temporary catalog first. +`canonical-agents` then discovers and materializes declarations at +`agents///agent.kdl`. It is mutually exclusive with compact `team` / `agent` seats: +the discovered vector is the sole authority for launch, kickoff routing, supervision, logs, and +teardown. Missing, malformed, duplicate, retired, nonlocal, noncanonical, root-overriding, or +unrunnable declarations fail before a seat starts. Without the directive, Agent Spec-shaped files +inside a fixture remain inert and compact evals retain their flat bus semantics. + `st2 compile-agent` remains experimental. Hand-authored KDL is the canonical st2 authoring interface, and generated output must be reviewed before materialization. diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index 1c5f9016..6f0a1d7b 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -16,6 +16,34 @@ delivers messages. The agent grammar and harness-facing contract remain canonical in [`compoundingtech/evals/AGENT-SPEC.md`](https://github.com/compoundingtech/evals/blob/main/AGENT-SPEC.md). +## Canonical Agent Spec eval teams + +An eval may opt into `canonical-agents` after its fixture copy and deterministic +run steps have populated the hermetic temporary catalog. The directive is +mutually exclusive with compact `team` / `agent` seats. st2 discovers and +materializes only declarations at +`agents///agent.kdl`, then carries that one Agent Spec vector +unchanged through launch admission, kickoff resolution, supervision, logging, +and teardown. + +Admission fails before spawn when discovery is empty, malformed, +warning-bearing, duplicate, retired, nonlocal, noncanonical, root-overriding, +unrunnable, or does not expose exactly one main PTY per declaration. The +kickoff target must resolve to exactly one member of the discovered fleet. The +eval owns one native `CATALOG` / `ST_ROOT` and its `/pty` registry; +declarations cannot override those roots. Workspace renders are materialized +before any seat starts. + +Without `canonical-agents`, fixture declarations are not discovered or launched +and compact evals retain their catalog-less flat bus. This explicit opt-in keeps +ordinary fixtures inert while allowing the same canonical declaration to be +exercised in an eval and real work. Parser and admission evidence lives in +`eval_spec::tests::canonical_agents_is_bare_once_and_excludes_compact_seats` and +the `canonical_eval_team_*` unit tests. End-to-end isolation, canonical inbox +routing, the no-opt-in collision control, and pre-spawn path refusal are proven +by the four `canonical_agents_*` / `fixture_agent_specs_*` cases in +`tests/eval_run_e2e.rs`. + ## Resource bindings (R20-R21) An agent may directly declare zero or more generic Resource bindings: diff --git a/src/eval_run.rs b/src/eval_run.rs index 87f5ebad..f7a7666b 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -1,8 +1,8 @@ //! Runtime for the st2 spec (P2+): boot a spec's team, and (P3/P4) run its `eval` + judges. The -//! team-boot REUSES the existing reconcile/execute machinery — a parsed [`Spec`] is mapped to in-memory -//! [`AgentSpec`]s (each agent → a pty task for its command + an exec task per `exec` block), then -//! `reconcile` + `execute` spawn them (isolated, teardown-clean) exactly as `st2 up ` does. -//! No catalog discovery: the one spec file IS the source. +//! team-boot REUSES the existing reconcile/execute machinery. Compact `team` declarations map to +//! in-memory [`AgentSpec`]s; an explicit `canonical-agents` eval instead discovers the post-run +//! hermetic catalog. Either way one [`AgentSpec`] vector flows through reconcile, execution, +//! supervision, and teardown exactly as `st2 up ` does. use std::collections::{BTreeMap, HashSet}; use std::path::{Path, PathBuf}; @@ -106,6 +106,130 @@ pub fn spec_to_agent_specs(agents: &[SpecAgent], host: &str, root: &Path) -> Vec .collect() } +#[derive(Debug)] +struct CanonicalEvalTeam { + specs: Vec, + seat_ids: Vec, +} + +fn main_pty_id(spec: &AgentSpec, host: &str) -> Result { + let bus_id = spec.bus_id(host); + let main = spec + .tasks + .iter() + .filter(|task| task.kind == TaskKind::Pty && task.name == "agent") + .collect::>(); + let [main] = main.as_slice() else { + anyhow::bail!( + "Agent Spec `{bus_id}` must declare exactly one main PTY named `agent`, found {}", + main.len() + ); + }; + Ok(main + .id + .clone() + .unwrap_or_else(|| format!("{bus_id}.agent"))) +} + +/// Discover the sole declaration authority for a `canonical-agents` eval after its fixture and run +/// steps have populated the hermetic catalog. This deliberately consumes the shared Agent Spec +/// parser instead of projecting the compact eval grammar into a second, partial declaration. +fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result { + let found = crate::discover(catalog); + if !found.errors.is_empty() { + let errors = found + .errors + .iter() + .map(|error| format!("{}: {}", error.path.display(), error.message)) + .collect::>() + .join("; "); + anyhow::bail!("canonical eval Agent Spec discovery failed: {errors}"); + } + if !found.warnings.is_empty() { + anyhow::bail!( + "canonical eval Agent Specs must discover without warnings: {}", + found.warnings.join("; ") + ); + } + if found.specs.is_empty() { + anyhow::bail!("canonical-agents found no canonical Agent Specs in {}", catalog.display()); + } + if crate::catalog::pty_root(catalog) != catalog.join("pty") { + anyhow::bail!( + "canonical-agents requires the hermetic PTY root `{}`", + catalog.join("pty").display() + ); + } + + let mut paths = BTreeMap::::new(); + let mut bus_ids = HashSet::new(); + let mut seat_ids = Vec::new(); + for spec in &found.specs { + *paths.entry(spec.path.clone()).or_default() += 1; + let bus_id = spec.bus_id(host); + if !bus_ids.insert(bus_id.clone()) { + anyhow::bail!("canonical-agents found duplicate Agent Spec bus identity `{bus_id}`"); + } + if spec.retired { + anyhow::bail!("canonical-agents refuses retired Agent Spec `{bus_id}`"); + } + if spec.resolved_host(host) != host { + anyhow::bail!( + "canonical-agents Agent Spec `{bus_id}` belongs to host `{}`, not eval host `{host}`", + spec.resolved_host(host) + ); + } + let expected_path = catalog + .join("agents") + .join(spec.resolved_host(host)) + .join(&spec.identity) + .join("agent.kdl"); + if spec.path != expected_path { + anyhow::bail!( + "canonical-agents declaration {} must use canonical path {}", + spec.path.display(), + expected_path.display() + ); + } + if !spec.is_runnable() { + anyhow::bail!("canonical-agents Agent Spec `{bus_id}` is not runnable"); + } + for task in &spec.tasks { + for root in ["CATALOG", "ST_ROOT", "PTY_ROOT"] { + if task.env.contains_key(root) { + anyhow::bail!( + "canonical-agents Agent Spec `{bus_id}` must not override eval-owned `{root}`" + ); + } + } + } + seat_ids.push(main_pty_id(spec, host)?); + } + if let Some((path, count)) = paths.into_iter().find(|(_, count)| *count != 1) { + anyhow::bail!( + "canonical-agents declaration {} must contain exactly one Agent Spec, found {count}", + path.display() + ); + } + seat_ids.sort(); + + let materialized = + crate::materialize::materialize_catalog(catalog, &found.specs, host); + if !materialized.errors.is_empty() { + anyhow::bail!( + "canonical eval Agent Spec materialization failed: {}", + materialized.errors.join("; ") + ); + } + for warning in materialized.warnings { + eval_log!("WARN canonical eval materialization: {warning}"); + } + Ok(CanonicalEvalTeam { + specs: found.specs, + seat_ids, + }) +} + fn shell_single_quote(value: &str) -> String { format!("'{}'", value.replace('\'', "'\\''")) } @@ -330,15 +454,31 @@ fn from_is(from: Option<&str>, id: &str) -> bool { /// early "on it" ack from the sup can't fire it). Bounded by `timeout`. Returns whether done fired. fn wait_done( bus: &Path, + host: &str, + canonical_bus: bool, sup: &str, requester: &str, workers: &[String], timeout: Duration, on_tick: &mut dyn FnMut(), ) -> bool { - let sup_inbox = bus.join(sup).join("inbox"); - let sup_archive = bus.join(sup).join("archive"); - let req_inbox = bus.join(requester).join("inbox"); + let inbox = |id: &str| { + if canonical_bus { + crate::message::resolve_inbox(bus, id, host) + } else { + bus.join(id).join("inbox") + } + }; + let archive = |id: &str| { + if canonical_bus { + crate::message::resolve_archive(bus, id, host) + } else { + bus.join(id).join("archive") + } + }; + let sup_inbox = inbox(sup); + let sup_archive = archive(sup); + let req_inbox = inbox(requester); let deadline = Instant::now() + timeout; loop { if EVAL_INTERRUPTED.load(Ordering::SeqCst) { @@ -382,9 +522,9 @@ fn wait_done( /// startup) must fail the eval LOUDLY now, not leave it hanging until `max-timeout` waiting for a /// confirmation that can never come. Poll briefly for all seats to be live — a real seat is up within /// ~1s; a dead-at-boot one never is (tolerant of a slow start + a transient pty-list flicker). -fn boot_gate(agents: &[SpecAgent], specs: &[AgentSpec], host: &str, catalog: &Path) -> Result<()> { +fn boot_gate(seat_ids: &[String], specs: &[AgentSpec], host: &str, catalog: &Path) -> Result<()> { let runner = SystemRunner::new(catalog.to_path_buf(), catalog.join("exec")); - let want: Vec<&str> = agents.iter().map(|a| a.id.as_str()).collect(); + let want: Vec<&str> = seat_ids.iter().map(String::as_str).collect(); let deadline = Instant::now() + Duration::from_secs(5); loop { let sessions = runner.list_sessions().unwrap_or_default(); @@ -638,23 +778,23 @@ fn run_steps( /// `pty peek`), so judges can review/assert an agent's output by log. Best-effort: `pty` has no /// continuous plain-text log, so this is the scrollback captured at judge time — enough to inspect a /// wedged/finished agent's history. A truly continuous agent log would need a `pty` feature. -fn dump_agent_logs(agents: &[SpecAgent], catalog: &Path) { - if agents.is_empty() { +fn dump_agent_logs(seat_ids: &[String], catalog: &Path) { + if seat_ids.is_empty() { return; } let logs_dir = catalog.join("logs"); let _ = std::fs::create_dir_all(&logs_dir); let pty_root = crate::run::effective_pty_root(catalog); - for a in agents { + for seat_id in seat_ids { let out = std::process::Command::new("pty") - .args(["peek", "--full", "--plain", &a.id]) + .args(["peek", "--full", "--plain", seat_id]) .env("PTY_ROOT", &pty_root) .output(); if let Ok(o) = out && o.status.success() && !o.stdout.is_empty() { - let _ = std::fs::write(logs_dir.join(format!("{}.log", a.id)), &o.stdout); + let _ = std::fs::write(logs_dir.join(format!("{seat_id}.log")), &o.stdout); } } } @@ -668,16 +808,21 @@ fn env_key(id: &str) -> String { /// root (whose supervisor is `None` — the cos). Returns the ancestor ids, nearest first. A cycle or a /// supervisor that names no declared agent terminates the walk (the named id is still included — we ding /// its inbox regardless of whether it is a booted seat). -fn supervisor_chain(seat_id: &str, specs: &[AgentSpec]) -> Vec { +fn supervisor_chain(seat_id: &str, specs: &[AgentSpec], host: &str) -> Vec { let mut chain = Vec::new(); let mut seen = std::collections::HashSet::new(); - let mut current = specs.iter().find(|s| s.identity == seat_id).and_then(|s| s.supervisor.clone()); + let find = |identity: &str| { + specs + .iter() + .find(|spec| spec.identity == identity || spec.bus_id(host) == identity) + }; + let mut current = find(seat_id).and_then(|s| s.supervisor.clone()); while let Some(sup) = current { if !seen.insert(sup.clone()) { break; // cycle guard } chain.push(sup.clone()); - current = specs.iter().find(|s| s.identity == sup).and_then(|s| s.supervisor.clone()); + current = find(&sup).and_then(|s| s.supervisor.clone()); } chain } @@ -685,8 +830,14 @@ fn supervisor_chain(seat_id: &str, specs: &[AgentSpec]) -> Vec { /// Emit a crash-ding for a crashed seat: a `worker crash: ` bus message to EVERY ancestor in its /// supervisor chain (the direct supervisor up to the cos root) — so the whole supervision chain learns /// the worker died without polling. A seat with no supervisor has nothing to notify. -fn crash_ding(seat_id: &str, specs: &[AgentSpec], bus: &Path) { - let chain = supervisor_chain(seat_id, specs); +fn crash_ding( + seat_id: &str, + specs: &[AgentSpec], + bus: &Path, + host: &str, + canonical_bus: bool, +) { + let chain = supervisor_chain(seat_id, specs, host); if chain.is_empty() { return; } @@ -696,7 +847,11 @@ fn crash_ding(seat_id: &str, specs: &[AgentSpec], bus: &Path) { st2 respawned it from spec; surfacing the crash up the supervision chain." ); for ancestor in &chain { - let inbox = bus.join(ancestor).join("inbox"); + let inbox = if canonical_bus { + crate::message::resolve_inbox(bus, ancestor, host) + } else { + bus.join(ancestor).join("inbox") + }; let _ = crate::message::send_to_inbox(&inbox, "st2", Some(&subject), None, &[], &body); eval_log!("== crash-ding: {seat_id} → {ancestor} =="); } @@ -717,44 +872,106 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos // the verdict); `run_env` ($RUNS_DIR + each $RUN__EXIT) is handed to the judges to read captures. let (mut judges, run_env) = run_steps(&eval.run_steps, catalog, &spec.env); - // The base team + eval-only agents. An eval with NONE is TEAM-LESS: the jobs are the work. - let mut agents = spec.agents.clone(); - agents.extend(eval.agents.clone()); + // The base team + eval-only compact agents. `canonical-agents` is a mutually exclusive authority: + // it discovers the post-run hermetic catalog rather than projecting this compact grammar. + let mut compact_agents = spec.agents.clone(); + compact_agents.extend(eval.agents.clone()); - let (done, specs) = if agents.is_empty() { + let (done, specs, seat_ids) = if compact_agents.is_empty() && !eval.canonical_agents { // TEAM-LESS: nothing to boot, kick off, or wait on — the run steps did the work → straight to judging. if !eval.run_steps.is_empty() { eval_log!("== team-less eval: {} run step(s) ran → judging ==", eval.run_steps.len()); } - (true, Vec::new()) + (true, Vec::new(), Vec::new()) } else { - let mut specs = spec_to_agent_specs(&agents, host, catalog); - if eval.supervise { + let (mut specs, seat_ids, participant_ids) = if eval.canonical_agents { + if bus != catalog { + anyhow::bail!( + "canonical-agents requires the native flat ST_ROOT `{}`, got `{}`", + catalog.display(), + bus.display() + ); + } + let team = load_canonical_eval_team(catalog, host)?; + let participants = team + .specs + .iter() + .map(|spec| spec.bus_id(host)) + .collect::>(); + (team.specs, team.seat_ids, participants) + } else { + let specs = spec_to_agent_specs(&compact_agents, host, catalog); + let seats = compact_agents + .iter() + .map(|agent| agent.id.clone()) + .collect::>(); + let participants = compact_agents + .iter() + .map(|agent| agent.id.clone()) + .collect::>(); + (specs, seats, participants) + }; + if eval.supervise && !eval.canonical_agents { add_eval_exit_markers(&mut specs, catalog); } - // Pre-trust each agent's workspace so a real claude seat never hangs on the trust dialog. - let dirs: Vec = - agents.iter().filter_map(|a| a.workspace.as_deref()).map(|w| catalog.join(w)).collect(); - if !dirs.is_empty() { - let _ = crate::pretrust::pretrust(&dirs); + if !eval.canonical_agents { + // Compact legacy seats intentionally retain their historical ambient trust behavior. + // Canonical managed Agent Specs own trust inside their declared adapter trajectory. + let dirs: Vec = compact_agents + .iter() + .filter_map(|a| a.workspace.as_deref()) + .map(|w| catalog.join(w)) + .collect(); + if !dirs.is_empty() { + let _ = crate::pretrust::pretrust(&dirs); + } } + let canonical_sup = if eval.canonical_agents { + let msg = eval.message.as_ref().ok_or_else(|| { + anyhow::anyhow!( + "a canonical-agents eval needs a message{{}} kickoff before any seat can launch" + ) + })?; + let matches = specs + .iter() + .filter(|agent| agent.identity == msg.to || agent.bus_id(host) == msg.to) + .map(|agent| agent.bus_id(host)) + .collect::>(); + let [target] = matches.as_slice() else { + anyhow::bail!( + "canonical-agents kickoff target `{}` must resolve to exactly one Agent Spec, found {}", + msg.to, + matches.len() + ); + }; + Some(target.clone()) + } else { + None + }; eval_log!("== boot team ({} agents) ==", specs.len()); boot_team(&specs, host, catalog)?; - boot_gate(&agents, &specs, host, catalog)?; + boot_gate(&seat_ids, &specs, host, catalog)?; // Deliver the kickoff onto the bus the seats' dings watch (ST_ROOT), from the requester. let msg = eval.message.as_ref().ok_or_else(|| { anyhow::anyhow!("a team eval needs a message{{}} kickoff (only a team-less eval may omit it)") })?; let body = resolve_content(&msg.content, spec_dir)?; - let to_inbox = bus.join(&msg.to).join("inbox"); + let sup = canonical_sup.unwrap_or_else(|| msg.to.clone()); + let to_inbox = if eval.canonical_agents { + crate::message::resolve_inbox(&bus, &sup, host) + } else { + bus.join(&sup).join("inbox") + }; crate::message::send_to_inbox(&to_inbox, &msg.from, None, None, &[], &body) .with_context(|| format!("seeding kickoff into {}", to_inbox.display()))?; - eval_log!("== kickoff → {} (from {}) ==", msg.to, msg.from); + eval_log!("== kickoff → {sup} (from {}) ==", msg.from); - let sup = msg.to.clone(); - let workers: Vec = spec.agents.iter().map(|a| a.id.clone()).filter(|id| *id != sup).collect(); + let workers: Vec = participant_ids + .into_iter() + .filter(|id| *id != sup) + .collect(); eval_log!( "== waiting for {sup}→{} confirmation post-dating a worker report (≤{:?}) ==", msg.from, eval.max_timeout @@ -774,7 +991,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos // as never booted and suppress its crash ding. `dinged` dedups so one crash = one ding // until the seat is alive again. let mut ever_alive: std::collections::HashSet = - specs.iter().map(|seat| seat.identity.clone()).collect(); + seat_ids.iter().cloned().collect(); let mut dinged: std::collections::HashSet = std::collections::HashSet::new(); let mut tick = || { if eval.supervise { @@ -789,7 +1006,9 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos &crate::reconcile::Session, > = sessions.iter().map(|s| (s.pty_id.as_str(), s)).collect(); for seat in &specs { - let id = seat.identity.as_str(); // the seat's main pty session id == its identity + let id = main_pty_id(seat, host) + .expect("eval team main PTY was validated"); + let id = id.as_str(); match by_id.get(id) { Some(s) if s.alive => { ever_alive.insert(id.to_string()); @@ -803,7 +1022,18 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos && !clean && !dinged.contains(id) { - crash_ding(&seat.identity, &specs, &bus); + let ding_id = if eval.canonical_agents { + seat.bus_id(host) + } else { + seat.identity.clone() + }; + crash_ding( + &ding_id, + &specs, + &bus, + host, + eval.canonical_agents, + ); dinged.insert(id.to_string()); } } @@ -833,7 +1063,16 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos } } }; - wait_done(&bus, &sup, &msg.from, &workers, eval.max_timeout, &mut tick) + wait_done( + &bus, + host, + eval.canonical_agents, + &sup, + &msg.from, + &workers, + eval.max_timeout, + &mut tick, + ) }; if EVAL_INTERRUPTED.load(Ordering::SeqCst) { @@ -845,13 +1084,13 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos } else { eval_log!("== max-timeout: no confirmation within {:?} — judging the final state ==", eval.max_timeout); } - (done, specs) + (done, specs, seat_ids) }; // Snapshot each agent's terminal output to logs/.log so judges can review/assert an agent's // output by log (alongside the run-step + exec sidecar logs already there). Done BEFORE teardown so // the sessions are still peekable. - dump_agent_logs(&agents, catalog); + dump_agent_logs(&seat_ids, catalog); // Judges: the run-step gate results first, then the declared judges (all must pass). Judge BEFORE // teardown — an ask-agent judge needs its judge agent still alive to answer. @@ -1048,6 +1287,80 @@ mod tests { use crate::reconcile::{Session, TaskTarget}; use std::cell::RefCell; + fn write_eval_agent(catalog: &Path, relative: &str, body: &str) { + let path = catalog.join(relative); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, body).unwrap(); + } + + #[test] + fn canonical_eval_team_uses_exact_catalog_declarations_and_runtime_ids() { + let catalog = tempfile::tempdir().unwrap(); + write_eval_agent( + catalog.path(), + "agents/evalhost/sup/agent.kdl", + r#"agent "sup" { + identity "sup" + host "evalhost" + workspace "$CATALOG/sup" + argv "sh" "-c" "sleep 60" +} +"#, + ); + write_eval_agent( + catalog.path(), + "agents/evalhost/worker/agent.kdl", + r#"agent "worker" { + identity "worker" + host "evalhost" + workspace "$CATALOG/worker" + argv "sh" "-c" "sleep 60" +} +"#, + ); + + let team = load_canonical_eval_team(catalog.path(), "evalhost").unwrap(); + assert_eq!(team.specs.len(), 2); + assert_eq!(team.seat_ids, ["evalhost.sup", "evalhost.worker"]); + assert!(team.specs.iter().all(|spec| spec.path.ends_with("agent.kdl"))); + } + + #[test] + fn canonical_eval_team_fails_closed_before_launch_on_zero_duplicate_or_noncanonical_specs() { + let empty = tempfile::tempdir().unwrap(); + assert!( + load_canonical_eval_team(empty.path(), "evalhost") + .unwrap_err() + .to_string() + .contains("no canonical Agent Specs") + ); + + let misplaced = tempfile::tempdir().unwrap(); + write_eval_agent( + misplaced.path(), + "fixture/agent.kdl", + r#"agent "worker" { identity "worker"; host "evalhost"; argv "true" }"#, + ); + let error = load_canonical_eval_team(misplaced.path(), "evalhost") + .unwrap_err() + .to_string(); + assert!(error.contains("canonical path"), "{error}"); + + let duplicate = tempfile::tempdir().unwrap(); + write_eval_agent( + duplicate.path(), + "agents/evalhost/worker/agent.kdl", + r#" +agent "worker" { identity "worker"; host "evalhost"; argv "true" } +agent "worker" { identity "worker"; host "evalhost"; argv "true" } +"#, + ); + let error = load_canonical_eval_team(duplicate.path(), "evalhost") + .unwrap_err() + .to_string(); + assert!(error.contains("duplicate") || error.contains("exactly one"), "{error}"); + } + struct RaceRunner { lists: RefCell>>, ops: RefCell> } impl Runner for RaceRunner { fn list_sessions(&self) -> anyhow::Result> { Ok(self.lists.borrow_mut().remove(0)) } @@ -1206,15 +1519,15 @@ mod tests { let workers = vec!["mix.worker".to_string()]; let noop = &mut (|| {}) as &mut dyn FnMut(); // No worker report → never fires (bounded timeout). - assert!(!wait_done(root, sup, req, &workers, Duration::from_millis(100), noop)); + assert!(!wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(100), noop)); // A worker→sup report at t=2000 + a sup→requester confirm that PRE-dates it (t=1000) → false // (the early "on it" ack the discriminator exists to reject). seed_msg(&root.join(sup).join("inbox"), 1_700_000_002_000, "aaaaaa", "mix.worker"); seed_msg(&root.join(req).join("inbox"), 1_700_000_001_000, "bbbbbb", "mix.sup"); - assert!(!wait_done(root, sup, req, &workers, Duration::from_millis(100), noop)); + assert!(!wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(100), noop)); // A confirm that POST-dates the report (t=3000) → done. seed_msg(&root.join(req).join("inbox"), 1_700_000_003_000, "cccccc", "mix.sup"); - assert!(wait_done(root, sup, req, &workers, Duration::from_millis(2000), noop)); + assert!(wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(2000), noop)); } #[test] @@ -1232,7 +1545,7 @@ mod tests { seed_msg(&root.join(sup).join("archive"), 1_700_000_002_000, "aaaaaa", "mix.worker"); seed_msg(&root.join(req).join("inbox"), 1_700_000_003_000, "cccccc", "mix.sup"); assert!( - wait_done(root, sup, req, &workers, Duration::from_millis(2000), noop), + wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(2000), noop), "an archived worker report must still be seen (else archive-on-act hygiene hangs the eval)" ); } @@ -1245,7 +1558,16 @@ mod tests { let bus = tempfile::tempdir().unwrap(); let ticks = std::cell::Cell::new(0u32); let mut tick = || ticks.set(ticks.get() + 1); - let fired = wait_done(bus.path(), "sup", "req", &["w".to_string()], Duration::from_millis(400), &mut tick); + let fired = wait_done( + bus.path(), + "h", + false, + "sup", + "req", + &["w".to_string()], + Duration::from_millis(400), + &mut tick, + ); assert!(!fired, "no confirmation was seeded → must time out"); assert!(ticks.get() >= 1, "the supervise tick must run during the wait"); } diff --git a/src/eval_spec.rs b/src/eval_spec.rs index 84d4c658..c9b0bf42 100644 --- a/src/eval_spec.rs +++ b/src/eval_spec.rs @@ -1,8 +1,8 @@ //! The **st2 spec** — the native, legible eval/agent format (`st2 eval ./folder`), nailed down in -//! review. One `.kdl` file describes a team -//! and (optionally) an `eval` that copies a fixture, kicks off the team, waits for the sup's -//! confirmation, and runs judges. This module is the PARSER + model (P1); `st2 up`/`st2 eval` (team -//! boot + eval flow + judge engine) build on it. +//! review. One `.kdl` file describes a compact team, or explicitly selects canonical Agent Specs +//! already materialized in the eval's hermetic catalog, and an optional `eval` that copies a +//! fixture, kicks off the team, waits for the sup's confirmation, and runs judges. This module is +//! the PARSER + model (P1); `st2 up`/`st2 eval` (team boot + eval flow + judge engine) build on it. //! //! Principles (from the design — do not violate): legible top-to-bottom, everything in one file, st2 //! self-contained (`st2 ding`, only `pty` on PATH). Ergonomic collapses (review-driven): a bare `ding` @@ -86,6 +86,10 @@ pub struct Eval { /// cell. Recovery remains respawn-from-spec rather than `pty restart`, so eval supervision stays /// owned by st2 even though PTY metadata can now restore the managed environment manually. pub supervise: bool, + /// Opt-in: after `copy` and `run` finish, discover the eval team from canonical Agent Specs in + /// the hermetic catalog. Mutually exclusive with the compact `team` / `agent` grammar: one eval + /// has one declaration authority. + pub canonical_agents: bool, } /// The kickoff. `content` is a file path (relative to the spec folder) or inline text — resolved at @@ -341,6 +345,13 @@ pub fn parse_spec(text: &str) -> anyhow::Result { // Re-fold: agents parsed before `env` would miss it, so parse env first. Enforce order simply: // if any agent has empty env but a top-level env exists, it means `env` came after — reject for // legibility (declare env at the top). + if eval.as_ref().is_some_and(|eval| eval.canonical_agents) + && (!agents.is_empty() || eval.as_ref().is_some_and(|eval| !eval.agents.is_empty())) + { + anyhow::bail!( + "eval `canonical-agents` is mutually exclusive with compact `team` / `agent` seats" + ); + } Ok(Spec { host, env: top_env, agents, eval }) } @@ -472,6 +483,7 @@ fn parse_eval(node: &KdlNode, top_env: &BTreeMap) -> anyhow::Res let mut run_steps = Vec::new(); let mut judges = Vec::new(); let mut supervise = false; + let mut canonical_agents = false; for c in ch.nodes() { match c.name().value() { @@ -489,8 +501,18 @@ fn parse_eval(node: &KdlNode, top_env: &BTreeMap) -> anyhow::Res "judges" => judges = parse_judges(c)?, // Opt-in: a bare `supervise` directive turns on respawn-from-spec during the run. "supervise" => supervise = true, + // Opt-in: use canonical Agent Specs materialized in the hermetic eval catalog. + "canonical-agents" => { + if canonical_agents { + anyhow::bail!("eval: `canonical-agents` may occur only once"); + } + if !c.entries().is_empty() || c.children().is_some() { + anyhow::bail!("eval: `canonical-agents` is a bare directive"); + } + canonical_agents = true; + } other => anyhow::bail!( - "eval: unexpected node '{other}' (expected copy|message|max-timeout|agent|team|run|judges|supervise)" + "eval: unexpected node '{other}' (expected copy|message|max-timeout|agent|team|run|judges|supervise|canonical-agents)" ), } } @@ -506,6 +528,7 @@ fn parse_eval(node: &KdlNode, top_env: &BTreeMap) -> anyhow::Res run_steps, judges, supervise, + canonical_agents, }) } @@ -835,6 +858,28 @@ team "mix" { assert!(!parse_spec(&ev("")).unwrap().eval.unwrap().supervise); } + #[test] + fn canonical_agents_is_bare_once_and_excludes_compact_seats() { + let canonical = parse_spec( + "eval {\n canonical-agents\n message { from \"r\"; to \"h.sup\"; content \"go\" }\n max-timeout \"5s\"\n}\n", + ) + .unwrap(); + assert!(canonical.eval.unwrap().canonical_agents); + + for invalid in [ + "eval { canonical-agents \"./fleet\"; max-timeout \"5s\"; run \"x\" { command \"true\" } }", + "eval { canonical-agents { }; max-timeout \"5s\"; run \"x\" { command \"true\" } }", + "eval { canonical-agents; canonical-agents; max-timeout \"5s\"; run \"x\" { command \"true\" } }", + "agent \"a\" { command \"true\" }\neval { canonical-agents; message { from \"r\"; to \"a\"; content \"go\" }; max-timeout \"5s\" }", + "eval { canonical-agents; agent \"a\" { command \"true\" }; message { from \"r\"; to \"a\"; content \"go\" }; max-timeout \"5s\" }", + ] { + assert!( + parse_spec(invalid).is_err(), + "accepted ambiguous canonical agent authority:\n{invalid}" + ); + } + } + #[test] fn judge_signal_flag_is_opt_in() { // A bare `signal` directive marks a judge show-but-don't-gate; absent → a normal gating judge. diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index 912bb99d..47b38938 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -194,6 +194,324 @@ sleep 60 assert!(!invalid.status.success(), "invalid eval input must retain nonzero exit"); } +#[test] +fn canonical_agents_run_from_the_hermetic_catalog_with_one_root_and_native_bus() { + if !pty_available() { + assert!( + std::env::var_os("ST2_ALLOW_PTY_SKIP").is_some(), + "`pty` not on PATH; set ST2_ALLOW_PTY_SKIP=1" + ); + eprintln!("SKIP canonical_agents_run_from_the_hermetic_catalog_with_one_root_and_native_bus"); + return; + } + + let bin = env!("CARGO_BIN_EXE_st2"); + let bin_dir = Path::new(bin).parent().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let cell = tmp.path().join("cell"); + let fixture = cell.join("fixture"); + for path in [ + "agents/evalhost/sup", + "agents/evalhost/worker", + "scripts", + "sup", + "worker", + ] { + std::fs::create_dir_all(fixture.join(path)).unwrap(); + } + std::fs::write( + cell.join("cell.kdl"), + r#" +host "evalhost" +eval { + copy "./fixture" + canonical-agents + message { from "requester"; to "evalhost.sup"; content "do the bounded work" } + max-timeout "30s" + judges { + judge "canonical team completed" { exec "test -f $CATALOG/worker/DONE" } + judge "one eval-owned root" { + exec "test -f $CATALOG/sup/roots-ok && test -f $CATALOG/worker/roots-ok" + } + judge "kickoff used canonical inbox" { + exec "test -d $CATALOG/agents/evalhost/sup/resources/inbox && test ! -e $CATALOG/evalhost.sup/inbox" + } + } +} +"#, + ) + .unwrap(); + std::fs::write( + fixture.join("agents/evalhost/sup/agent.kdl"), + r#"agent "sup" { + identity "sup" + host "evalhost" + workspace "$CATALOG/sup" + env { ST_AGENT "evalhost.sup" } + argv "sh" "$CATALOG/scripts/sup.sh" +} +"#, + ) + .unwrap(); + std::fs::write( + fixture.join("agents/evalhost/worker/agent.kdl"), + r#"agent "worker" { + identity "worker" + host "evalhost" + workspace "$CATALOG/worker" + supervisor "sup" + env { ST_AGENT "evalhost.worker" } + argv "sh" "$CATALOG/scripts/worker.sh" +} +"#, + ) + .unwrap(); + std::fs::write( + fixture.join("scripts/worker.sh"), + r#"#!/bin/sh +test "$CATALOG" = "$ST_ROOT" && + test "$PTY_ROOT" = "$CATALOG/pty" && + test "$ST_AGENT" = "evalhost.worker" && + : > "$CATALOG/worker/roots-ok" +: > "$CATALOG/worker/DONE" +st2 message send evalhost.sup --root "$ST_ROOT" --as evalhost.worker -m "worker done" >/dev/null 2>&1 +exec sleep 60 +"#, + ) + .unwrap(); + std::fs::write( + fixture.join("scripts/sup.sh"), + r#"#!/bin/sh +test "$CATALOG" = "$ST_ROOT" && + test "$PTY_ROOT" = "$CATALOG/pty" && + test "$ST_AGENT" = "evalhost.sup" && + : > "$CATALOG/sup/roots-ok" +for _ in $(seq 1 150); do + kick=$(st2 message ls evalhost.sup --root "$ST_ROOT" --from requester --count 2>/dev/null || echo 0) + report=$(st2 message ls evalhost.sup --root "$ST_ROOT" --from evalhost.worker --count 2>/dev/null || echo 0) + [ "$kick" -gt 0 ] && [ "$report" -gt 0 ] && break + sleep 0.2 +done +st2 message send requester --root "$ST_ROOT" --as evalhost.sup -m "done" >/dev/null 2>&1 +exec sleep 60 +"#, + ) + .unwrap(); + + let path = format!("{}:{}", bin_dir.display(), std::env::var("PATH").unwrap_or_default()); + let out = Command::new(bin) + .args(["eval", "--host", "evalhost"]) + .arg(&cell) + .env("PATH", path) + .env_remove("CATALOG") + .env_remove("ST_ROOT") + .env_remove("PTY_ROOT") + .env("XDG_STATE_HOME", tmp.path().join("xdg")) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success() + && stdout.contains("team signalled done") + && stdout.contains("SCORE: 3 PASS / 0 FAIL"), + "canonical eval did not close:\n--stdout--\n{stdout}\n--stderr--\n{stderr}" + ); +} + +#[test] +fn fixture_agent_specs_are_inert_without_canonical_agents_opt_in() { + if !pty_available() { + assert!( + std::env::var_os("ST2_ALLOW_PTY_SKIP").is_some(), + "`pty` not on PATH; set ST2_ALLOW_PTY_SKIP=1" + ); + eprintln!("SKIP fixture_agent_specs_are_inert_without_canonical_agents_opt_in"); + return; + } + let bin = env!("CARGO_BIN_EXE_st2"); + let bin_dir = Path::new(bin).parent().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let cell = tmp.path().join("cell"); + let fixture = cell.join("fixture"); + std::fs::create_dir_all(fixture.join("agents/evalhost/legacy")).unwrap(); + std::fs::create_dir_all(fixture.join("scripts")).unwrap(); + std::fs::write( + fixture.join("agents/evalhost/legacy/agent.kdl"), + r#"agent "legacy" { + identity "legacy" + host "evalhost" + argv "sh" "-c" "touch \"$CATALOG/CANONICAL-SHOULD-NOT-LAUNCH\"; sleep 60" +} +"#, + ) + .unwrap(); + std::fs::write( + fixture.join("scripts/legacy.sh"), + r#"#!/bin/sh +for _ in $(seq 1 100); do + set -- "$ST_ROOT/legacy/inbox/"*.md + [ -e "$1" ] && : > "$CATALOG/SAW-FLAT-KICKOFF" && break + sleep 0.05 +done +exec sleep 60 +"#, + ) + .unwrap(); + std::fs::write( + cell.join("cell.kdl"), + r#" +host "evalhost" +agent "legacy" { + env { ST_AGENT "legacy" } + command "sh $CATALOG/scripts/legacy.sh" +} +eval { + copy "./fixture" + message { from "requester"; to "legacy"; content "go" } + max-timeout "2s" + judges { + judge "legacy flat authority stayed intact" { + exec "test -f $CATALOG/SAW-FLAT-KICKOFF && test ! -e $CATALOG/CANONICAL-SHOULD-NOT-LAUNCH && test ! -e $CATALOG/agents/evalhost/legacy/resources/inbox" + } + } +} +"#, + ) + .unwrap(); + let path = format!("{}:{}", bin_dir.display(), std::env::var("PATH").unwrap_or_default()); + let out = Command::new(bin) + .args(["eval", "--host", "evalhost"]) + .arg(&cell) + .env("PATH", path) + .env_remove("CATALOG") + .env_remove("ST_ROOT") + .env_remove("PTY_ROOT") + .env("XDG_STATE_HOME", tmp.path().join("xdg")) + .output() + .unwrap(); + assert!( + out.status.success(), + "colliding fixture Agent Spec changed legacy eval semantics:\n{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn canonical_agents_reject_noncanonical_declarations_before_spawn() { + let bin = env!("CARGO_BIN_EXE_st2"); + let tmp = tempfile::tempdir().unwrap(); + let cell = tmp.path().join("cell"); + let fixture = cell.join("fixture"); + std::fs::create_dir_all(fixture.join("misplaced")).unwrap(); + std::fs::write( + fixture.join("misplaced/agent.kdl"), + r#"agent "bad" { + identity "bad" + host "evalhost" + argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" +} +"#, + ) + .unwrap(); + std::fs::write( + cell.join("cell.kdl"), + r#" +host "evalhost" +eval { + copy "./fixture" + canonical-agents + message { from "requester"; to "evalhost.bad"; content "go" } + max-timeout "10s" + judges { judge "never reached" { exec "false" } } +} +"#, + ) + .unwrap(); + let child = Command::new(bin) + .args(["eval", "--keep", "--host", "evalhost"]) + .arg(&cell) + .env_remove("CATALOG") + .env_remove("ST_ROOT") + .env_remove("PTY_ROOT") + .env("XDG_STATE_HOME", tmp.path().join("xdg")) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let catalog = std::env::temp_dir().join(format!("st2e-{}", child.id())); + let _catalog_cleanup = RemoveDirOnDrop(catalog.clone()); + let out = child.wait_with_output().unwrap(); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!(!out.status.success(), "noncanonical declaration was accepted:\n{combined}"); + assert!(combined.contains("canonical path"), "wrong refusal:\n{combined}"); + assert!(!catalog.join("SPAWNED").exists(), "seat spawned before canonical admission"); +} + +#[test] +fn canonical_agents_reject_an_unknown_kickoff_target_before_spawn() { + let bin = env!("CARGO_BIN_EXE_st2"); + let tmp = tempfile::tempdir().unwrap(); + let cell = tmp.path().join("cell"); + let fixture = cell.join("fixture"); + std::fs::create_dir_all(fixture.join("agents/evalhost/valid")).unwrap(); + std::fs::write( + fixture.join("agents/evalhost/valid/agent.kdl"), + r#"agent "valid" { + identity "valid" + host "evalhost" + argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" +} +"#, + ) + .unwrap(); + std::fs::write( + cell.join("cell.kdl"), + r#" +host "evalhost" +eval { + copy "./fixture" + canonical-agents + message { from "requester"; to "evalhost.missing"; content "go" } + max-timeout "10s" + judges { judge "never reached" { exec "false" } } +} +"#, + ) + .unwrap(); + let child = Command::new(bin) + .args(["eval", "--keep", "--host", "evalhost"]) + .arg(&cell) + .env_remove("CATALOG") + .env_remove("ST_ROOT") + .env_remove("PTY_ROOT") + .env("XDG_STATE_HOME", tmp.path().join("xdg")) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let catalog = std::env::temp_dir().join(format!("st2e-{}", child.id())); + let _catalog_cleanup = RemoveDirOnDrop(catalog.clone()); + let out = child.wait_with_output().unwrap(); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!(!out.status.success(), "unknown kickoff target was accepted:\n{combined}"); + assert!( + combined.contains("kickoff target `evalhost.missing`") + && combined.contains("found 0"), + "wrong refusal:\n{combined}" + ); + assert!(!catalog.join("SPAWNED").exists(), "seat spawned before kickoff admission"); +} + /// Under `supervise`, teardown reaps RUNTIME-spawned seats too (the team-standup pattern: a seat spins /// up an undeclared peer mid-run), not just the declared team. The seat spawns an undeclared `rtpeer` /// into the eval's hermetic PTY_ROOT; after the eval, no orphan carrying the peer's marker survives. From a40af17c11afd91236092fdc2f109b4082f38594 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:09:35 +0200 Subject: [PATCH 2/4] fix(eval): freeze canonical admission agent-session-id: dev3.dotfiles-cos-misc-agent-runtime-simplification agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /home/schickling/.config/coding-agents/profile.json agent-skills-manifest: /nix/store/nk9iml2841l1yjjg0f6f0d3y60zkg1nn-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@4a0515f --- README.md | 10 +- docs/vrs/spec.md | 33 ++- src/eval_run.rs | 463 +++++++++++++++++++++++++++++++++++++----- tests/eval_run_e2e.rs | 372 ++++++++++++++++++++++++++++++++- 4 files changed, 805 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index 12f9d66c..c17c184a 100644 --- a/README.md +++ b/README.md @@ -409,9 +409,13 @@ eval { `canonical-agents` then discovers and materializes declarations at `agents///agent.kdl`. It is mutually exclusive with compact `team` / `agent` seats: the discovered vector is the sole authority for launch, kickoff routing, supervision, logs, and -teardown. Missing, malformed, duplicate, retired, nonlocal, noncanonical, root-overriding, or -unrunnable declarations fail before a seat starts. Without the directive, Agent Spec-shaped files -inside a fixture remain inert and compact evals retain their flat bus semantics. +teardown. Strict catalog validation, main-PTY admission, and warning-free materialization all finish +before a seat starts; backend launch errors are fatal. The native inbox/archive paths are frozen from +that admitted vector, so later catalog mutation cannot redirect eval traffic. A multi-seat team +completes only after the existing worker-report ordering; a singleton completes on its first +interviewer-to-requester confirmation that post-dates the exact kickoff receipt. Canonical completion +gates the verdict. Without the directive, Agent Spec-shaped files inside a fixture remain inert and +compact evals retain their flat bus and completion semantics. `st2 compile-agent` remains experimental. Hand-authored KDL is the canonical st2 authoring interface, and generated output must be reviewed before materialization. diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index 6f0a1d7b..ff5db00c 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -26,23 +26,34 @@ materializes only declarations at unchanged through launch admission, kickoff resolution, supervision, logging, and teardown. -Admission fails before spawn when discovery is empty, malformed, -warning-bearing, duplicate, retired, nonlocal, noncanonical, root-overriding, -unrunnable, or does not expose exactly one main PTY per declaration. The -kickoff target must resolve to exactly one member of the discovered fleet. The -eval owns one native `CATALOG` / `ST_ROOT` and its `/pty` registry; -declarations cannot override those roots. Workspace renders are materialized -before any seat starts. +Admission applies `validate_for_host` strictly, then fails before spawn when +discovery is empty, malformed, warning-bearing, duplicate, retired, nonlocal, +noncanonical, root-overriding, unrunnable, or does not expose exactly one +independently launchable service main PTY per declaration. Main runtime IDs +must be nonempty and fleet-unique. Materialization warnings and backend launch +errors are fatal. The kickoff target must resolve to exactly one member of the +discovered fleet. The eval owns one native `CATALOG` / `ST_ROOT` and its +`/pty` registry; declarations cannot override those roots. Workspace +renders are materialized before any seat starts. + +Native inbox/archive paths are derived once from the admitted Agent Spec paths +and carried as frozen data; routing never re-discovers the mutable catalog. +The requester alone is an explicit eval-owned flat mailbox. Multi-seat +completion retains the worker-report-before-supervisor-confirmation ordering. +A singleton canonical team completes when its interviewer-to-requester +confirmation post-dates the exact kickoff receipt. Canonical completion is a +gating judge, so a timeout cannot pass on unrelated final-state checks alone. Without `canonical-agents`, fixture declarations are not discovered or launched and compact evals retain their catalog-less flat bus. This explicit opt-in keeps ordinary fixtures inert while allowing the same canonical declaration to be exercised in an eval and real work. Parser and admission evidence lives in `eval_spec::tests::canonical_agents_is_bare_once_and_excludes_compact_seats` and -the `canonical_eval_team_*` unit tests. End-to-end isolation, canonical inbox -routing, the no-opt-in collision control, and pre-spawn path refusal are proven -by the four `canonical_agents_*` / `fixture_agent_specs_*` cases in -`tests/eval_run_e2e.rs`. +the `canonical_*` unit tests. Named-PTY end-to-end cases prove strict +pre-spawn refusals, poisoned ambient-root isolation, real render +materialization, frozen routing after declaration removal, singleton +completion, custom main-ID supervision/logging/teardown, and the no-opt-in +legacy control in `tests/eval_run_e2e.rs`. ## Resource bindings (R20-R21) diff --git a/src/eval_run.rs b/src/eval_run.rs index f7a7666b..1c3e82a8 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -110,6 +110,22 @@ pub fn spec_to_agent_specs(agents: &[SpecAgent], host: &str, root: &Path) -> Vec struct CanonicalEvalTeam { specs: Vec, seat_ids: Vec, + routes: BTreeMap, +} + +#[derive(Debug, Clone)] +struct CanonicalRoute { + inbox: PathBuf, + archive: PathBuf, +} + +fn admitted_route<'a>( + routes: &'a BTreeMap, + id: &str, +) -> &'a CanonicalRoute { + routes + .get(id) + .unwrap_or_else(|| panic!("strict canonical admission did not freeze route for `{id}`")) } fn main_pty_id(spec: &AgentSpec, host: &str) -> Result { @@ -125,16 +141,48 @@ fn main_pty_id(spec: &AgentSpec, host: &str) -> Result { main.len() ); }; - Ok(main + let launchable = match (&main.command, &main.argv) { + (Some(command), None) => !command.trim().is_empty(), + (None, Some(argv)) => argv.first().is_some_and(|program| !program.trim().is_empty()), + _ => false, + }; + if main.lifecycle != TaskLifecycle::Service || !launchable { + anyhow::bail!( + "Agent Spec `{bus_id}` main PTY named `agent` must be an independently launchable service" + ); + } + let id = main .id .clone() - .unwrap_or_else(|| format!("{bus_id}.agent"))) + .unwrap_or_else(|| format!("{bus_id}.agent")); + if id.trim().is_empty() { + anyhow::bail!("Agent Spec `{bus_id}` main PTY id must be nonempty"); + } + Ok(id) } /// Discover the sole declaration authority for a `canonical-agents` eval after its fixture and run /// steps have populated the hermetic catalog. This deliberately consumes the shared Agent Spec /// parser instead of projecting the compact eval grammar into a second, partial declaration. fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result { + let validation = crate::validate::validate_for_host(catalog, host); + if !validation.issues.is_empty() { + let issues = validation + .issues + .iter() + .map(|issue| { + format!( + "{} {} {}: {}", + issue.severity.tag(), + issue.code, + issue.path, + issue.message + ) + }) + .collect::>() + .join("; "); + anyhow::bail!("canonical eval Agent Specs failed strict validation: {issues}"); + } let found = crate::discover(catalog); if !found.errors.is_empty() { let errors = found @@ -163,7 +211,9 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result::new(); let mut bus_ids = HashSet::new(); + let mut main_ids = HashSet::new(); let mut seat_ids = Vec::new(); + let mut routes = BTreeMap::new(); for spec in &found.specs { *paths.entry(spec.path.clone()).or_default() += 1; let bus_id = spec.bus_id(host); @@ -203,7 +253,21 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result Result, id: &str) -> bool { from.is_some_and(|f| f == id || f.ends_with(&format!(".{id}"))) } -/// Wait for the DONE signal, message-driven (not grade-poll): a `sup → requester` confirmation whose -/// timestamp POST-DATES a `worker → sup` report (Q1(b) — the same discriminator the graders use, so an -/// early "on it" ack from the sup can't fire it). Bounded by `timeout`. Returns whether done fired. +/// Wait for the DONE signal, message-driven (not grade-poll). Multi-seat teams require a +/// `sup → requester` confirmation whose timestamp follows a `worker → sup` report. A canonical +/// singleton instead requires its confirmation to post-date the exact kickoff receipt. Compact +/// singleton semantics remain unchanged. Bounded by `timeout`. Returns whether done fired. fn wait_done( bus: &Path, - host: &str, - canonical_bus: bool, + canonical_routes: Option<&BTreeMap>, sup: &str, requester: &str, workers: &[String], + kickoff_ts: Option, timeout: Duration, on_tick: &mut dyn FnMut(), ) -> bool { - let inbox = |id: &str| { - if canonical_bus { - crate::message::resolve_inbox(bus, id, host) - } else { - bus.join(id).join("inbox") - } - }; - let archive = |id: &str| { - if canonical_bus { - crate::message::resolve_archive(bus, id, host) - } else { - bus.join(id).join("archive") + let (sup_inbox, sup_archive) = match canonical_routes { + Some(routes) => { + let route = admitted_route(routes, sup); + (route.inbox.clone(), route.archive.clone()) } + None => ( + bus.join(sup).join("inbox"), + bus.join(sup).join("archive"), + ), }; - let sup_inbox = inbox(sup); - let sup_archive = archive(sup); - let req_inbox = inbox(requester); + // The requester is eval-owned, not an admitted Agent Spec, and deliberately keeps one explicit + // flat mailbox. Every canonical agent route above comes from the frozen admitted vector. + let req_inbox = bus.join(requester).join("inbox"); let deadline = Instant::now() + timeout; loop { if EVAL_INTERRUPTED.load(Ordering::SeqCst) { @@ -492,6 +557,17 @@ fn wait_done( // a passive eval-runner seed that never archives.) let sup_msgs = crate::message::list_dir(&sup_inbox).unwrap_or_default(); let sup_archived = crate::message::list_dir(&sup_archive).unwrap_or_default(); + if workers.is_empty() + && let Some(kickoff_ts) = kickoff_ts + { + let confirmed = crate::message::list_dir(&req_inbox) + .unwrap_or_default() + .iter() + .any(|m| from_is(m.from.as_deref(), sup) && m.ts_ms > kickoff_ts); + if confirmed { + return true; + } + } let report_ts = sup_msgs .iter() .chain(sup_archived.iter()) @@ -545,6 +621,39 @@ fn boot_gate(seat_ids: &[String], specs: &[AgentSpec], host: &str, catalog: &Pat } } +fn require_canonical_boot(report: &UpReport, seat_ids: &[String]) -> Result<()> { + let missing = seat_ids + .iter() + .filter(|id| !report.launched.contains(id)) + .cloned() + .collect::>(); + if report.skipped + || !report.errors.is_empty() + || !report.flapping.is_empty() + || !report.held.is_empty() + || !report.unrunnable.is_empty() + || !missing.is_empty() + { + anyhow::bail!( + "canonical Agent Spec boot did not launch every admitted main PTY: missing={missing:?}; \ + skipped={}; held={:?}; unrunnable={:?}; flapping={:?}; errors={:?}", + report.skipped, + report.held, + report.unrunnable, + report.flapping, + report.errors + ); + } + Ok(()) +} + +fn message_timestamp(filename: &str) -> Result { + filename + .split_once('-') + .and_then(|(timestamp, _)| timestamp.parse().ok()) + .ok_or_else(|| anyhow::anyhow!("message receipt `{filename}` has no timestamp")) +} + /// Tear down the team (nomad-safe): mark the specs retired and reconcile → the runner kills the live /// sessions (process-group kill). Best-effort — an eval always tears down, no zombie seats. /// @@ -835,7 +944,7 @@ fn crash_ding( specs: &[AgentSpec], bus: &Path, host: &str, - canonical_bus: bool, + canonical_routes: Option<&BTreeMap>, ) { let chain = supervisor_chain(seat_id, specs, host); if chain.is_empty() { @@ -847,10 +956,9 @@ fn crash_ding( st2 respawned it from spec; surfacing the crash up the supervision chain." ); for ancestor in &chain { - let inbox = if canonical_bus { - crate::message::resolve_inbox(bus, ancestor, host) - } else { - bus.join(ancestor).join("inbox") + let inbox = match canonical_routes { + Some(routes) => admitted_route(routes, ancestor).inbox.clone(), + None => bus.join(ancestor).join("inbox"), }; let _ = crate::message::send_to_inbox(&inbox, "st2", Some(&subject), None, &[], &body); eval_log!("== crash-ding: {seat_id} → {ancestor} =="); @@ -884,7 +992,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos } (true, Vec::new(), Vec::new()) } else { - let (mut specs, seat_ids, participant_ids) = if eval.canonical_agents { + let (mut specs, seat_ids, participant_ids, canonical_routes) = if eval.canonical_agents { if bus != catalog { anyhow::bail!( "canonical-agents requires the native flat ST_ROOT `{}`, got `{}`", @@ -898,7 +1006,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos .iter() .map(|spec| spec.bus_id(host)) .collect::>(); - (team.specs, team.seat_ids, participants) + (team.specs, team.seat_ids, participants, Some(team.routes)) } else { let specs = spec_to_agent_specs(&compact_agents, host, catalog); let seats = compact_agents @@ -909,7 +1017,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos .iter() .map(|agent| agent.id.clone()) .collect::>(); - (specs, seats, participants) + (specs, seats, participants, None) }; if eval.supervise && !eval.canonical_agents { add_eval_exit_markers(&mut specs, catalog); @@ -950,7 +1058,10 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos }; eval_log!("== boot team ({} agents) ==", specs.len()); - boot_team(&specs, host, catalog)?; + let boot = boot_team(&specs, host, catalog)?; + if eval.canonical_agents { + require_canonical_boot(&boot, &seat_ids)?; + } boot_gate(&seat_ids, &specs, host, catalog)?; // Deliver the kickoff onto the bus the seats' dings watch (ST_ROOT), from the requester. @@ -959,13 +1070,17 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos })?; let body = resolve_content(&msg.content, spec_dir)?; let sup = canonical_sup.unwrap_or_else(|| msg.to.clone()); - let to_inbox = if eval.canonical_agents { - crate::message::resolve_inbox(&bus, &sup, host) - } else { - bus.join(&sup).join("inbox") + let to_inbox = match canonical_routes.as_ref() { + Some(routes) => admitted_route(routes, &sup).inbox.clone(), + None => bus.join(&sup).join("inbox"), }; - crate::message::send_to_inbox(&to_inbox, &msg.from, None, None, &[], &body) + let kickoff_receipt = + crate::message::send_to_inbox(&to_inbox, &msg.from, None, None, &[], &body) .with_context(|| format!("seeding kickoff into {}", to_inbox.display()))?; + let kickoff_ts = eval + .canonical_agents + .then(|| message_timestamp(&kickoff_receipt)) + .transpose()?; eval_log!("== kickoff → {sup} (from {}) ==", msg.from); let workers: Vec = participant_ids @@ -1032,7 +1147,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos &specs, &bus, host, - eval.canonical_agents, + canonical_routes.as_ref(), ); dinged.insert(id.to_string()); } @@ -1065,11 +1180,11 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos }; wait_done( &bus, - host, - eval.canonical_agents, + canonical_routes.as_ref(), &sup, &msg.from, &workers, + kickoff_ts, eval.max_timeout, &mut tick, ) @@ -1087,6 +1202,19 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos (done, specs, seat_ids) }; + if eval.canonical_agents { + judges.push(JudgeResult { + name: "canonical team completion".to_string(), + passed: done, + detail: if done { + "post-kickoff completion received".to_string() + } else { + "no post-kickoff completion before max-timeout".to_string() + }, + signal: false, + }); + } + // Snapshot each agent's terminal output to logs/.log so judges can review/assert an agent's // output by log (alongside the run-step + exec sidecar logs already there). Done BEFORE teardown so // the sessions are still peekable. @@ -1296,6 +1424,8 @@ mod tests { #[test] fn canonical_eval_team_uses_exact_catalog_declarations_and_runtime_ids() { let catalog = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(catalog.path().join("sup")).unwrap(); + std::fs::create_dir_all(catalog.path().join("worker")).unwrap(); write_eval_agent( catalog.path(), "agents/evalhost/sup/agent.kdl", @@ -1361,6 +1491,127 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } assert!(error.contains("duplicate") || error.contains("exactly one"), "{error}"); } + #[test] + fn canonical_eval_team_applies_strict_validation_and_main_pty_invariants() { + let cases = [ + ( + "unknown-type", + vec![( + "agents/evalhost/worker/agent.kdl", + r#"agent "worker" { + identity "worker" + host "evalhost" + type "srvice" + argv "true" +}"#, + )], + ), + ( + "unknown-task-kind", + vec![( + "agents/evalhost/worker/agent.kdl", + r#"agent "worker" { + identity "worker" + host "evalhost" + argv "true" + pty { command "true" } +}"#, + )], + ), + ( + "dangling-supervisor", + vec![( + "agents/evalhost/worker/agent.kdl", + r#"agent "worker" { + identity "worker" + host "evalhost" + supervisor "missing" + argv "true" +}"#, + )], + ), + ( + "bad-path", + vec![( + "agents/evalhost/worker/agent.kdl", + r#"agent "worker" { + identity "worker" + host "evalhost" + workspace "$CATALOG/missing" + argv "true" +}"#, + )], + ), + ( + "duplicate main PTY", + vec![ + ( + "agents/evalhost/one/agent.kdl", + r#"agent "one" { + identity "one" + host "evalhost" + pty "agent" { id "shared"; command "sleep 60" } +}"#, + ), + ( + "agents/evalhost/two/agent.kdl", + r#"agent "two" { + identity "two" + host "evalhost" + pty "agent" { id "shared"; command "sleep 60" } +}"#, + ), + ], + ), + ( + "main PTY", + vec![( + "agents/evalhost/worker/agent.kdl", + r#"agent "worker" { + identity "worker" + host "evalhost" + pty "agent" + exec "side-effect" { command "true" } +}"#, + )], + ), + ]; + for (expected, specs) in cases { + let catalog = tempfile::tempdir().unwrap(); + for (path, body) in specs { + write_eval_agent(catalog.path(), path, body); + } + let error = load_canonical_eval_team(catalog.path(), "evalhost") + .unwrap_err() + .to_string(); + assert!( + error.contains(expected), + "expected `{expected}` refusal, got: {error}" + ); + } + + let warning = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(warning.path().join("workspace")).unwrap(); + write_eval_agent( + warning.path(), + "agents/evalhost/worker/agent.kdl", + r#"agent "worker" { + identity "worker" + host "evalhost" + workspace "$CATALOG/workspace" + argv "true" + render { git-exclude ".st2/" } +}"#, + ); + let error = load_canonical_eval_team(warning.path(), "evalhost") + .unwrap_err() + .to_string(); + assert!( + error.contains("materialization") && error.contains("git-exclude"), + "materialization warning was not a pre-spawn failure: {error}" + ); + } + struct RaceRunner { lists: RefCell>>, ops: RefCell> } impl Runner for RaceRunner { fn list_sessions(&self) -> anyhow::Result> { Ok(self.lists.borrow_mut().remove(0)) } @@ -1519,15 +1770,42 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } let workers = vec!["mix.worker".to_string()]; let noop = &mut (|| {}) as &mut dyn FnMut(); // No worker report → never fires (bounded timeout). - assert!(!wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(100), noop)); + assert!(!wait_done( + root, + None, + sup, + req, + &workers, + None, + Duration::from_millis(100), + noop, + )); // A worker→sup report at t=2000 + a sup→requester confirm that PRE-dates it (t=1000) → false // (the early "on it" ack the discriminator exists to reject). seed_msg(&root.join(sup).join("inbox"), 1_700_000_002_000, "aaaaaa", "mix.worker"); seed_msg(&root.join(req).join("inbox"), 1_700_000_001_000, "bbbbbb", "mix.sup"); - assert!(!wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(100), noop)); + assert!(!wait_done( + root, + None, + sup, + req, + &workers, + None, + Duration::from_millis(100), + noop, + )); // A confirm that POST-dates the report (t=3000) → done. seed_msg(&root.join(req).join("inbox"), 1_700_000_003_000, "cccccc", "mix.sup"); - assert!(wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(2000), noop)); + assert!(wait_done( + root, + None, + sup, + req, + &workers, + None, + Duration::from_millis(2000), + noop, + )); } #[test] @@ -1545,7 +1823,16 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } seed_msg(&root.join(sup).join("archive"), 1_700_000_002_000, "aaaaaa", "mix.worker"); seed_msg(&root.join(req).join("inbox"), 1_700_000_003_000, "cccccc", "mix.sup"); assert!( - wait_done(root, "h", false, sup, req, &workers, Duration::from_millis(2000), noop), + wait_done( + root, + None, + sup, + req, + &workers, + None, + Duration::from_millis(2000), + noop, + ), "an archived worker report must still be seen (else archive-on-act hygiene hangs the eval)" ); } @@ -1560,11 +1847,11 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } let mut tick = || ticks.set(ticks.get() + 1); let fired = wait_done( bus.path(), - "h", - false, + None, "sup", "req", &["w".to_string()], + None, Duration::from_millis(400), &mut tick, ); @@ -1572,6 +1859,82 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } assert!(ticks.get() >= 1, "the supervise tick must run during the wait"); } + #[test] + fn canonical_singleton_done_requires_a_confirmation_after_the_exact_kickoff() { + let root = tempfile::tempdir().unwrap(); + let agent_dir = root.path().join("agents/h/interviewer"); + let routes = BTreeMap::from([( + "h.interviewer".to_string(), + CanonicalRoute { + inbox: crate::message::inbox_dir(&agent_dir), + archive: crate::message::archive_dir(&agent_dir), + }, + )]); + let requester = root.path().join("requester/inbox"); + seed_msg(&requester, 1_700_000_001_000, "aaaaaa", "h.interviewer"); + let noop = &mut (|| {}) as &mut dyn FnMut(); + assert!( + !wait_done( + root.path(), + Some(&routes), + "h.interviewer", + "requester", + &[], + Some(1_700_000_002_000), + Duration::from_millis(100), + noop, + ), + "a pre-kickoff acknowledgement must not complete a singleton" + ); + seed_msg(&requester, 1_700_000_003_000, "bbbbbb", "h.interviewer"); + assert!( + wait_done( + root.path(), + Some(&routes), + "h.interviewer", + "requester", + &[], + Some(1_700_000_002_000), + Duration::from_secs(1), + noop, + ), + "a post-kickoff singleton confirmation should complete promptly" + ); + } + + #[test] + fn canonical_boot_report_requires_every_admitted_main_and_propagates_backend_errors() { + let ids = vec!["main-a".to_string(), "main-b".to_string()]; + let clean = UpReport { + launched: ids.clone(), + ..UpReport::default() + }; + require_canonical_boot(&clean, &ids).unwrap(); + + let missing = UpReport { + launched: vec!["main-a".to_string()], + ..UpReport::default() + }; + assert!( + require_canonical_boot(&missing, &ids) + .unwrap_err() + .to_string() + .contains("main-b") + ); + + let backend_error = UpReport { + launched: ids.clone(), + errors: vec!["spawn main-b: backend refused".to_string()], + ..UpReport::default() + }; + assert!( + require_canonical_boot(&backend_error, &ids) + .unwrap_err() + .to_string() + .contains("backend refused") + ); + } + #[test] fn bus_root_expands_st_root_else_defaults() { let s = parse_spec("env { ST_ROOT \"$CATALOG/bus\" }\nagent \"a\" { command \"run\" }").unwrap(); diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index 47b38938..f33524ed 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -215,6 +215,7 @@ fn canonical_agents_run_from_the_hermetic_catalog_with_one_root_and_native_bus() "agents/evalhost/worker", "scripts", "sup", + "templates", "worker", ] { std::fs::create_dir_all(fixture.join(path)).unwrap(); @@ -226,6 +227,7 @@ host "evalhost" eval { copy "./fixture" canonical-agents + supervise message { from "requester"; to "evalhost.sup"; content "do the bounded work" } max-timeout "30s" judges { @@ -233,6 +235,12 @@ eval { judge "one eval-owned root" { exec "test -f $CATALOG/sup/roots-ok && test -f $CATALOG/worker/roots-ok" } + judge "render materialized before launch" { + exec "test \"$(cat $CATALOG/sup/materialized.txt)\" = rendered" + } + judge "custom main id survived supervision" { + exec "test -f $CATALOG/worker/restarted-once" + } judge "kickoff used canonical inbox" { exec "test -d $CATALOG/agents/evalhost/sup/resources/inbox && test ! -e $CATALOG/evalhost.sup/inbox" } @@ -248,7 +256,13 @@ eval { host "evalhost" workspace "$CATALOG/sup" env { ST_AGENT "evalhost.sup" } - argv "sh" "$CATALOG/scripts/sup.sh" + pty "agent" { + id "canonical-sup-main" + argv "sh" "$CATALOG/scripts/sup.sh" + } + render { + copy "templates/proof.txt" "materialized.txt" + } } "#, ) @@ -261,18 +275,28 @@ eval { workspace "$CATALOG/worker" supervisor "sup" env { ST_AGENT "evalhost.worker" } - argv "sh" "$CATALOG/scripts/worker.sh" + pty "agent" { + id "canonical-worker-main" + argv "sh" "$CATALOG/scripts/worker.sh" + } } "#, ) .unwrap(); + std::fs::write(fixture.join("templates/proof.txt"), "rendered\n").unwrap(); std::fs::write( fixture.join("scripts/worker.sh"), r#"#!/bin/sh +echo "canonical worker main" test "$CATALOG" = "$ST_ROOT" && test "$PTY_ROOT" = "$CATALOG/pty" && test "$ST_AGENT" = "evalhost.worker" && : > "$CATALOG/worker/roots-ok" +if [ ! -e "$CATALOG/worker/restarted-once" ]; then + : > "$CATALOG/worker/restarted-once" + sleep 2 + exit 17 +fi : > "$CATALOG/worker/DONE" st2 message send evalhost.sup --root "$ST_ROOT" --as evalhost.worker -m "worker done" >/dev/null 2>&1 exec sleep 60 @@ -282,6 +306,7 @@ exec sleep 60 std::fs::write( fixture.join("scripts/sup.sh"), r#"#!/bin/sh +echo "canonical supervisor main" test "$CATALOG" = "$ST_ROOT" && test "$PTY_ROOT" = "$CATALOG/pty" && test "$ST_AGENT" = "evalhost.sup" && @@ -299,24 +324,52 @@ exec sleep 60 .unwrap(); let path = format!("{}:{}", bin_dir.display(), std::env::var("PATH").unwrap_or_default()); - let out = Command::new(bin) - .args(["eval", "--host", "evalhost"]) + let poison = tmp.path().join("ambient-poison"); + let child = Command::new(bin) + .args(["eval", "--keep", "--host", "evalhost"]) .arg(&cell) .env("PATH", path) - .env_remove("CATALOG") - .env_remove("ST_ROOT") - .env_remove("PTY_ROOT") + .env("CATALOG", poison.join("catalog")) + .env("ST_ROOT", poison.join("bus")) + .env("PTY_ROOT", poison.join("pty")) .env("XDG_STATE_HOME", tmp.path().join("xdg")) - .output() + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() .unwrap(); + let catalog = std::env::temp_dir().join(format!("st2e-{}", child.id())); + let _catalog_cleanup = RemoveDirOnDrop(catalog.clone()); + let out = child.wait_with_output().unwrap(); let stdout = String::from_utf8_lossy(&out.stdout); let stderr = String::from_utf8_lossy(&out.stderr); assert!( out.status.success() && stdout.contains("team signalled done") - && stdout.contains("SCORE: 3 PASS / 0 FAIL"), + && stdout.contains("SCORE: 6 PASS / 0 FAIL"), "canonical eval did not close:\n--stdout--\n{stdout}\n--stderr--\n{stderr}" ); + assert!( + !poison.exists(), + "ambient CATALOG/ST_ROOT/PTY_ROOT leaked outside the eval-owned catalog" + ); + assert_eq!( + std::fs::read_to_string(catalog.join("sup/materialized.txt")).unwrap(), + "rendered\n" + ); + for id in ["canonical-sup-main", "canonical-worker-main"] { + let log = catalog.join("logs").join(format!("{id}.log")); + assert!(log.exists(), "custom main id did not flow into log capture: {}", log.display()); + } + let sessions = Command::new("pty") + .args(["ls", "--json"]) + .env("PTY_ROOT", catalog.join("pty")) + .output() + .unwrap(); + let sessions = String::from_utf8_lossy(&sessions.stdout); + assert!( + !sessions.contains("canonical-sup-main") && !sessions.contains("canonical-worker-main"), + "custom main ids survived teardown: {sessions}" + ); } #[test] @@ -512,6 +565,307 @@ eval { assert!(!catalog.join("SPAWNED").exists(), "seat spawned before kickoff admission"); } +#[test] +fn canonical_agents_freeze_the_admitted_route_across_post_boot_catalog_mutation() { + if !pty_available() { + assert!( + std::env::var_os("ST2_ALLOW_PTY_SKIP").is_some(), + "`pty` not on PATH; set ST2_ALLOW_PTY_SKIP=1" + ); + eprintln!( + "SKIP canonical_agents_freeze_the_admitted_route_across_post_boot_catalog_mutation" + ); + return; + } + let bin = env!("CARGO_BIN_EXE_st2"); + let bin_dir = Path::new(bin).parent().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let cell = tmp.path().join("cell"); + let fixture = cell.join("fixture"); + std::fs::create_dir_all(fixture.join("agents/evalhost/interviewer")).unwrap(); + std::fs::create_dir_all(fixture.join("scripts")).unwrap(); + std::fs::create_dir_all(fixture.join("workspace")).unwrap(); + std::fs::write( + fixture.join("agents/evalhost/interviewer/agent.kdl"), + r#"agent "interviewer" { + identity "interviewer" + host "evalhost" + workspace "$CATALOG/workspace" + argv "sh" "$CATALOG/scripts/interviewer.sh" +} +"#, + ) + .unwrap(); + std::fs::write( + fixture.join("scripts/interviewer.sh"), + r#"#!/bin/sh +rm "$CATALOG/agents/evalhost/interviewer/agent.kdl" +for _ in $(seq 1 100); do + set -- "$CATALOG/agents/evalhost/interviewer/resources/inbox/"*.md + if [ -e "$1" ]; then + sleep 0.02 + mkdir -p "$ST_ROOT/requester/inbox" + timestamp=$(date +%s%3N) + printf '%s\n' '---' 'from: evalhost.interviewer' '---' 'done' \ + > "$ST_ROOT/requester/inbox/$timestamp-aaaaaa.md" + echo "completed through frozen route" + break + fi + sleep 0.05 +done +exec sleep 60 +"#, + ) + .unwrap(); + std::fs::write( + cell.join("cell.kdl"), + r#" +host "evalhost" +eval { + copy "./fixture" + canonical-agents + message { from "requester"; to "evalhost.interviewer"; content "go" } + max-timeout "10s" + judges { + judge "mutation happened after admission" { + exec "test ! -e $CATALOG/agents/evalhost/interviewer/agent.kdl" + } + } +} +"#, + ) + .unwrap(); + let path = format!("{}:{}", bin_dir.display(), std::env::var("PATH").unwrap_or_default()); + let child = Command::new(bin) + .args(["eval", "--keep", "--host", "evalhost"]) + .arg(&cell) + .env("PATH", path) + .env("XDG_STATE_HOME", tmp.path().join("xdg")) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let catalog = std::env::temp_dir().join(format!("st2e-{}", child.id())); + let _catalog_cleanup = RemoveDirOnDrop(catalog.clone()); + let out = child.wait_with_output().unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + let log = + std::fs::read_to_string(catalog.join("logs/evalhost.interviewer.log")).unwrap_or_default(); + assert!( + out.status.success() + && stdout.contains("team signalled done") + && stdout.contains("SCORE: 2 PASS / 0 FAIL"), + "frozen canonical route did not survive declaration removal:\n{stdout}\n{stderr}\n--log--\n{log}" + ); +} + +#[test] +fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { + let bin = env!("CARGO_BIN_EXE_st2"); + let cases: Vec<(&str, &str, Vec<(&str, &str)>)> = vec![ + ( + "unknown-type", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; type "srvice"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + )], + ), + ( + "unknown-task-kind", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60"; pty { command "true" } }"#, + )], + ), + ( + "dangling-supervisor", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; supervisor "missing"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + )], + ), + ( + "bad-path", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; workspace "$CATALOG/missing"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + )], + ), + ( + "main PTY", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; pty "agent"; exec "poison" { command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, + )], + ), + ( + "materialization warnings", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; workspace "$CATALOG/workspace"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60"; render { git-exclude ".st2/" } }"#, + )], + ), + ( + "duplicate main PTY", + "evalhost.one", + vec![ + ( + "one", + r#"agent "one" { identity "one"; host "evalhost"; pty "agent" { id "shared"; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, + ), + ( + "two", + r#"agent "two" { identity "two"; host "evalhost"; pty "agent" { id "shared"; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, + ), + ], + ), + ( + "retired Agent Spec", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; retired #true; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + )], + ), + ( + "belongs to host", + "other.worker", + vec![( + "../other/worker", + r#"agent "worker" { identity "worker"; host "other"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + )], + ), + ( + "override eval-owned `ST_ROOT`", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; pty "agent" { command "touch \"$CATALOG/SPAWNED\"; sleep 60"; env { ST_ROOT "/tmp/poison" } } }"#, + )], + ), + ( + "main PTY id must be nonempty", + "evalhost.worker", + vec![( + "worker", + r#"agent "worker" { identity "worker"; host "evalhost"; pty "agent" { id ""; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, + )], + ), + ]; + for (expected, target, declarations) in cases { + let tmp = tempfile::tempdir().unwrap(); + let cell = tmp.path().join("cell"); + let fixture = cell.join("fixture"); + std::fs::create_dir_all(fixture.join("workspace")).unwrap(); + for (identity, declaration) in declarations { + let dir = fixture.join(format!("agents/evalhost/{identity}")); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("agent.kdl"), declaration).unwrap(); + } + std::fs::write( + cell.join("cell.kdl"), + format!( + r#" +host "evalhost" +eval {{ + copy "./fixture" + canonical-agents + message {{ from "requester"; to "{target}"; content "go" }} + max-timeout "10s" + judges {{ judge "never reached" {{ exec "false" }} }} +}} +"# + ), + ) + .unwrap(); + let child = Command::new(bin) + .args(["eval", "--keep", "--host", "evalhost"]) + .arg(&cell) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let catalog = std::env::temp_dir().join(format!("st2e-{}", child.id())); + let _catalog_cleanup = RemoveDirOnDrop(catalog.clone()); + let out = child.wait_with_output().unwrap(); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!(!out.status.success(), "`{expected}` case launched:\n{combined}"); + assert!( + combined.contains(expected), + "`{expected}` case produced the wrong refusal:\n{combined}" + ); + assert!( + !catalog.join("SPAWNED").exists(), + "`{expected}` case allowed a pre-admission side effect" + ); + } + + let tmp = tempfile::tempdir().unwrap(); + let cell = tmp.path().join("cell"); + let fixture = cell.join("fixture"); + let agent_dir = fixture.join("agents/evalhost/worker"); + std::fs::create_dir_all(&agent_dir).unwrap(); + std::fs::write( + fixture.join("catalog.kdl"), + "catalog { pty_root \"/tmp/poison\" }\n", + ) + .unwrap(); + std::fs::write( + agent_dir.join("agent.kdl"), + r#"agent "worker" { identity "worker"; host "evalhost"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, + ) + .unwrap(); + std::fs::write( + cell.join("cell.kdl"), + r#" +host "evalhost" +eval { + copy "./fixture" + canonical-agents + message { from "requester"; to "evalhost.worker"; content "go" } + max-timeout "10s" + judges { judge "never reached" { exec "false" } } +} +"#, + ) + .unwrap(); + let child = Command::new(bin) + .args(["eval", "--keep", "--host", "evalhost"]) + .arg(&cell) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let catalog = std::env::temp_dir().join(format!("st2e-{}", child.id())); + let _catalog_cleanup = RemoveDirOnDrop(catalog.clone()); + let out = child.wait_with_output().unwrap(); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!(!out.status.success(), "malformed catalog config launched:\n{combined}"); + assert!( + combined.contains("catalog-config") && combined.contains("pty_root"), + "malformed catalog config produced the wrong refusal:\n{combined}" + ); + assert!( + !catalog.join("SPAWNED").exists(), + "malformed catalog config allowed a pre-admission side effect" + ); +} + /// Under `supervise`, teardown reaps RUNTIME-spawned seats too (the team-standup pattern: a seat spins /// up an undeclared peer mid-run), not just the declared team. The seat spawns an undeclared `rtpeer` /// into the eval's hermetic PTY_ROOT; after the eval, no orphan carrying the peer's marker survives. From 0ec6a22e877f9374260202c5f76dabc4579fc7e3 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:20:25 +0200 Subject: [PATCH 3/4] fix(eval): close canonical causal admission agent-session-id: dev3.dotfiles-cos-misc-agent-runtime-simplification agent-tool: Codex agent-tool-version: 0.145.0 agent-model: gpt-5.6-sol agent-runtime-profile: /home/schickling/.config/coding-agents/profile.json agent-skills-manifest: /nix/store/nk9iml2841l1yjjg0f6f0d3y60zkg1nn-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@4a0515f --- README.md | 15 +++--- docs/vrs/spec.md | 16 ++++--- src/eval_run.rs | 109 +++++++++++++++++++++++++++++++++++------- tests/eval_run_e2e.rs | 18 ++++++- 4 files changed, 127 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index c17c184a..78b01fca 100644 --- a/README.md +++ b/README.md @@ -409,13 +409,14 @@ eval { `canonical-agents` then discovers and materializes declarations at `agents///agent.kdl`. It is mutually exclusive with compact `team` / `agent` seats: the discovered vector is the sole authority for launch, kickoff routing, supervision, logs, and -teardown. Strict catalog validation, main-PTY admission, and warning-free materialization all finish -before a seat starts; backend launch errors are fatal. The native inbox/archive paths are frozen from -that admitted vector, so later catalog mutation cannot redirect eval traffic. A multi-seat team -completes only after the existing worker-report ordering; a singleton completes on its first -interviewer-to-requester confirmation that post-dates the exact kickoff receipt. Canonical completion -gates the verdict. Without the directive, Agent Spec-shaped files inside a fixture remain inert and -compact evals retain their flat bus and completion semantics. +teardown. Strict catalog validation, main-PTY admission, fleet-unique nonempty task runtime IDs, and +warning-free materialization all finish before a seat starts; backend launch errors are fatal. The +native inbox/archive paths are frozen from that admitted vector, so later catalog mutation cannot +redirect eval traffic. A multi-seat team completes only after the existing worker-report ordering. +For a singleton, the requester inbox is snapshotted before kickoff; only a newly appearing +interviewer reply at-or-after the exact kickoff receipt completes it. Canonical completion gates the +verdict. Without the directive, Agent Spec-shaped files inside a fixture remain inert and compact +evals retain their flat bus and completion semantics. `st2 compile-agent` remains experimental. Hand-authored KDL is the canonical st2 authoring interface, and generated output must be reviewed before materialization. diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index ff5db00c..35f9a03f 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -29,9 +29,10 @@ and teardown. Admission applies `validate_for_host` strictly, then fails before spawn when discovery is empty, malformed, warning-bearing, duplicate, retired, nonlocal, noncanonical, root-overriding, unrunnable, or does not expose exactly one -independently launchable service main PTY per declaration. Main runtime IDs -must be nonempty and fleet-unique. Materialization warnings and backend launch -errors are fatal. The kickoff target must resolve to exactly one member of the +independently launchable service main PTY per declaration. Every resolved task +runtime ID must be nonempty and fleet-unique, including collisions between a +main PTY and another declaration's sidecar. Materialization warnings and +backend launch errors are fatal. The kickoff target must resolve to exactly one member of the discovered fleet. The eval owns one native `CATALOG` / `ST_ROOT` and its `/pty` registry; declarations cannot override those roots. Workspace renders are materialized before any seat starts. @@ -40,9 +41,12 @@ Native inbox/archive paths are derived once from the admitted Agent Spec paths and carried as frozen data; routing never re-discovers the mutable catalog. The requester alone is an explicit eval-owned flat mailbox. Multi-seat completion retains the worker-report-before-supervisor-confirmation ordering. -A singleton canonical team completes when its interviewer-to-requester -confirmation post-dates the exact kickoff receipt. Canonical completion is a -gating judge, so a timeout cannot pass on unrelated final-state checks alone. +For a singleton, the eval snapshots the requester inbox before kickoff and +completes only for a newly appearing interviewer reply whose timestamp is +at-or-after the exact kickoff receipt. The filename snapshot rejects +future-dated pre-seeded messages while `>=` accepts a causally new same-ms +reply. Canonical completion is a gating judge, so a timeout cannot pass on +unrelated final-state checks alone. Without `canonical-agents`, fixture declarations are not discovered or launched and compact evals retain their catalog-less flat bus. This explicit opt-in keeps diff --git a/src/eval_run.rs b/src/eval_run.rs index 1c3e82a8..02ede20f 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -128,6 +128,12 @@ fn admitted_route<'a>( .unwrap_or_else(|| panic!("strict canonical admission did not freeze route for `{id}`")) } +fn task_runtime_id(spec: &AgentSpec, task: &Task, host: &str) -> String { + task.id + .clone() + .unwrap_or_else(|| format!("{}.{}", spec.bus_id(host), task.name)) +} + fn main_pty_id(spec: &AgentSpec, host: &str) -> Result { let bus_id = spec.bus_id(host); let main = spec @@ -151,10 +157,7 @@ fn main_pty_id(spec: &AgentSpec, host: &str) -> Result { "Agent Spec `{bus_id}` main PTY named `agent` must be an independently launchable service" ); } - let id = main - .id - .clone() - .unwrap_or_else(|| format!("{bus_id}.agent")); + let id = task_runtime_id(spec, main, host); if id.trim().is_empty() { anyhow::bail!("Agent Spec `{bus_id}` main PTY id must be nonempty"); } @@ -211,7 +214,7 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result::new(); let mut bus_ids = HashSet::new(); - let mut main_ids = HashSet::new(); + let mut runtime_ids = BTreeMap::::new(); let mut seat_ids = Vec::new(); let mut routes = BTreeMap::new(); for spec in &found.specs { @@ -254,8 +257,27 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result, id: &str) -> bool { /// Wait for the DONE signal, message-driven (not grade-poll). Multi-seat teams require a /// `sup → requester` confirmation whose timestamp follows a `worker → sup` report. A canonical -/// singleton instead requires its confirmation to post-date the exact kickoff receipt. Compact -/// singleton semantics remain unchanged. Bounded by `timeout`. Returns whether done fired. +/// singleton instead requires a causally new requester-inbox entry at-or-after the exact kickoff +/// receipt. Compact singleton semantics remain unchanged. Bounded by `timeout`. Returns whether done +/// fired. fn wait_done( bus: &Path, canonical_routes: Option<&BTreeMap>, @@ -528,6 +551,7 @@ fn wait_done( requester: &str, workers: &[String], kickoff_ts: Option, + requester_before_kickoff: Option<&HashSet>, timeout: Duration, on_tick: &mut dyn FnMut(), ) -> bool { @@ -559,11 +583,16 @@ fn wait_done( let sup_archived = crate::message::list_dir(&sup_archive).unwrap_or_default(); if workers.is_empty() && let Some(kickoff_ts) = kickoff_ts + && let Some(before) = requester_before_kickoff { let confirmed = crate::message::list_dir(&req_inbox) .unwrap_or_default() .iter() - .any(|m| from_is(m.from.as_deref(), sup) && m.ts_ms > kickoff_ts); + .any(|m| { + !before.contains(&m.filename) + && from_is(m.from.as_deref(), sup) + && m.ts_ms >= kickoff_ts + }); if confirmed { return true; } @@ -1074,6 +1103,13 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos Some(routes) => admitted_route(routes, &sup).inbox.clone(), None => bus.join(&sup).join("inbox"), }; + let requester_before_kickoff = eval.canonical_agents.then(|| { + crate::message::list_dir(&bus.join(&msg.from).join("inbox")) + .unwrap_or_default() + .into_iter() + .map(|message| message.filename) + .collect::>() + }); let kickoff_receipt = crate::message::send_to_inbox(&to_inbox, &msg.from, None, None, &[], &body) .with_context(|| format!("seeding kickoff into {}", to_inbox.display()))?; @@ -1185,6 +1221,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos &msg.from, &workers, kickoff_ts, + requester_before_kickoff.as_ref(), eval.max_timeout, &mut tick, ) @@ -1559,6 +1596,28 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } identity "two" host "evalhost" pty "agent" { id "shared"; command "sleep 60" } +}"#, + ), + ], + ), + ( + "duplicate runtime task id", + vec![ + ( + "agents/evalhost/one/agent.kdl", + r#"agent "one" { + identity "one" + host "evalhost" + pty "agent" { id "shared"; command "sleep 60" } +}"#, + ), + ( + "agents/evalhost/two/agent.kdl", + r#"agent "two" { + identity "two" + host "evalhost" + pty "agent" { id "two-main"; command "sleep 60" } + exec "poison" { id "shared"; command "true" } }"#, ), ], @@ -1777,6 +1836,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } req, &workers, None, + None, Duration::from_millis(100), noop, )); @@ -1791,6 +1851,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } req, &workers, None, + None, Duration::from_millis(100), noop, )); @@ -1803,6 +1864,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } req, &workers, None, + None, Duration::from_millis(2000), noop, )); @@ -1830,6 +1892,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } req, &workers, None, + None, Duration::from_millis(2000), noop, ), @@ -1852,6 +1915,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } "req", &["w".to_string()], None, + None, Duration::from_millis(400), &mut tick, ); @@ -1860,7 +1924,7 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } } #[test] - fn canonical_singleton_done_requires_a_confirmation_after_the_exact_kickoff() { + fn canonical_singleton_done_is_new_after_kickoff_and_allows_the_same_millisecond() { let root = tempfile::tempdir().unwrap(); let agent_dir = root.path().join("agents/h/interviewer"); let routes = BTreeMap::from([( @@ -1871,7 +1935,15 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } }, )]); let requester = root.path().join("requester/inbox"); - seed_msg(&requester, 1_700_000_001_000, "aaaaaa", "h.interviewer"); + let kickoff_ts = 1_700_000_002_000; + // An already-present message may even claim a FUTURE timestamp. Causality comes from the + // pre-kickoff filename snapshot, not wall-clock trust. + seed_msg(&requester, kickoff_ts + 1_000, "aaaaaa", "h.interviewer"); + let before = crate::message::list_dir(&requester) + .unwrap() + .into_iter() + .map(|message| message.filename) + .collect::>(); let noop = &mut (|| {}) as &mut dyn FnMut(); assert!( !wait_done( @@ -1880,13 +1952,15 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } "h.interviewer", "requester", &[], - Some(1_700_000_002_000), + Some(kickoff_ts), + Some(&before), Duration::from_millis(100), noop, ), - "a pre-kickoff acknowledgement must not complete a singleton" + "a future-dated pre-kickoff acknowledgement must not complete a singleton" ); - seed_msg(&requester, 1_700_000_003_000, "bbbbbb", "h.interviewer"); + // Filename novelty establishes after-kickoff causality; `>=` keeps a valid same-ms reply. + seed_msg(&requester, kickoff_ts, "bbbbbb", "h.interviewer"); assert!( wait_done( root.path(), @@ -1894,11 +1968,12 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } "h.interviewer", "requester", &[], - Some(1_700_000_002_000), + Some(kickoff_ts), + Some(&before), Duration::from_secs(1), noop, ), - "a post-kickoff singleton confirmation should complete promptly" + "a newly appearing same-ms singleton confirmation should complete promptly" ); } diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index f33524ed..3b7211f6 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -236,7 +236,7 @@ eval { exec "test -f $CATALOG/sup/roots-ok && test -f $CATALOG/worker/roots-ok" } judge "render materialized before launch" { - exec "test \"$(cat $CATALOG/sup/materialized.txt)\" = rendered" + exec "test -f $CATALOG/sup/render-seen-at-process-start" } judge "custom main id survived supervision" { exec "test -f $CATALOG/worker/restarted-once" @@ -306,6 +306,8 @@ exec sleep 60 std::fs::write( fixture.join("scripts/sup.sh"), r#"#!/bin/sh +test "$(cat "$CATALOG/sup/materialized.txt")" = rendered || exit 41 +: > "$CATALOG/sup/render-seen-at-process-start" echo "canonical supervisor main" test "$CATALOG" = "$ST_ROOT" && test "$PTY_ROOT" = "$CATALOG/pty" && @@ -726,6 +728,20 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { ), ], ), + ( + "duplicate runtime task id", + "evalhost.one", + vec![ + ( + "one", + r#"agent "one" { identity "one"; host "evalhost"; pty "agent" { id "shared"; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, + ), + ( + "two", + r#"agent "two" { identity "two"; host "evalhost"; pty "agent" { id "two-main"; command "sleep 60" }; exec "poison" { id "shared"; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, + ), + ], + ), ( "retired Agent Spec", "evalhost.worker", From 45210b29c1f874c189af8cae822d723a94f832d1 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 21:49:37 +0200 Subject: [PATCH 4/4] fix: align canonical eval agents with agent contract --- README.md | 16 +- docs/vrs/spec.md | 35 ++-- src/eval_run.rs | 431 +++++++++++++++++++++++------------------- src/eval_spec.rs | 4 +- tests/eval_run_e2e.rs | 116 +++++++----- 5 files changed, 333 insertions(+), 269 deletions(-) diff --git a/README.md b/README.md index 78b01fca..aa2c63d1 100644 --- a/README.md +++ b/README.md @@ -406,13 +406,15 @@ eval { ``` `copy` and deterministic `run` steps populate the hermetic temporary catalog first. -`canonical-agents` then discovers and materializes declarations at -`agents///agent.kdl`. It is mutually exclusive with compact `team` / `agent` seats: -the discovered vector is the sole authority for launch, kickoff routing, supervision, logs, and -teardown. Strict catalog validation, main-PTY admission, fleet-unique nonempty task runtime IDs, and -warning-free materialization all finish before a seat starts; backend launch errors are fatal. The -native inbox/archive paths are frozen from that admitted vector, so later catalog mutation cannot -redirect eval traffic. A multi-seat team completes only after the existing worker-report ordering. +`canonical-agents` then recursively discovers the catalog and projects declarations resolved to the +eval host. Explicit `identity` and `host` fields remain authoritative independent of organizational +placement; each declaration parent remains its native state/resource anchor. The directive is +mutually exclusive with compact `team` / `agent` declarations, so the local Agent Spec vector is the +sole authority for launch, kickoff routing, supervision, logs, and teardown. Strict catalog +validation, fleet-unique nonempty task runtime IDs, and warning-free local materialization all finish +before an agent task starts; backend launch errors are fatal. Remote-host declarations remain inert. +Native inbox/archive paths are frozen from the admitted local vector, so later catalog mutation +cannot redirect eval traffic. A multi-agent team completes only after the existing worker-report ordering. For a singleton, the requester inbox is snapshotted before kickoff; only a newly appearing interviewer reply at-or-after the exact kickoff receipt completes it. Canonical completion gates the verdict. Without the directive, Agent Spec-shaped files inside a fixture remain inert and compact diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index 35f9a03f..50498cda 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -20,26 +20,27 @@ canonical in An eval may opt into `canonical-agents` after its fixture copy and deterministic run steps have populated the hermetic temporary catalog. The directive is -mutually exclusive with compact `team` / `agent` seats. st2 discovers and -materializes only declarations at -`agents///agent.kdl`, then carries that one Agent Spec vector -unchanged through launch admission, kickoff resolution, supervision, logging, -and teardown. +mutually exclusive with compact `team` / `agent` declarations. st2 recursively +discovers the catalog, preserves explicit `identity` and `host` authority +independent of organizational placement, and projects only declarations +resolved to the eval host. Each declaration parent remains its native +state/resource anchor. The resulting local Agent Spec vector flows unchanged +through launch admission, kickoff resolution, supervision, logging, and +declaration-driven teardown; remote-host declarations remain inert. Admission applies `validate_for_host` strictly, then fails before spawn when -discovery is empty, malformed, warning-bearing, duplicate, retired, nonlocal, -noncanonical, root-overriding, unrunnable, or does not expose exactly one -independently launchable service main PTY per declaration. Every resolved task -runtime ID must be nonempty and fleet-unique, including collisions between a -main PTY and another declaration's sidecar. Materialization warnings and -backend launch errors are fatal. The kickoff target must resolve to exactly one member of the -discovered fleet. The eval owns one native `CATALOG` / `ST_ROOT` and its -`/pty` registry; declarations cannot override those roots. Workspace -renders are materialized before any seat starts. +discovery is malformed or warning-bearing, when the selected local projection +is empty, duplicate, retired, root-overriding, or unrunnable, or when any +resolved local task runtime ID is empty or duplicates another local task. +Materialization warnings and backend launch errors are fatal. The kickoff +target must resolve to exactly one local agent. The eval owns one native +`CATALOG` / `ST_ROOT` and its `/pty` registry; declarations cannot +override those roots. Local workspace renders are materialized before any +agent task starts. Native inbox/archive paths are derived once from the admitted Agent Spec paths and carried as frozen data; routing never re-discovers the mutable catalog. -The requester alone is an explicit eval-owned flat mailbox. Multi-seat +The requester alone is an explicit eval-owned flat mailbox. Multi-agent completion retains the worker-report-before-supervisor-confirmation ordering. For a singleton, the eval snapshots the requester inbox before kickoff and completes only for a newly appearing interviewer reply whose timestamp is @@ -52,11 +53,11 @@ Without `canonical-agents`, fixture declarations are not discovered or launched and compact evals retain their catalog-less flat bus. This explicit opt-in keeps ordinary fixtures inert while allowing the same canonical declaration to be exercised in an eval and real work. Parser and admission evidence lives in -`eval_spec::tests::canonical_agents_is_bare_once_and_excludes_compact_seats` and +`eval_spec::tests::canonical_agents_is_bare_once_and_excludes_compact_agents` and the `canonical_*` unit tests. Named-PTY end-to-end cases prove strict pre-spawn refusals, poisoned ambient-root isolation, real render materialization, frozen routing after declaration removal, singleton -completion, custom main-ID supervision/logging/teardown, and the no-opt-in +completion, custom task-ID supervision/logging/teardown, and the no-opt-in legacy control in `tests/eval_run_e2e.rs`. ## Resource bindings (R20-R21) diff --git a/src/eval_run.rs b/src/eval_run.rs index 02ede20f..bd18f915 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -109,10 +109,17 @@ pub fn spec_to_agent_specs(agents: &[SpecAgent], host: &str, root: &Path) -> Vec #[derive(Debug)] struct CanonicalEvalTeam { specs: Vec, - seat_ids: Vec, + runtime_tasks: Vec, routes: BTreeMap, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct EvalRuntimeTask { + agent_id: String, + runtime_id: String, + is_pty: bool, +} + #[derive(Debug, Clone)] struct CanonicalRoute { inbox: PathBuf, @@ -134,34 +141,8 @@ fn task_runtime_id(spec: &AgentSpec, task: &Task, host: &str) -> String { .unwrap_or_else(|| format!("{}.{}", spec.bus_id(host), task.name)) } -fn main_pty_id(spec: &AgentSpec, host: &str) -> Result { - let bus_id = spec.bus_id(host); - let main = spec - .tasks - .iter() - .filter(|task| task.kind == TaskKind::Pty && task.name == "agent") - .collect::>(); - let [main] = main.as_slice() else { - anyhow::bail!( - "Agent Spec `{bus_id}` must declare exactly one main PTY named `agent`, found {}", - main.len() - ); - }; - let launchable = match (&main.command, &main.argv) { - (Some(command), None) => !command.trim().is_empty(), - (None, Some(argv)) => argv.first().is_some_and(|program| !program.trim().is_empty()), - _ => false, - }; - if main.lifecycle != TaskLifecycle::Service || !launchable { - anyhow::bail!( - "Agent Spec `{bus_id}` main PTY named `agent` must be an independently launchable service" - ); - } - let id = task_runtime_id(spec, main, host); - if id.trim().is_empty() { - anyhow::bail!("Agent Spec `{bus_id}` main PTY id must be nonempty"); - } - Ok(id) +fn task_is_launchable(task: &Task) -> bool { + task.command.is_some() || task.argv.is_some() } /// Discover the sole declaration authority for a `canonical-agents` eval after its fixture and run @@ -202,9 +183,6 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result Result::new(); + let local_specs = found + .specs + .iter() + .filter(|spec| spec.resolved_host(host) == host) + .cloned() + .collect::>(); + if local_specs.is_empty() { + anyhow::bail!( + "canonical-agents found no local canonical Agent Specs for host `{host}` in {}", + catalog.display() + ); + } + let mut bus_ids = HashSet::new(); - let mut runtime_ids = BTreeMap::::new(); - let mut seat_ids = Vec::new(); + let mut runtime_ids = BTreeMap::::new(); + let mut runtime_tasks = Vec::new(); let mut routes = BTreeMap::new(); - for spec in &found.specs { - *paths.entry(spec.path.clone()).or_default() += 1; + for spec in &local_specs { let bus_id = spec.bus_id(host); if !bus_ids.insert(bus_id.clone()) { anyhow::bail!("canonical-agents found duplicate Agent Spec bus identity `{bus_id}`"); @@ -226,24 +215,6 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result Result Result Result Result, id: &str) -> bool { from.is_some_and(|f| f == id || f.ends_with(&format!(".{id}"))) } -/// Wait for the DONE signal, message-driven (not grade-poll). Multi-seat teams require a +/// Wait for the DONE signal, message-driven (not grade-poll). Multi-agent teams require a /// `sup → requester` confirmation whose timestamp follows a `worker → sup` report. A canonical /// singleton instead requires a causally new requester-inbox entry at-or-after the exact kickoff /// receipt. Compact singleton semantics remain unchanged. Bounded by `timeout`. Returns whether done @@ -573,7 +535,7 @@ fn wait_done( if EVAL_INTERRUPTED.load(Ordering::SeqCst) { return false; } - // Earliest worker→sup report (a message from a worker seat). Scan the sup's inbox AND archive: + // Earliest worker→sup report (a message from a worker agent). Scan inbox AND archive: // DING-BUS mandates "archive a message the moment you act on it", so a well-behaved sup MOVES the // report inbox→archive the instant it acts. Scanning inbox-only makes the done-signal a race // against the sup's archiving — a fully-closed loop hangs to max-timeout because the report left @@ -616,20 +578,20 @@ fn wait_done( if Instant::now() > deadline { return false; } - // A per-tick hook: under `supervise`, this respawns any dead team seat FROM SPEC (full env) so a - // fault-injected restart/crash recovers mid-run. A no-op for a boot-once (unsupervised) eval. + // A per-tick hook: under `supervise`, this respawns any dead team task FROM SPEC (full env) so + // a fault-injected restart/crash recovers mid-run. A no-op for boot-once (unsupervised). on_tick(); std::thread::sleep(Duration::from_millis(300)); } } -/// Fail-fast boot gate: a seat whose command exits immediately (127 harness-not-on-PATH, or a crash at +/// Fail-fast boot gate: a task whose command exits immediately (127 harness-not-on-PATH, or a crash at /// startup) must fail the eval LOUDLY now, not leave it hanging until `max-timeout` waiting for a -/// confirmation that can never come. Poll briefly for all seats to be live — a real seat is up within +/// confirmation that can never come. Poll briefly for all tasks to be live — a real task is up within /// ~1s; a dead-at-boot one never is (tolerant of a slow start + a transient pty-list flicker). -fn boot_gate(seat_ids: &[String], specs: &[AgentSpec], host: &str, catalog: &Path) -> Result<()> { +fn boot_gate(task_ids: &[String], specs: &[AgentSpec], host: &str, catalog: &Path) -> Result<()> { let runner = SystemRunner::new(catalog.to_path_buf(), catalog.join("exec")); - let want: Vec<&str> = seat_ids.iter().map(String::as_str).collect(); + let want: Vec<&str> = task_ids.iter().map(String::as_str).collect(); let deadline = Instant::now() + Duration::from_secs(5); loop { let sessions = runner.list_sessions().unwrap_or_default(); @@ -639,9 +601,9 @@ fn boot_gate(seat_ids: &[String], specs: &[AgentSpec], host: &str, catalog: &Pat return Ok(()); } if Instant::now() > deadline { - teardown_team(specs, host, catalog, false); // boot failure → no runtime seats yet + teardown_team(specs, host, catalog, false); // boot failure → no runtime tasks yet anyhow::bail!( - "seat(s) {dead:?} exited at boot — the command didn't stay running (harness not on PATH, \ + "task(s) {dead:?} exited at boot — the command didn't stay running (harness not on PATH, \ e.g. claude/codex not installed, or a crash at startup). Failing fast instead of hanging \ until the eval's max-timeout." ); @@ -650,8 +612,8 @@ fn boot_gate(seat_ids: &[String], specs: &[AgentSpec], host: &str, catalog: &Pat } } -fn require_canonical_boot(report: &UpReport, seat_ids: &[String]) -> Result<()> { - let missing = seat_ids +fn require_canonical_boot(report: &UpReport, task_ids: &[String]) -> Result<()> { + let missing = task_ids .iter() .filter(|id| !report.launched.contains(id)) .cloned() @@ -664,7 +626,7 @@ fn require_canonical_boot(report: &UpReport, seat_ids: &[String]) -> Result<()> || !missing.is_empty() { anyhow::bail!( - "canonical Agent Spec boot did not launch every admitted main PTY: missing={missing:?}; \ + "canonical Agent Spec boot did not launch every admitted task: missing={missing:?}; \ skipped={}; held={:?}; unrunnable={:?}; flapping={:?}; errors={:?}", report.skipped, report.held, @@ -684,14 +646,19 @@ fn message_timestamp(filename: &str) -> Result { } /// Tear down the team (nomad-safe): mark the specs retired and reconcile → the runner kills the live -/// sessions (process-group kill). Best-effort — an eval always tears down, no zombie seats. +/// sessions (process-group kill). Best-effort — an eval always tears down, with no zombie tasks. /// /// `reap_all` (set under `supervise`): after the declared teardown, ALSO reap every remaining session -/// in the eval's hermetic PTY_ROOT — the RUNTIME-spawned seats that are NOT in the spec (e.g. a -/// team-standup specialist the CoS spun up mid-run). `teardown_team` alone only reaps DECLARED seats, so -/// a runtime seat would leak as an orphan; since the PTY_ROOT is hermetic to this eval, anything still +/// in the eval's hermetic PTY_ROOT — runtime-spawned tasks that are NOT in the spec (e.g. a +/// team-standup specialist the CoS spun up mid-run). Declared teardown only reaps declared tasks, so +/// a runtime task would leak as an orphan; since the PTY_ROOT is hermetic to this eval, anything still /// alive is ours to clean. Killing an already-dead declared session is a harmless no-op. -fn teardown_team(specs: &[AgentSpec], host: &str, root: &Path, reap_all: bool) { +fn teardown_team_with_runner( + specs: &[AgentSpec], + host: &str, + runner: &dyn Runner, + reap_all: bool, +) { let retired: Vec = specs .iter() .cloned() @@ -700,12 +667,11 @@ fn teardown_team(specs: &[AgentSpec], host: &str, root: &Path, reap_all: bool) { s }) .collect(); - let runner = SystemRunner::new(root.to_path_buf(), root.join("exec")); if let Ok(sessions) = runner.list_sessions() { let plan = reconcile(&retired, &sessions, host); let mut report = UpReport::default(); let mut cap = FlappingCap::default(); - execute(&plan, &runner, &mut cap, &mut report); + execute(&plan, runner, &mut cap, &mut report); } if reap_all && let Ok(remaining) = runner.list_sessions() @@ -717,6 +683,11 @@ fn teardown_team(specs: &[AgentSpec], host: &str, root: &Path, reap_all: bool) { } } +fn teardown_team(specs: &[AgentSpec], host: &str, root: &Path, reap_all: bool) { + let runner = SystemRunner::new(root.to_path_buf(), root.join("exec")); + teardown_team_with_runner(specs, host, &runner, reap_all); +} + /// `st2 eval ` — run the eval end to end: mint a hermetic temp catalog, copy the fixture /// (`_git`→`.git`), boot the base team + eval-only agents, pretrust their workspaces, deliver the /// kickoff, wait for the sup's confirmation (post-dating a worker report) or `max-timeout`, tear down. @@ -912,27 +883,27 @@ fn run_steps( (results, judge_env) } -/// Snapshot each agent's terminal output to `/logs/.log` (plain-text full scrollback via +/// Snapshot each PTY task's terminal output to `/logs/.log` (plain-text full scrollback via /// `pty peek`), so judges can review/assert an agent's output by log. Best-effort: `pty` has no /// continuous plain-text log, so this is the scrollback captured at judge time — enough to inspect a /// wedged/finished agent's history. A truly continuous agent log would need a `pty` feature. -fn dump_agent_logs(seat_ids: &[String], catalog: &Path) { - if seat_ids.is_empty() { +fn dump_agent_logs(pty_task_ids: &[String], catalog: &Path) { + if pty_task_ids.is_empty() { return; } let logs_dir = catalog.join("logs"); let _ = std::fs::create_dir_all(&logs_dir); let pty_root = crate::run::effective_pty_root(catalog); - for seat_id in seat_ids { + for task_id in pty_task_ids { let out = std::process::Command::new("pty") - .args(["peek", "--full", "--plain", seat_id]) + .args(["peek", "--full", "--plain", task_id]) .env("PTY_ROOT", &pty_root) .output(); if let Ok(o) = out && o.status.success() && !o.stdout.is_empty() { - let _ = std::fs::write(logs_dir.join(format!("{seat_id}.log")), &o.stdout); + let _ = std::fs::write(logs_dir.join(format!("{task_id}.log")), &o.stdout); } } } @@ -942,11 +913,11 @@ fn env_key(id: &str) -> String { id.chars().map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }).collect() } -/// The supervisor chain of `seat_id`, walked transitively via each agent's `supervisor` field to the +/// The supervisor chain of `agent_id`, walked transitively via each agent's `supervisor` field to the /// root (whose supervisor is `None` — the cos). Returns the ancestor ids, nearest first. A cycle or a /// supervisor that names no declared agent terminates the walk (the named id is still included — we ding -/// its inbox regardless of whether it is a booted seat). -fn supervisor_chain(seat_id: &str, specs: &[AgentSpec], host: &str) -> Vec { +/// its inbox regardless of whether it has a running task). +fn supervisor_chain(agent_id: &str, specs: &[AgentSpec], host: &str) -> Vec { let mut chain = Vec::new(); let mut seen = std::collections::HashSet::new(); let find = |identity: &str| { @@ -954,7 +925,7 @@ fn supervisor_chain(seat_id: &str, specs: &[AgentSpec], host: &str) -> Vec Vec` bus message to EVERY ancestor in its -/// supervisor chain (the direct supervisor up to the cos root) — so the whole supervision chain learns -/// the worker died without polling. A seat with no supervisor has nothing to notify. +/// Emit a crash-ding for a crashed task to every ancestor in its owning agent's supervisor chain. fn crash_ding( - seat_id: &str, + agent_id: &str, + task_id: &str, specs: &[AgentSpec], bus: &Path, host: &str, canonical_routes: Option<&BTreeMap>, ) { - let chain = supervisor_chain(seat_id, specs, host); + let chain = supervisor_chain(agent_id, specs, host); if chain.is_empty() { return; } - let subject = format!("worker crash: {seat_id}"); + let subject = format!("worker crash: {task_id}"); let body = format!( - "Worker '{seat_id}' crashed — its pty session died non-cleanly (non-zero exit / killed / vanished). \ + "Agent task '{task_id}' crashed — its session died non-cleanly (non-zero exit / killed / vanished). \ st2 respawned it from spec; surfacing the crash up the supervision chain." ); for ancestor in &chain { @@ -990,7 +960,7 @@ fn crash_ding( None => bus.join(ancestor).join("inbox"), }; let _ = crate::message::send_to_inbox(&inbox, "st2", Some(&subject), None, &[], &body); - eval_log!("== crash-ding: {seat_id} → {ancestor} =="); + eval_log!("== crash-ding: {task_id} → {ancestor} =="); } } @@ -1014,14 +984,14 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos let mut compact_agents = spec.agents.clone(); compact_agents.extend(eval.agents.clone()); - let (done, specs, seat_ids) = if compact_agents.is_empty() && !eval.canonical_agents { + let (done, specs, pty_task_ids) = if compact_agents.is_empty() && !eval.canonical_agents { // TEAM-LESS: nothing to boot, kick off, or wait on — the run steps did the work → straight to judging. if !eval.run_steps.is_empty() { eval_log!("== team-less eval: {} run step(s) ran → judging ==", eval.run_steps.len()); } (true, Vec::new(), Vec::new()) } else { - let (mut specs, seat_ids, participant_ids, canonical_routes) = if eval.canonical_agents { + let (mut specs, runtime_tasks, participant_ids, canonical_routes) = if eval.canonical_agents { if bus != catalog { anyhow::bail!( "canonical-agents requires the native flat ST_ROOT `{}`, got `{}`", @@ -1035,24 +1005,42 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos .iter() .map(|spec| spec.bus_id(host)) .collect::>(); - (team.specs, team.seat_ids, participants, Some(team.routes)) + ( + team.specs, + team.runtime_tasks, + participants, + Some(team.routes), + ) } else { let specs = spec_to_agent_specs(&compact_agents, host, catalog); - let seats = compact_agents + let runtime_tasks = compact_agents .iter() - .map(|agent| agent.id.clone()) + .map(|agent| EvalRuntimeTask { + agent_id: agent.id.clone(), + runtime_id: agent.id.clone(), + is_pty: true, + }) .collect::>(); let participants = compact_agents .iter() .map(|agent| agent.id.clone()) .collect::>(); - (specs, seats, participants, None) + (specs, runtime_tasks, participants, None) }; + let task_ids = runtime_tasks + .iter() + .map(|task| task.runtime_id.clone()) + .collect::>(); + let pty_task_ids = runtime_tasks + .iter() + .filter(|task| task.is_pty) + .map(|task| task.runtime_id.clone()) + .collect::>(); if eval.supervise && !eval.canonical_agents { add_eval_exit_markers(&mut specs, catalog); } if !eval.canonical_agents { - // Compact legacy seats intentionally retain their historical ambient trust behavior. + // Compact legacy agents intentionally retain their historical ambient trust behavior. // Canonical managed Agent Specs own trust inside their declared adapter trajectory. let dirs: Vec = compact_agents .iter() @@ -1066,7 +1054,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos let canonical_sup = if eval.canonical_agents { let msg = eval.message.as_ref().ok_or_else(|| { anyhow::anyhow!( - "a canonical-agents eval needs a message{{}} kickoff before any seat can launch" + "a canonical-agents eval needs a message{{}} kickoff before any agent can launch" ) })?; let matches = specs @@ -1089,11 +1077,11 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos eval_log!("== boot team ({} agents) ==", specs.len()); let boot = boot_team(&specs, host, catalog)?; if eval.canonical_agents { - require_canonical_boot(&boot, &seat_ids)?; + require_canonical_boot(&boot, &task_ids)?; } - boot_gate(&seat_ids, &specs, host, catalog)?; + boot_gate(&task_ids, &specs, host, catalog)?; - // Deliver the kickoff onto the bus the seats' dings watch (ST_ROOT), from the requester. + // Deliver the kickoff onto the bus the agents' DING tasks watch (ST_ROOT), from the requester. let msg = eval.message.as_ref().ok_or_else(|| { anyhow::anyhow!("a team eval needs a message{{}} kickoff (only a team-less eval may omit it)") })?; @@ -1128,7 +1116,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos msg.from, eval.max_timeout ); - // `supervise`: each wait tick, respawn any dead seat FROM SPEC (full env → rejoins cold). Carry a + // `supervise`: each wait tick, respawn any dead task FROM SPEC (full env → rejoins cold). Carry a // FlappingCap + LivenessDebounce ACROSS ticks so a fault-injected kill is respawned exactly ONCE // (cap rate-limits; debounce absorbs a transient `pty list` misread). Scoped so the tick's borrow // of `specs` ends before we hand `specs` to teardown. @@ -1136,17 +1124,17 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos let supervise_runner = SystemRunner::new(catalog.to_path_buf(), catalog.join("exec")); let mut sup_cap = crate::flapping::FlappingCap::default(); let mut sup_debounce = crate::run::LivenessDebounce::new(Duration::from_secs(2)); - // Crash-ding state: the boot gate immediately above proved every declared seat alive, so + // Crash-ding state: the boot gate immediately above proved every declared task alive, so // carry that proof into supervision. PTY may self-reap a fast exit before the first - // post-kickoff list snapshot; starting empty would then misclassify the proven-live seat + // post-kickoff list snapshot; starting empty would then misclassify the proven-live task // as never booted and suppress its crash ding. `dinged` dedups so one crash = one ding - // until the seat is alive again. + // until the task is alive again. let mut ever_alive: std::collections::HashSet = - seat_ids.iter().cloned().collect(); + task_ids.iter().cloned().collect(); let mut dinged: std::collections::HashSet = std::collections::HashSet::new(); let mut tick = || { if eval.supervise { - // Detect crashes BEFORE respawn (reconcile reaps the dead session): a declared seat + // Detect crashes BEFORE respawn (reconcile reaps the dead session): a declared task // that was alive and is now dead non-cleanly (non-zero/killed/vanished) → crash-ding // its supervisor chain. A clean exit (code 0) stays SILENT (a false ding on a routine // finish is as bad as a missed crash). @@ -1156,10 +1144,8 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos &str, &crate::reconcile::Session, > = sessions.iter().map(|s| (s.pty_id.as_str(), s)).collect(); - for seat in &specs { - let id = main_pty_id(seat, host) - .expect("eval team main PTY was validated"); - let id = id.as_str(); + for task in &runtime_tasks { + let id = task.runtime_id.as_str(); match by_id.get(id) { Some(s) if s.alive => { ever_alive.insert(id.to_string()); @@ -1173,13 +1159,9 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos && !clean && !dinged.contains(id) { - let ding_id = if eval.canonical_agents { - seat.bus_id(host) - } else { - seat.identity.clone() - }; crash_ding( - &ding_id, + &task.agent_id, + id, &specs, &bus, host, @@ -1236,7 +1218,7 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos } else { eval_log!("== max-timeout: no confirmation within {:?} — judging the final state ==", eval.max_timeout); } - (done, specs, seat_ids) + (done, specs, pty_task_ids) }; if eval.canonical_agents { @@ -1255,12 +1237,12 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos // Snapshot each agent's terminal output to logs/.log so judges can review/assert an agent's // output by log (alongside the run-step + exec sidecar logs already there). Done BEFORE teardown so // the sessions are still peekable. - dump_agent_logs(&seat_ids, catalog); + dump_agent_logs(&pty_task_ids, catalog); // Judges: the run-step gate results first, then the declared judges (all must pass). Judge BEFORE // teardown — an ask-agent judge needs its judge agent still alive to answer. judges.extend(run_judges(&eval.judges, spec_dir, catalog, &bus, &requester, &run_env)); - // Under `supervise`, reap runtime-spawned seats too (team-standup) — not just the declared team. + // Under `supervise`, reap runtime-spawned tasks too (team-standup), not just the declared team. teardown_team(&specs, host, catalog, eval.supervise); Ok(EvalReport { done, judges, timeout: eval.max_timeout }) } @@ -1488,31 +1470,115 @@ mod tests { let team = load_canonical_eval_team(catalog.path(), "evalhost").unwrap(); assert_eq!(team.specs.len(), 2); - assert_eq!(team.seat_ids, ["evalhost.sup", "evalhost.worker"]); + assert_eq!( + team.runtime_tasks + .iter() + .map(|task| task.runtime_id.as_str()) + .collect::>(), + ["evalhost.sup", "evalhost.worker"] + ); assert!(team.specs.iter().all(|spec| spec.path.ends_with("agent.kdl"))); } #[test] - fn canonical_eval_team_fails_closed_before_launch_on_zero_duplicate_or_noncanonical_specs() { + fn canonical_eval_team_projects_local_path_independent_agents_and_tears_down_every_task() { + let catalog = tempfile::tempdir().unwrap(); + write_eval_agent( + catalog.path(), + "organization/.managed/arbitrary/declaration/agent.kdl", + r#"agent "local" { + identity "local" + host "evalhost" + pty "work" { id "local-work"; command "sleep 60" } + exec "watch" { id "local-watch"; command "sleep 60" } +}"#, + ); + write_eval_agent( + catalog.path(), + "fleet/remote/declaration/agent.kdl", + r#"agent "remote" { + identity "remote" + host "other" + pty "remote-work" { id "remote-work"; command "sleep 60" } +}"#, + ); + + let team = load_canonical_eval_team(catalog.path(), "evalhost").unwrap(); + assert_eq!( + team.specs + .iter() + .map(|spec| spec.bus_id("evalhost")) + .collect::>(), + ["evalhost.local"] + ); + assert_eq!( + team.runtime_tasks, + [ + EvalRuntimeTask { + agent_id: "evalhost.local".into(), + runtime_id: "local-watch".into(), + is_pty: false, + }, + EvalRuntimeTask { + agent_id: "evalhost.local".into(), + runtime_id: "local-work".into(), + is_pty: true, + }, + ] + ); + assert_eq!( + admitted_route(&team.routes, "evalhost.local").inbox, + catalog + .path() + .join("organization/.managed/arbitrary/declaration/resources/inbox") + ); + + struct RecordingRunner { + sessions: Vec, + killed: RefCell>, + } + impl Runner for RecordingRunner { + fn list_sessions(&self) -> anyhow::Result> { + Ok(self.sessions.clone()) + } + fn spawn(&self, _: &TaskTarget, _: &Path) -> anyhow::Result<()> { + unreachable!("teardown must not spawn") + } + fn kill(&self, id: &str) -> anyhow::Result<()> { + self.killed.borrow_mut().push(id.to_string()); + Ok(()) + } + fn remove(&self, _: &str) -> anyhow::Result<()> { + Ok(()) + } + } + let runner = RecordingRunner { + sessions: ["local-work", "local-watch", "remote-work"] + .into_iter() + .map(|id| Session { + pty_id: id.into(), + alive: true, + exit_code: None, + }) + .collect(), + killed: RefCell::new(Vec::new()), + }; + teardown_team_with_runner(&team.specs, "evalhost", &runner, false); + let mut killed = runner.killed.into_inner(); + killed.sort(); + assert_eq!(killed, ["local-watch", "local-work"]); + } + + #[test] + fn canonical_eval_team_fails_closed_before_launch_on_zero_or_duplicate_specs() { let empty = tempfile::tempdir().unwrap(); assert!( load_canonical_eval_team(empty.path(), "evalhost") .unwrap_err() .to_string() - .contains("no canonical Agent Specs") + .contains("no local canonical Agent Specs") ); - let misplaced = tempfile::tempdir().unwrap(); - write_eval_agent( - misplaced.path(), - "fixture/agent.kdl", - r#"agent "worker" { identity "worker"; host "evalhost"; argv "true" }"#, - ); - let error = load_canonical_eval_team(misplaced.path(), "evalhost") - .unwrap_err() - .to_string(); - assert!(error.contains("canonical path"), "{error}"); - let duplicate = tempfile::tempdir().unwrap(); write_eval_agent( duplicate.path(), @@ -1525,11 +1591,11 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } let error = load_canonical_eval_team(duplicate.path(), "evalhost") .unwrap_err() .to_string(); - assert!(error.contains("duplicate") || error.contains("exactly one"), "{error}"); + assert!(error.contains("duplicate"), "{error}"); } #[test] - fn canonical_eval_team_applies_strict_validation_and_main_pty_invariants() { + fn canonical_eval_team_applies_strict_validation_and_task_invariants() { let cases = [ ( "unknown-type", @@ -1579,27 +1645,6 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } }"#, )], ), - ( - "duplicate main PTY", - vec![ - ( - "agents/evalhost/one/agent.kdl", - r#"agent "one" { - identity "one" - host "evalhost" - pty "agent" { id "shared"; command "sleep 60" } -}"#, - ), - ( - "agents/evalhost/two/agent.kdl", - r#"agent "two" { - identity "two" - host "evalhost" - pty "agent" { id "shared"; command "sleep 60" } -}"#, - ), - ], - ), ( "duplicate runtime task id", vec![ @@ -1622,18 +1667,6 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } ), ], ), - ( - "main PTY", - vec![( - "agents/evalhost/worker/agent.kdl", - r#"agent "worker" { - identity "worker" - host "evalhost" - pty "agent" - exec "side-effect" { command "true" } -}"#, - )], - ), ]; for (expected, specs) in cases { let catalog = tempfile::tempdir().unwrap(); @@ -1978,8 +2011,8 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } } #[test] - fn canonical_boot_report_requires_every_admitted_main_and_propagates_backend_errors() { - let ids = vec!["main-a".to_string(), "main-b".to_string()]; + fn canonical_boot_report_requires_every_admitted_task_and_propagates_backend_errors() { + let ids = vec!["task-a".to_string(), "task-b".to_string()]; let clean = UpReport { launched: ids.clone(), ..UpReport::default() @@ -1987,19 +2020,19 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" } require_canonical_boot(&clean, &ids).unwrap(); let missing = UpReport { - launched: vec!["main-a".to_string()], + launched: vec!["task-a".to_string()], ..UpReport::default() }; assert!( require_canonical_boot(&missing, &ids) .unwrap_err() .to_string() - .contains("main-b") + .contains("task-b") ); let backend_error = UpReport { launched: ids.clone(), - errors: vec!["spawn main-b: backend refused".to_string()], + errors: vec!["spawn task-b: backend refused".to_string()], ..UpReport::default() }; assert!( diff --git a/src/eval_spec.rs b/src/eval_spec.rs index c9b0bf42..3873075b 100644 --- a/src/eval_spec.rs +++ b/src/eval_spec.rs @@ -349,7 +349,7 @@ pub fn parse_spec(text: &str) -> anyhow::Result { && (!agents.is_empty() || eval.as_ref().is_some_and(|eval| !eval.agents.is_empty())) { anyhow::bail!( - "eval `canonical-agents` is mutually exclusive with compact `team` / `agent` seats" + "eval `canonical-agents` is mutually exclusive with compact `team` / `agent` declarations" ); } Ok(Spec { host, env: top_env, agents, eval }) @@ -859,7 +859,7 @@ team "mix" { } #[test] - fn canonical_agents_is_bare_once_and_excludes_compact_seats() { + fn canonical_agents_is_bare_once_and_excludes_compact_agents() { let canonical = parse_spec( "eval {\n canonical-agents\n message { from \"r\"; to \"h.sup\"; content \"go\" }\n max-timeout \"5s\"\n}\n", ) diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index 3b7211f6..04bc0681 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -454,19 +454,65 @@ eval { } #[test] -fn canonical_agents_reject_noncanonical_declarations_before_spawn() { +fn canonical_agents_accept_path_independent_local_tasks_and_ignore_remote_projection() { + if !pty_available() { + assert!( + std::env::var_os("ST2_ALLOW_PTY_SKIP").is_some(), + "`pty` not on PATH; set ST2_ALLOW_PTY_SKIP=1" + ); + eprintln!( + "SKIP canonical_agents_accept_path_independent_local_tasks_and_ignore_remote_projection" + ); + return; + } let bin = env!("CARGO_BIN_EXE_st2"); + let bin_dir = Path::new(bin).parent().unwrap(); let tmp = tempfile::tempdir().unwrap(); let cell = tmp.path().join("cell"); let fixture = cell.join("fixture"); - std::fs::create_dir_all(fixture.join("misplaced")).unwrap(); + let local_dir = fixture.join("organization/.managed/arbitrary/declaration"); + let remote_dir = fixture.join("fleet/remote/declaration"); + std::fs::create_dir_all(&local_dir).unwrap(); + std::fs::create_dir_all(&remote_dir).unwrap(); + std::fs::create_dir_all(fixture.join("scripts")).unwrap(); std::fs::write( - fixture.join("misplaced/agent.kdl"), - r#"agent "bad" { - identity "bad" + local_dir.join("agent.kdl"), + r#"agent "local" { + identity "local" host "evalhost" - argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" + pty "work" { + id "custom-local-task" + argv "sh" "$CATALOG/scripts/local.sh" + } +} +"#, + ) + .unwrap(); + std::fs::write( + remote_dir.join("agent.kdl"), + r#"agent "remote" { + identity "remote" + host "other" + pty "work" { + id "remote-task" + command "touch \"$CATALOG/REMOTE-SPAWNED\"; sleep 60" + } } +"#, + ) + .unwrap(); + std::fs::write( + fixture.join("scripts/local.sh"), + r#"#!/bin/sh +for _ in $(seq 1 100); do + set -- "$CATALOG/organization/.managed/arbitrary/declaration/resources/inbox/"*.md + if [ -e "$1" ]; then + st2 message send requester --root "$ST_ROOT" --as evalhost.local -m "done" >/dev/null 2>&1 + exec sleep 60 + fi + sleep 0.05 +done +exit 42 "#, ) .unwrap(); @@ -477,16 +523,22 @@ host "evalhost" eval { copy "./fixture" canonical-agents - message { from "requester"; to "evalhost.bad"; content "go" } + message { from "requester"; to "evalhost.local"; content "go" } max-timeout "10s" - judges { judge "never reached" { exec "false" } } + judges { + judge "local projection only" { + exec "test -d $CATALOG/organization/.managed/arbitrary/declaration/resources/inbox && test ! -e $CATALOG/REMOTE-SPAWNED" + } + } } "#, ) .unwrap(); + let path = format!("{}:{}", bin_dir.display(), std::env::var("PATH").unwrap_or_default()); let child = Command::new(bin) .args(["eval", "--keep", "--host", "evalhost"]) .arg(&cell) + .env("PATH", path) .env_remove("CATALOG") .env_remove("ST_ROOT") .env_remove("PTY_ROOT") @@ -498,14 +550,15 @@ eval { let catalog = std::env::temp_dir().join(format!("st2e-{}", child.id())); let _catalog_cleanup = RemoveDirOnDrop(catalog.clone()); let out = child.wait_with_output().unwrap(); - let combined = format!( - "{}{}", - String::from_utf8_lossy(&out.stdout), - String::from_utf8_lossy(&out.stderr) + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success() + && stdout.contains("team signalled done") + && stdout.contains("SCORE: 2 PASS / 0 FAIL"), + "path-independent local projection failed:\n{stdout}\n{stderr}" ); - assert!(!out.status.success(), "noncanonical declaration was accepted:\n{combined}"); - assert!(combined.contains("canonical path"), "wrong refusal:\n{combined}"); - assert!(!catalog.join("SPAWNED").exists(), "seat spawned before canonical admission"); + assert!(catalog.join("logs/custom-local-task.log").is_file()); } #[test] @@ -564,7 +617,7 @@ eval { && combined.contains("found 0"), "wrong refusal:\n{combined}" ); - assert!(!catalog.join("SPAWNED").exists(), "seat spawned before kickoff admission"); + assert!(!catalog.join("SPAWNED").exists(), "task spawned before kickoff admission"); } #[test] @@ -606,10 +659,7 @@ for _ in $(seq 1 100); do set -- "$CATALOG/agents/evalhost/interviewer/resources/inbox/"*.md if [ -e "$1" ]; then sleep 0.02 - mkdir -p "$ST_ROOT/requester/inbox" - timestamp=$(date +%s%3N) - printf '%s\n' '---' 'from: evalhost.interviewer' '---' 'done' \ - > "$ST_ROOT/requester/inbox/$timestamp-aaaaaa.md" + st2 message send requester --root "$ST_ROOT" --as evalhost.interviewer -m "done" >/dev/null 2>&1 echo "completed through frozen route" break fi @@ -698,14 +748,6 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { r#"agent "worker" { identity "worker"; host "evalhost"; workspace "$CATALOG/missing"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, )], ), - ( - "main PTY", - "evalhost.worker", - vec![( - "worker", - r#"agent "worker" { identity "worker"; host "evalhost"; pty "agent"; exec "poison" { command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, - )], - ), ( "materialization warnings", "evalhost.worker", @@ -714,20 +756,6 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { r#"agent "worker" { identity "worker"; host "evalhost"; workspace "$CATALOG/workspace"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60"; render { git-exclude ".st2/" } }"#, )], ), - ( - "duplicate main PTY", - "evalhost.one", - vec![ - ( - "one", - r#"agent "one" { identity "one"; host "evalhost"; pty "agent" { id "shared"; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, - ), - ( - "two", - r#"agent "two" { identity "two"; host "evalhost"; pty "agent" { id "shared"; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, - ), - ], - ), ( "duplicate runtime task id", "evalhost.one", @@ -751,7 +779,7 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { )], ), ( - "belongs to host", + "no local canonical Agent Specs", "other.worker", vec![( "../other/worker", @@ -767,7 +795,7 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { )], ), ( - "main PTY id must be nonempty", + "runtime task id must be nonempty", "evalhost.worker", vec![( "worker",