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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/vrs/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ validate ──► materialize ──► host-local st2 scheduler/reconciler
ordinary declaration space; only `.git` and `.st2` directories at any depth,
the catalog root's `pty` child, and a declaration parent's `resources`,
`archive`, and `inbox` children are excluded.
A resolved workspace-relative render destination has one coherent desired
state across the active local fleet: byte-equivalent idempotent claims may
share it, while incompatible
claims fail every conflicting owner before the first workspace write.
Targeted reconciliation checks the selected owner against the full fleet, so
selection cannot bypass this ownership boundary.
- **R04:** Each machine schedules and reconciles only its pinned work. The st2
loop is deterministic; exactly one declared root agent provides intelligent
host-local supervision, bounded recovery, and escalation. Filesystem reads
Expand Down
8 changes: 7 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,7 @@ fn up(

if materialize_only {
let mut found = discover(&catalog_root);
let ownership_specs = found.specs.clone();
if let Some(selector) = task.as_deref() {
let (owner, _, _) = st2::reconcile::resolve_task(&found.specs, selector, &this_host)?;
let owner_identity = owner.identity.clone();
Expand All @@ -1809,7 +1810,12 @@ fn up(
"verifying explicitly installed lifecycle hooks before Codex materialization",
)?;
}
let report = st2::materialize::materialize_catalog(&catalog_root, &found.specs, &this_host);
let report = st2::materialize::materialize_catalog_against(
&catalog_root,
&found.specs,
&ownership_specs,
&this_host,
);
for item in &report.materialized {
println!("{item}");
}
Expand Down
174 changes: 172 additions & 2 deletions src/materialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! The runner stays harness-agnostic: these are generic, ordered file operations. Content operations
//! gate an agent's boot; `git-exclude` is advisory and can never prevent a launch.

use std::collections::{BTreeMap, HashSet};
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::fs;
use std::path::{Component, Path, PathBuf};
use std::process::Command;
Expand Down Expand Up @@ -452,6 +452,143 @@ enum PreparedOp {
},
}

#[derive(Debug, Clone, PartialEq)]
enum RenderClaim {
Replace(Vec<u8>),
JsonUpsert(serde_json::Value),
EnsureLine(String),
}

#[derive(Debug)]
pub struct RenderOwnershipConflict {
pub destination: PathBuf,
pub owners: BTreeSet<String>,
}

impl RenderOwnershipConflict {
fn error(&self) -> String {
format!(
"conflicting render ownership for '{}': active agents {} declare incompatible content for one shared workspace target",
self.destination.display(),
self.owners.iter().cloned().collect::<Vec<_>>().join(", ")
)
}
}

fn claims_for_agent(
root: &Path,
spec: &AgentSpec,
this_host: &str,
) -> Result<BTreeMap<PathBuf, Vec<RenderClaim>>> {
let plan = parse_plan(spec)?;
if plan.ops.is_empty() {
return Ok(BTreeMap::new());
}
let workspace_raw = spec
.workspace
.as_deref()
.with_context(|| format!("agent '{}' has render{{}} but no workspace", spec.identity))?;
let env = render_env(root, spec, this_host);
let workspace = PathBuf::from(expand(workspace_raw, &env));
let workspace = workspace
.canonicalize()
.with_context(|| format!("canonicalizing workspace {}", workspace.display()))?;
let spec_dir = spec.path.parent().unwrap_or(root);
let mut claims = BTreeMap::<PathBuf, Vec<RenderClaim>>::new();
for op in plan.ops {
let (destination, claim) = match op {
RenderOp::Copy {
source: raw_source,
destination: raw_destination,
} => {
let source = source(root, spec_dir, &raw_source, &env)?;
let bytes = fs::read(&source)
.with_context(|| format!("reading copy source {}", source.display()))?;
(
destination(&workspace, &raw_destination, &env)?,
RenderClaim::Replace(bytes),
)
}
RenderOp::File {
destination: raw_destination,
content,
} => (
destination(&workspace, &raw_destination, &env)?,
RenderClaim::Replace(expand(&content, &env).into_bytes()),
),
RenderOp::JsonUpsert {
destination: raw_destination,
content,
} => {
let patch = serde_json::from_str(&expand(&content, &env)).with_context(|| {
format!(
"expanded json-upsert for '{}' is not valid JSON",
spec.identity
)
})?;
(
destination(&workspace, &raw_destination, &env)?,
RenderClaim::JsonUpsert(patch),
)
}
RenderOp::EnsureLine {
destination: raw_destination,
line,
} => (
destination(&workspace, &raw_destination, &env)?,
RenderClaim::EnsureLine(expand(&line, &env)),
),
// Every git-exclude is additive by contract and resolves through Git's own shared
// metadata path rather than a declared workspace-relative render destination.
RenderOp::GitExclude { .. } => continue,
};
claims.entry(destination).or_default().push(claim);
}
Ok(claims)
}

/// Find active local agents that declare incompatible desired content for one resolved workspace
/// destination. Equivalent idempotent plans may share a target; differing plans have no implicit
/// last-writer-wins semantics.
pub fn render_ownership_conflicts(
root: &Path,
specs: &[AgentSpec],
this_host: &str,
) -> Vec<RenderOwnershipConflict> {
let mut by_destination = BTreeMap::<PathBuf, BTreeMap<String, Vec<RenderClaim>>>::new();
for spec in specs {
if spec.retired || spec.resolved_host(this_host) != this_host {
continue;
}
let Ok(claims) = claims_for_agent(root, spec, this_host) else {
// The normal per-agent materialization path reports malformed plans and unavailable
// inputs. Ownership analysis only compares claims it can resolve without writing.
continue;
};
let owner = spec.bus_id(this_host);
for (destination, plan) in claims {
by_destination
.entry(destination)
.or_default()
.insert(owner.clone(), plan);
}
}

by_destination
.into_iter()
.filter_map(|(destination, claims)| {
let mut plans = claims.values();
let first = plans.next()?;
plans
.any(|plan| plan != first)
.then(|| RenderOwnershipConflict {
destination,
owners: claims.into_keys().collect(),
})
})
.collect()
}

/// Execute one agent's render plan in declaration order.
pub fn materialize_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Result<Vec<String>> {
let plan = parse_plan(spec)?;
Expand Down Expand Up @@ -699,12 +836,45 @@ pub fn validate_agent(root: &Path, spec: &AgentSpec, this_host: &str) -> Result<

/// Materialize every active agent assigned to `this_host`.
pub fn materialize_catalog(root: &Path, specs: &[AgentSpec], this_host: &str) -> MaterializeReport {
materialize_catalog_against(root, specs, specs, this_host)
}

/// Materialize `selected_specs` after checking their workspace target ownership against the complete
/// active fleet. This preserves shortest-path selection while preventing a selected owner from
/// bypassing a collision declared by an unselected sibling.
pub fn materialize_catalog_against(
root: &Path,
selected_specs: &[AgentSpec],
ownership_specs: &[AgentSpec],
this_host: &str,
) -> MaterializeReport {
let mut report = MaterializeReport::default();
for spec in specs {
let selected_ids = selected_specs
.iter()
.map(|spec| spec.bus_id(this_host))
.collect::<HashSet<_>>();
for conflict in render_ownership_conflicts(root, ownership_specs, this_host) {
let affected = conflict
.owners
.iter()
.filter(|owner| selected_ids.contains(*owner))
.cloned()
.collect::<Vec<_>>();
if affected.is_empty() {
continue;
}
report.failed_agents.extend(affected);
report.errors.push(conflict.error());
}

for spec in selected_specs {
if spec.retired || spec.resolved_host(this_host) != this_host {
continue;
}
let bus_id = spec.bus_id(this_host);
if report.failed_agents.contains(&bus_id) {
continue;
}
match materialize_agent(root, spec, this_host) {
Ok(notes) => {
for note in notes {
Expand Down
17 changes: 12 additions & 5 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,10 +803,16 @@ fn reconcile_pass(
.cloned()
.collect::<Vec<_>>();

// Ordered, idempotent pre-boot materialization. A gating render failure removes only that agent
// from this pass; advisory git-exclude failures remain warnings and never block a launch.
let materialized =
crate::materialize::materialize_catalog(root, &materializable_specs, this_host);
// Ordered, idempotent pre-boot materialization, with ownership checked against the complete
// active fleet even when another gate defers one owner's writes. A gating render failure removes
// only that agent from this pass; advisory git-exclude failures remain warnings and never block
// a launch.
let materialized = crate::materialize::materialize_catalog_against(
root,
&materializable_specs,
&found.specs,
this_host,
);
report.warnings.extend(materialized.warnings);
report.errors.extend(materialized.errors);
let eligible_specs: Vec<_> = found
Expand Down Expand Up @@ -998,9 +1004,10 @@ pub fn up_once_selected(
.push(format!("verify lifecycle hooks: {error}"));
return Ok(report);
}
let materialized = crate::materialize::materialize_catalog(
let materialized = crate::materialize::materialize_catalog_against(
catalog_root,
std::slice::from_ref(&owner),
&found.specs,
this_host,
);
report.warnings.extend(materialized.warnings);
Expand Down
15 changes: 15 additions & 0 deletions src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,21 @@ fn validate_scoped(root: &Path, this_host: Option<&str>) -> Report {
}
}

if let Some(host) = this_host {
for conflict in crate::materialize::render_ownership_conflicts(root, &d.specs, host) {
issues.push(Issue::error(
"render-owner-conflict",
".".to_string(),
None,
format!(
"conflicting render ownership for '{}': active agents {} declare incompatible content for one shared workspace target",
conflict.destination.display(),
conflict.owners.iter().cloned().collect::<Vec<_>>().join(", ")
),
));
}
}

Report {
issues,
agents: d.specs.len(),
Expand Down
66 changes: 66 additions & 0 deletions tests/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,3 +547,69 @@ fn missing_hooks_do_not_rewrite_or_stop_an_already_live_codex_agent() {
);
assert!(!hooks_root.exists());
}

#[test]
fn missing_hooks_cannot_hide_a_shared_workspace_render_conflict() {
let tmp = tempfile::tempdir().unwrap();
let catalog = tmp.path().join("catalog");
let workspace = tmp.path().join("shared-workspace");
let hooks_root = tmp.path().join("hooks");
let bin = tmp.path().join("bin");
let pty_log = tmp.path().join("pty.log");
fs::create_dir_all(&workspace).unwrap();
fs::create_dir_all(&bin).unwrap();
let declaration = |identity: &str, command: &str, content: &str| {
format!(
"agent \"{identity}\" {{\n host \"h\"\n workspace \"{}\"\n \
env {{ ST_AGENT \"h.{identity}\" }}\n command \"{command}\"\n \
render {{ file \"shared\" \"{content}\" }}\n}}\n",
workspace.display()
)
};
let codex = catalog.join("agents/h/codex/agent.kdl");
let sibling = catalog.join("agents/h/sibling/agent.kdl");
fs::create_dir_all(codex.parent().unwrap()).unwrap();
fs::create_dir_all(sibling.parent().unwrap()).unwrap();
fs::write(&codex, declaration("codex", "exec codex", "codex")).unwrap();
fs::write(&sibling, declaration("sibling", "exec sibling", "sibling")).unwrap();
write_executable(
&bin.join("pty"),
&format!(
"#!/bin/sh\nprintf '%s\\n' \"$*\" >> '{}'\nif [ \"$1\" = list ]; then printf '[]\\n'; fi\n",
pty_log.display()
),
);

let output = command(&hooks_root)
.arg("up")
.arg(&catalog)
.args(["--host", "h", "--once"])
.env("PATH", &bin)
.output()
.unwrap();
let report = format!(
"{}{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);

assert!(output.status.success(), "{report}");
assert!(report.contains("materialization deferred"), "{report}");
assert!(report.contains("conflicting render ownership"), "{report}");
assert!(
!workspace.join("shared").exists(),
"a hook-gated owner must remain visible to fleet-wide conflict preflight"
);
assert_eq!(
fs::read_to_string(&pty_log)
.unwrap_or_default()
.lines()
.collect::<Vec<_>>(),
["list --json"],
"neither conflicting owner may launch"
);
assert!(
!hooks_root.exists(),
"ordinary up must not create or rewrite the hook root"
);
}
Loading
Loading