Skip to content

Commit b30015e

Browse files
committed
fix(resync): retry publication and tighten carriers
1 parent 5823ea4 commit b30015e

4 files changed

Lines changed: 125 additions & 45 deletions

File tree

crates/agent-spec/src/spec.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,18 @@ struct ResourceDescriptor {
240240
impl Resource {
241241
/// Construct a descriptor after enforcing the same invariants as catalog parsing.
242242
pub fn new(name: String, uri: String, reason: String) -> Result<Self, String> {
243-
if name.is_empty() {
244-
return Err("resource binding name cannot be empty".into());
243+
if name.is_empty()
244+
|| name.len() > 200
245+
|| name.trim() != name
246+
|| name.chars().any(char::is_control)
247+
{
248+
return Err(
249+
"resource binding name must be 1..=200 bytes without surrounding whitespace or controls"
250+
.into(),
251+
);
252+
}
253+
if name == "declaration" {
254+
return Err("resource binding name 'declaration' is reserved by resync".into());
245255
}
246256
validate_resource_uri(&uri).map_err(|reason| {
247257
format!("resource binding '{name}' `uri` must be an exact absolute URI or a catalog-relative path: {reason}")
@@ -752,22 +762,16 @@ fn validate_resource_explanation(name: &str, field: &str, value: &str) -> Result
752762
/// scheme at all, resolved by the consumer against the declaration directory. Relative carriers
753763
/// are an st2 extension pending canonical Agent Spec adoption (see 06-resync).
754764
fn validate_resource_uri(uri: &str) -> Result<(), &'static str> {
755-
match uri.split_once(':') {
756-
Some((scheme, _))
757-
if !scheme.contains('/')
758-
&& scheme
759-
.chars()
760-
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')) =>
761-
{
762-
validate_absolute_uri(uri)
763-
}
764-
_ => {
765-
if uri.is_empty() || uri.starts_with('/') || uri.contains("..") {
766-
return Err("catalog-relative uri must be a non-empty relative path without `..`");
767-
}
768-
Ok(())
765+
if let Some(colon) = uri.find(':') {
766+
let first_separator = uri.find(|character| matches!(character, '/' | '\\'));
767+
if first_separator.is_none_or(|separator| colon < separator) {
768+
return validate_absolute_uri(uri);
769769
}
770770
}
771+
if uri.is_empty() || uri.starts_with('/') || uri.contains("..") {
772+
return Err("catalog-relative uri must be a non-empty relative path without `..`");
773+
}
774+
Ok(())
771775
}
772776

773777
fn validate_absolute_uri(uri: &str) -> Result<(), &'static str> {

crates/agent-spec/tests/discovery.rs

Lines changed: 24 additions & 0 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();

docs/vrs/06-resync/spec.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ gets its own event so per-binding supersession stays meaningful.
7171
bus id, label, and class so every subscriber retains its digest and pending
7272
dirty state even when multiple bindings share one path; only new
7373
subscriptions seed silently.
74+
- A previously blind path is digest-diffed before its recovered parent directory is added to the
75+
registered-watch set, covering changes made during the blind interval.
7476
- Installation failure is diagnosed once and degrades to timer-based digest
7577
polling over the watch set (bounded by the number of bindings), never to
7678
silence about the mechanism.
@@ -82,11 +84,15 @@ agent without a declaration check; every other rule (host ownership,
8284
desired-state running, owner-binding validation, ring transaction) is the
8385
implemented [`STREAM-R03..R05`](../04-stream/spec.md) path untouched.
8486
Declaring a stream named `resync` in an Agent Spec is a validation error:
85-
the reservation must not be shadowable.
86-
87-
Digest state lives with the supervisor process (seeded at start, updated on
88-
each observed change); the durable dedup horizon remains the stream receipt
89-
ring, exactly as for external producers.
87+
the reservation must not be shadowable. Resource binding names use the event-key
88+
grammar (1..=200 bytes, no surrounding whitespace or controls), and
89+
`declaration` is reserved for the synthetic declaration carrier so supersession
90+
keys cannot collide.
91+
92+
Digest state lives with the supervisor process (seeded at start) and advances
93+
only after the unchanged ingress returns a successful receipt. Failed
94+
publication retains the old digest and dirty state and schedules the identical
95+
transition for retry; the durable dedup horizon remains the stream receipt ring.
9096

9197
## What this does not do
9298

src/resync.rs

Lines changed: 70 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,10 @@ impl Worker {
325325
self.diff_emit(&mut next);
326326
}
327327
self.carriers = next;
328-
self.refresh_watches();
329-
// Directories that could not be registered stay blind to events; their carriers fall
330-
// back to digest polling at refresh cadence (`RESYNC-R01`).
328+
// Diff paths that were blind before registering newly recovered parents; otherwise the
329+
// new watch suppresses polling of mutations that happened during the blind interval.
331330
self.poll_unwatched();
331+
self.refresh_watches();
332332
}
333333

334334
/// Digest-diff every entry in `next`, emitting observed transitions. Seeding stays silent
@@ -348,17 +348,18 @@ impl Worker {
348348
if entry.digest.as_deref() == Some(new_digest.as_str()) {
349349
continue;
350350
}
351-
let old = entry.digest.take();
352-
emit_resync(
351+
let old = entry.digest.as_deref();
352+
if emit_resync(
353353
&root,
354354
&this_host,
355355
&entry.bus_id,
356356
&entry.label,
357357
&path,
358-
old.as_deref(),
358+
old,
359359
&new_digest,
360-
);
361-
entry.digest = Some(new_digest);
360+
) {
361+
entry.digest = Some(new_digest);
362+
}
362363
}
363364
}
364365
}
@@ -487,33 +488,45 @@ impl Worker {
487488
}
488489
}
489490

490-
/// Flush every subscriber of one path: clear dirty flags, diff digests, emit transitions.
491+
/// Flush every subscriber of one path: diff digests, emit transitions, and retain failed
492+
/// publications for retry with the same event identity.
491493
fn flush_path(&mut self, path: &Path) {
492494
let Some(entries) = self.carriers.get_mut(path) else {
493495
return;
494496
};
497+
let mut retries = Vec::new();
495498
for entry in entries.iter_mut() {
496499
entry.dirty = false;
497500
let Some(new_digest) = read_digest(path) else {
498501
// Unreadable right now (deleted mid-window): stay quiet and keep the previous
499-
// digest, so a later reappearance still counts as a change. The next mutation
500-
// re-dirties.
502+
// digest, so a later reappearance still counts as a change.
501503
continue;
502504
};
503505
if entry.digest.as_deref() == Some(new_digest.as_str()) {
504506
continue;
505507
}
506-
let old = entry.digest.take();
507-
emit_resync(
508+
if emit_resync(
508509
&self.root,
509510
&self.this_host,
510511
&entry.bus_id,
511512
&entry.label,
512513
path,
513-
old.as_deref(),
514+
entry.digest.as_deref(),
514515
&new_digest,
515-
);
516-
entry.digest = Some(new_digest);
516+
) {
517+
entry.digest = Some(new_digest);
518+
} else {
519+
entry.dirty = true;
520+
retries.push(entry.class);
521+
}
522+
}
523+
let now = Instant::now();
524+
for class in retries {
525+
let deadline = now + class.window();
526+
self.deadlines
527+
.entry(class)
528+
.and_modify(|existing| *existing = (*existing).min(deadline))
529+
.or_insert(deadline);
517530
}
518531
}
519532
}
@@ -533,11 +546,11 @@ fn emit_resync(
533546
path: &Path,
534547
old: Option<&str>,
535548
new_digest: &str,
536-
) {
549+
) -> bool {
537550
let subject = format!("resource {label} changed");
538551
let body = render_body(label, path, old, new_digest);
539552
let event_id = transition_identity(&body);
540-
let receipt = crate::event::emit(
553+
match crate::event::emit(
541554
root,
542555
this_host,
543556
bus_id,
@@ -547,12 +560,15 @@ fn emit_resync(
547560
Some(subject.as_str()),
548561
&body,
549562
true,
550-
);
551-
if let Err(error) = receipt {
552-
eprintln!(
553-
"st2: resync emit for '{}' failed: {error:#}",
554-
path.display()
555-
);
563+
) {
564+
Ok(_) => true,
565+
Err(error) => {
566+
eprintln!(
567+
"st2: resync emit for '{}' failed: {error:#}",
568+
path.display()
569+
);
570+
false
571+
}
556572
}
557573
}
558574

@@ -678,6 +694,36 @@ mod tests {
678694
}
679695
}
680696

697+
#[test]
698+
fn failed_emit_retains_digest_and_schedules_the_same_transition_for_retry() {
699+
let root = tempfile::tempdir().unwrap();
700+
let carrier = root.path().join("carrier.md");
701+
std::fs::write(&carrier, "new bytes").unwrap();
702+
let mut worker = Worker {
703+
root: root.path().to_path_buf(),
704+
this_host: "host".to_owned(),
705+
carriers: BTreeMap::from([(
706+
carrier.clone(),
707+
vec![Entry {
708+
bus_id: "host.missing".to_owned(),
709+
label: "goal".to_owned(),
710+
class: CarrierClass::Immediate,
711+
digest: Some("old-digest".to_owned()),
712+
dirty: true,
713+
}],
714+
)]),
715+
deadlines: BTreeMap::new(),
716+
watched: BTreeMap::new(),
717+
watcher: None,
718+
};
719+
720+
worker.flush_path(&carrier);
721+
let entry = &worker.carriers[&carrier][0];
722+
assert_eq!(entry.digest.as_deref(), Some("old-digest"));
723+
assert!(entry.dirty);
724+
assert!(worker.deadlines.contains_key(&CarrierClass::Immediate));
725+
}
726+
681727
#[test]
682728
fn transition_identity_covers_every_rendered_transition_dimension() {
683729
let baseline = render_body(

0 commit comments

Comments
 (0)