Skip to content

Commit dd1e72b

Browse files
feat(resync): notify a live agent when a declared resource carrier changes (#345)
* feat(agent-spec): accept catalog-relative resource carrier URIs A resource binding uri may now be a scheme-less relative path, resolved by the consumer against the declaration directory. Absolute URIs keep the existing validation. This is the carrier form declaratively managed goal files need (docs/vrs/06-resync); it is an st2 extension pending canonical Agent Spec adoption. agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@a80129b-dirty * feat(resync): notify a live agent when a declared resource carrier changes Implements #341. The supervisor watches each running agent's local resource carriers (file:// and catalog-relative bindings plus its own declaration), classifies them (immediate / silent / coalesced), and emits digest-keyed, superseded events through a built-in reserved `resync` stream on the unchanged event-stream ingress. Equal-byte rewrites never wake; bursts coalesce per class window. Design decisions: docs/vrs/.decisions/0008; subsystem VRS: docs/vrs/06-resync. agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@a80129b-dirty * fix(resync): preserve in-flight carrier state across watch-set refreshes A reconcile pass can land between a carrier mutation and its flush window (agent.kdl changes wake reconcile immediately). Reseeding digests on every watch-set application silently erased that pending event; carriers already on record now keep their digest and dirty state, and only genuinely unknown paths seed silently. agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@a80129b-dirty * docs(vrs): record resync refresh-race discovery in the composition experiment agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@a80129b-dirty * fix(resync): address Codex review findings - run: restore unpark report merging and the crash-loop dedup reset in the supervisor loop, lost when the resync wiring landed - resync: derive the event identity from the whole transition (previous digest -> new digest), so an A->B->A oscillation re-notifies on the rollback leg instead of colliding with B's original identity - resync: resolve bus ids with the supervisor's logical host, so hostless declarations supervised under `st2 up --host <alias>` produce recipients the ingress can actually resolve - resync: retain every subscriber of a shared carrier path; several agents binding one file each get their own event - resync: carriers whose parent directory cannot be registered fall back to digest polling at refresh cadence instead of being watched by nobody agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@a80129b-dirty * test(codex): wait for parsable pidfile content in process-group cleanup test The shell's > redirection creates an empty pidfile before printf writes, so polling is_file() can observe an empty file under CI load and panic with ParseIntError::Empty. Poll for parseable content instead. agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@a80129b-dirty * fix(resync): preserve subscriber event identity * docs(resync): align event identity contract * fix(resync): retry publication and tighten carriers * fix(resync): close watch and carrier gaps * fix(resync): watch only proven live seats * fix(resync): preserve live watch state atomically * fix(resync): replay exact transitions with bounded hashing * fix(resync): preserve fallback lifecycle semantics * fix: normalize and refresh resync subscriptions * fix(resync): preserve transitions across metadata refresh * fix(resync): namespace transition occurrences * fix: retain resync occurrence state across suspension * fix(resync): stabilize subscriptions across relocation * fix: restrict resync event ingress to supervisor * fix: seed resync watches at live-seat transitions * fix: isolate generated task compile failures * fix resync carrier validation and live watches * fix(resync): close review lifecycle gaps --------- Co-authored-by: Johannes Schickling <schickling.j@gmail.com>
1 parent 2167ef3 commit dd1e72b

16 files changed

Lines changed: 4268 additions & 52 deletions

crates/agent-spec/src/spec.rs

Lines changed: 128 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,21 @@ struct ResourceDescriptor {
256256
impl Resource {
257257
/// Construct a descriptor after enforcing the same invariants as catalog parsing.
258258
pub fn new(name: String, uri: String, reason: String) -> Result<Self, String> {
259-
if name.is_empty() {
260-
return Err("resource binding name cannot be empty".into());
259+
if name.is_empty()
260+
|| name.len() > 200
261+
|| name.trim() != name
262+
|| name.chars().any(char::is_control)
263+
{
264+
return Err(
265+
"resource binding name must be 1..=200 bytes without surrounding whitespace or controls"
266+
.into(),
267+
);
268+
}
269+
if name == "declaration" {
270+
return Err("resource binding name 'declaration' is reserved by resync".into());
261271
}
262-
validate_absolute_uri(&uri).map_err(|reason| {
263-
format!("resource binding '{name}' `uri` must be an exact absolute URI: {reason}")
272+
validate_resource_uri(&uri).map_err(|reason| {
273+
format!("resource binding '{name}' `uri` must be an exact absolute URI or a catalog-relative path: {reason}")
264274
})?;
265275
validate_resource_explanation(&name, "reason", &reason)?;
266276
Ok(Self {
@@ -585,6 +595,10 @@ fn validate_stream_name(identity: &str, name: &str) -> anyhow::Result<()> {
585595
&& !name.ends_with('-'),
586596
"agent '{identity}' stream name '{name}' must match [a-z0-9]([a-z0-9-]*[a-z0-9])?"
587597
);
598+
anyhow::ensure!(
599+
name != "resync",
600+
"agent '{identity}' stream name '{name}' is reserved for built-in resync events"
601+
);
588602
Ok(())
589603
}
590604

@@ -764,6 +778,87 @@ fn validate_resource_explanation(name: &str, field: &str, value: &str) -> Result
764778
Ok(())
765779
}
766780

781+
/// A resource URI is either an exact absolute URI (any scheme) or a catalog-relative path with no
782+
/// scheme at all, resolved by the consumer against the declaration directory. Relative carriers
783+
/// are an st2 extension pending canonical Agent Spec adoption (see 06-resync).
784+
fn validate_resource_uri(uri: &str) -> Result<(), &'static str> {
785+
if let Some(colon) = uri.find(':') {
786+
let first_separator = uri.find(|character| matches!(character, '/' | '\\'));
787+
if first_separator.is_none_or(|separator| colon < separator) {
788+
return validate_absolute_uri(uri);
789+
}
790+
}
791+
if uri.is_empty() {
792+
return Err("catalog-relative uri must be a non-empty relative path");
793+
}
794+
let decoded = decode_percent_path(uri)?;
795+
if decoded.starts_with('/') || decoded.split('/').any(|part| part == "..") {
796+
return Err(
797+
"catalog-relative uri must be a relative path without parent (`..`) components",
798+
);
799+
}
800+
Ok(())
801+
}
802+
803+
/// Decode filesystem-path percent escapes without allowing an escape to change path segmentation.
804+
///
805+
/// Consumers may apply additional policy to the decoded path (for example, catalog-relative
806+
/// resources reject every parent component). This shared boundary rejects bytes that cannot name
807+
/// the same UTF-8 filesystem path the resource URI visibly denotes.
808+
pub fn decode_percent_path(encoded_path: &str) -> Result<String, &'static str> {
809+
let bytes = encoded_path.as_bytes();
810+
let mut decoded = Vec::with_capacity(bytes.len());
811+
let mut component_start = 0;
812+
let mut component_had_escape = false;
813+
let mut offset = 0;
814+
while offset < bytes.len() {
815+
if bytes[offset] == b'/' {
816+
if component_had_escape && decoded[component_start..] == *b".." {
817+
return Err("path contains an encoded parent (`..`) component");
818+
}
819+
decoded.push(b'/');
820+
component_start = decoded.len();
821+
component_had_escape = false;
822+
offset += 1;
823+
continue;
824+
}
825+
if bytes[offset] != b'%' {
826+
decoded.push(bytes[offset]);
827+
offset += 1;
828+
continue;
829+
}
830+
let Some(high) = bytes.get(offset + 1).and_then(|byte| hex_digit(*byte)) else {
831+
return Err("path contains an invalid percent escape");
832+
};
833+
let Some(low) = bytes.get(offset + 2).and_then(|byte| hex_digit(*byte)) else {
834+
return Err("path contains an invalid percent escape");
835+
};
836+
let byte = (high << 4) | low;
837+
if matches!(byte, b'/' | b'\\') {
838+
return Err("path contains an encoded separator");
839+
}
840+
if byte == b'\0' {
841+
return Err("path decodes to NUL");
842+
}
843+
decoded.push(byte);
844+
component_had_escape = true;
845+
offset += 3;
846+
}
847+
if component_had_escape && decoded[component_start..] == *b".." {
848+
return Err("path contains an encoded parent (`..`) component");
849+
}
850+
String::from_utf8(decoded).map_err(|_| "percent-decoded path is not valid UTF-8")
851+
}
852+
853+
fn hex_digit(byte: u8) -> Option<u8> {
854+
match byte {
855+
b'0'..=b'9' => Some(byte - b'0'),
856+
b'a'..=b'f' => Some(byte - b'a' + 10),
857+
b'A'..=b'F' => Some(byte - b'A' + 10),
858+
_ => None,
859+
}
860+
}
861+
767862
fn validate_absolute_uri(uri: &str) -> Result<(), &'static str> {
768863
let Some(colon) = uri.find(':') else {
769864
return Err("missing scheme");
@@ -1360,6 +1455,35 @@ mod tests {
13601455
assert!(parse_duration("").is_err());
13611456
}
13621457

1458+
#[test]
1459+
fn catalog_relative_uris_share_filesystem_safe_percent_decoding() {
1460+
for uri in [
1461+
"report..md",
1462+
"reports/.../goal.md",
1463+
"reports/%2Ereport.md",
1464+
"reports/child..name/goal.md",
1465+
"reports/with%20space/%E2%82%AC.md",
1466+
] {
1467+
assert_eq!(validate_resource_uri(uri), Ok(()), "{uri}");
1468+
}
1469+
1470+
for uri in [
1471+
"..",
1472+
"../goal.md",
1473+
"reports/../goal.md",
1474+
"%2e%2e/goal.md",
1475+
"reports/%2E%2e/goal.md",
1476+
"reports%2f..%2fgoal.md",
1477+
"reports/encoded%2Fseparator",
1478+
"reports/encoded%5cseparator",
1479+
"reports/a%00b",
1480+
"reports/%FF.md",
1481+
"reports/bad%escape",
1482+
] {
1483+
assert!(validate_resource_uri(uri).is_err(), "{uri}");
1484+
}
1485+
}
1486+
13631487
#[test]
13641488
fn absolute_uri_syntax_accepts_rfc3986_characters_without_normalizing() {
13651489
for uri in [

crates/agent-spec/tests/discovery.rs

Lines changed: 166 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,30 @@ fn resource_explanation_byte_bounds_are_enforced() {
11341134
);
11351135
}
11361136

1137+
#[test]
1138+
fn resource_binding_names_and_scheme_candidates_fail_loudly() {
1139+
for name in [
1140+
" declaration".to_owned(),
1141+
"declaration".to_owned(),
1142+
"line\nbreak".to_owned(),
1143+
"x".repeat(201),
1144+
] {
1145+
assert!(
1146+
Resource::new(name.clone(), "issue://one".into(), "Task.".into()).is_err(),
1147+
"invalid binding name was accepted: {name:?}"
1148+
);
1149+
}
1150+
assert!(
1151+
Resource::new(
1152+
"work".into(),
1153+
"_github://org/repo".into(),
1154+
"Task.".into(),
1155+
)
1156+
.is_err(),
1157+
"a malformed scheme prefix must not become a catalog-relative path"
1158+
);
1159+
}
1160+
11371161
#[test]
11381162
fn malformed_resource_envelopes_are_rejected_without_defining_downstream_types() {
11391163
let tmp = tempfile::tempdir().unwrap();
@@ -1148,10 +1172,6 @@ fn malformed_resource_envelopes_are_rejected_without_defining_downstream_types()
11481172
r#"resource "work" _tag="issue" uri="issue://example/1" reason="Task.""#,
11491173
),
11501174
("missing-uri", r#"resource "work" reason="Task.""#),
1151-
(
1152-
"relative-uri",
1153-
r#"resource "work" uri="./issue/1" reason="Task.""#,
1154-
),
11551175
(
11561176
"policy",
11571177
r#"resource "work" uri="issue://example/1" reason="Task." required=#true"#,
@@ -1170,7 +1190,7 @@ fn malformed_resource_envelopes_are_rejected_without_defining_downstream_types()
11701190

11711191
let found = discover(tmp.path());
11721192
assert!(found.specs.is_empty(), "{:?}", found.specs);
1173-
assert_eq!(found.errors.len(), 6, "{:?}", found.errors);
1193+
assert_eq!(found.errors.len(), 5, "{:?}", found.errors);
11741194
let errors = found
11751195
.errors
11761196
.iter()
@@ -1186,12 +1206,6 @@ fn malformed_resource_envelopes_are_rejected_without_defining_downstream_types()
11861206
.iter()
11871207
.any(|error| error.contains("unsupported property `_tag`"))
11881208
);
1189-
assert!(
1190-
errors
1191-
.iter()
1192-
.any(|error| error.contains("needs string `uri`"))
1193-
);
1194-
assert!(errors.iter().any(|error| error.contains("absolute URI")));
11951209
assert!(
11961210
errors
11971211
.iter()
@@ -1204,6 +1218,146 @@ fn malformed_resource_envelopes_are_rejected_without_defining_downstream_types()
12041218
);
12051219
}
12061220

1221+
#[test]
1222+
fn catalog_relative_resource_uris_are_an_st2_extension_resolved_against_the_declaration_dir() {
1223+
// Catalog-relative carrier paths are an st2 extension pending canonical Agent Spec adoption
1224+
// (see docs/vrs/06-resync): accepted here, resolved by the consumer against the declaration
1225+
// directory.
1226+
let tmp = tempfile::tempdir().unwrap();
1227+
write(
1228+
tmp.path(),
1229+
"agents/h/relative/agent.kdl",
1230+
r#"agent "relative" {
1231+
host "h"
1232+
command "true"
1233+
resource "work" uri="resources/goal.md" reason="Task."
1234+
}"#,
1235+
);
1236+
let found = discover(tmp.path());
1237+
assert!(
1238+
found.specs.iter().any(|spec| spec.identity == "relative"
1239+
&& spec
1240+
.resources
1241+
.iter()
1242+
.any(|r| r.uri() == "resources/goal.md")),
1243+
"{:?}",
1244+
found.errors
1245+
);
1246+
let descriptor: Resource =
1247+
serde_json::from_str(r#"{"name":"work","uri":"resources/goal.md","reason":"Task."}"#)
1248+
.expect("catalog-relative uri is valid");
1249+
assert_eq!(descriptor.uri(), "resources/goal.md");
1250+
1251+
let absolute = Resource::new(
1252+
"work".into(),
1253+
"/etc/absolute".into(),
1254+
"Still refused.".into(),
1255+
);
1256+
assert!(absolute.is_err(), "absolute paths must keep a scheme");
1257+
}
1258+
1259+
#[test]
1260+
fn resource_percent_paths_are_validated_consistently_across_public_inputs() {
1261+
let tmp = tempfile::tempdir().unwrap();
1262+
write(
1263+
tmp.path(),
1264+
"agents/h/valid-kdl/agent.kdl",
1265+
r#"agent "valid-kdl" {
1266+
host "h"
1267+
command "true"
1268+
resource "work" uri="resources/with%20space.md" reason="Task."
1269+
}"#,
1270+
);
1271+
write(
1272+
tmp.path(),
1273+
"agents/h/invalid-kdl/agent.kdl",
1274+
r#"agent "invalid-kdl" {
1275+
host "h"
1276+
command "true"
1277+
resource "work" uri="resources/a%00b" reason="Task."
1278+
}"#,
1279+
);
1280+
write(
1281+
tmp.path(),
1282+
"agents/h/valid-json/agent.json",
1283+
r#"{
1284+
"identity": "valid-json",
1285+
"host": "h",
1286+
"command": "true",
1287+
"resource": {"work": {"uri": "resources/with%20space.md", "reason": "Task."}}
1288+
}"#,
1289+
);
1290+
write(
1291+
tmp.path(),
1292+
"agents/h/invalid-json/agent.json",
1293+
r#"{
1294+
"identity": "invalid-json",
1295+
"host": "h",
1296+
"command": "true",
1297+
"resource": {"work": {"uri": "resources/encoded%2Fseparator", "reason": "Task."}}
1298+
}"#,
1299+
);
1300+
write(
1301+
tmp.path(),
1302+
"agents/h/valid-toml/agent.toml",
1303+
r#"identity = "valid-toml"
1304+
host = "h"
1305+
command = "true"
1306+
[resource.work]
1307+
uri = "resources/with%20space.md"
1308+
reason = "Task."
1309+
"#,
1310+
);
1311+
write(
1312+
tmp.path(),
1313+
"agents/h/invalid-toml/agent.toml",
1314+
r#"identity = "invalid-toml"
1315+
host = "h"
1316+
command = "true"
1317+
[resource.work]
1318+
uri = "resources/%FF.md"
1319+
reason = "Task."
1320+
"#,
1321+
);
1322+
1323+
let found = discover(tmp.path());
1324+
let identities = found
1325+
.specs
1326+
.iter()
1327+
.map(|spec| spec.identity.as_str())
1328+
.collect::<Vec<_>>();
1329+
assert_eq!(
1330+
identities,
1331+
vec!["valid-json", "valid-kdl", "valid-toml"],
1332+
"{:?}",
1333+
found.errors
1334+
);
1335+
assert_eq!(found.errors.len(), 3, "{:?}", found.errors);
1336+
1337+
let valid: Resource = serde_json::from_str(
1338+
r#"{"name":"work","uri":"resources/with%20space/%E2%82%AC.md","reason":"Task."}"#,
1339+
)
1340+
.unwrap();
1341+
assert_eq!(
1342+
valid.uri(),
1343+
"resources/with%20space/%E2%82%AC.md",
1344+
"validation must preserve the URI identity instead of normalizing it"
1345+
);
1346+
for uri in [
1347+
"resources/a%00b",
1348+
"resources/encoded%2Fseparator",
1349+
"resources/encoded%5cseparator",
1350+
"resources/%FF.md",
1351+
"resources/%2e%2e/goal.md",
1352+
] {
1353+
let descriptor = format!(r#"{{"name":"work","uri":"{uri}","reason":"Task."}}"#);
1354+
assert!(
1355+
serde_json::from_str::<Resource>(&descriptor).is_err(),
1356+
"{uri}"
1357+
);
1358+
}
1359+
}
1360+
12071361
#[test]
12081362
fn duplicate_json_resource_names_are_rejected_instead_of_last_write_winning() {
12091363
let tmp = tempfile::tempdir().unwrap();
@@ -1233,7 +1387,7 @@ fn duplicate_json_resource_names_are_rejected_instead_of_last_write_winning() {
12331387
fn public_resource_json_deserialization_enforces_the_catalog_invariants() {
12341388
for descriptor in [
12351389
r#"{"name":"","uri":"issue://one","reason":"Task."}"#,
1236-
r#"{"name":"work","uri":"./relative","reason":"Task."}"#,
1390+
r#"{"name":"work","uri":"/etc/absolute","reason":"Task."}"#,
12371391
r#"{"name":"work","uri":"issue://one","reason":"Task.","_tag":"issue"}"#,
12381392
r#"{"name":"work","uri":"issue://one","reason":"Task.","required":true}"#,
12391393
] {

0 commit comments

Comments
 (0)