Skip to content

Commit 2b998c3

Browse files
fix(resource): preserve publish ABI and refresh fence
agent-identity: dev3.direct.omp.2cshu64q agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.9 agent-runtime: OMP 18.0.9 tooling-profile: dotfiles@b607597
1 parent e574673 commit 2b998c3

3 files changed

Lines changed: 288 additions & 21 deletions

File tree

crates/st2-resource-protocol/src/lib.rs

Lines changed: 108 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -644,12 +644,11 @@ impl<'de> Deserialize<'de> for ObservationResult {
644644
}
645645
}
646646

647-
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
647+
#[derive(Debug, Clone, PartialEq, Serialize)]
648648
#[serde(
649649
tag = "type",
650650
rename_all = "camelCase",
651-
rename_all_fields = "camelCase",
652-
deny_unknown_fields
651+
rename_all_fields = "camelCase"
653652
)]
654653
pub enum RuntimeMessage {
655654
Publish {
@@ -678,6 +677,91 @@ pub enum RuntimeMessage {
678677
},
679678
}
680679

680+
#[derive(Deserialize)]
681+
#[serde(
682+
tag = "type",
683+
rename_all = "camelCase",
684+
rename_all_fields = "camelCase",
685+
deny_unknown_fields
686+
)]
687+
enum RuntimeMessageWire {
688+
Publish {
689+
owner: RuntimeOwner,
690+
binding_id: BindingId,
691+
registration: RegistrationToken,
692+
/// ABI-3 runtimes may still send this retired producer timestamp. The host never trusted it,
693+
/// so decode it only to preserve that ABI and discard it at this boundary.
694+
observed_at: Option<String>,
695+
#[serde(flatten)]
696+
publication: Publication,
697+
},
698+
Health {
699+
owner: RuntimeOwner,
700+
binding_id: Option<BindingId>,
701+
registration: Option<RegistrationToken>,
702+
state: RuntimeHealthState,
703+
detail: Option<String>,
704+
},
705+
ObservationResult {
706+
owner: RuntimeOwner,
707+
binding_id: BindingId,
708+
registration: RegistrationToken,
709+
demand_watermark: u64,
710+
result: ObservationResult,
711+
},
712+
}
713+
714+
impl<'de> Deserialize<'de> for RuntimeMessage {
715+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
716+
where
717+
D: Deserializer<'de>,
718+
{
719+
Ok(match RuntimeMessageWire::deserialize(deserializer)? {
720+
RuntimeMessageWire::Publish {
721+
owner,
722+
binding_id,
723+
registration,
724+
observed_at,
725+
publication,
726+
} => {
727+
drop(observed_at);
728+
Self::Publish {
729+
owner,
730+
binding_id,
731+
registration,
732+
publication,
733+
}
734+
},
735+
RuntimeMessageWire::Health {
736+
owner,
737+
binding_id,
738+
registration,
739+
state,
740+
detail,
741+
} => Self::Health {
742+
owner,
743+
binding_id,
744+
registration,
745+
state,
746+
detail,
747+
},
748+
RuntimeMessageWire::ObservationResult {
749+
owner,
750+
binding_id,
751+
registration,
752+
demand_watermark,
753+
result,
754+
} => Self::ObservationResult {
755+
owner,
756+
binding_id,
757+
registration,
758+
demand_watermark,
759+
result,
760+
},
761+
})
762+
}
763+
}
764+
681765
#[derive(Debug)]
682766
pub enum ProtocolError {
683767
MissingNewline,
@@ -968,6 +1052,26 @@ mod tests {
9681052
);
9691053
}
9701054

1055+
#[test]
1056+
fn abi_3_publish_decodes_with_or_without_deprecated_observed_at_only() {
1057+
let without_observed_at = b"{\"type\":\"publish\",\"owner\":{\"incarnation\":\"incarnation\",\"claim\":\"claim\"},\"bindingId\":\"binding\",\"registration\":\"registration\",\"schemaId\":\"schema.v1\",\"mediaType\":\"application/json\",\"bytes\":\"b25lIGJ5dGU=\",\"topics\":[\"selected\"]}\n";
1058+
let with_observed_at = b"{\"type\":\"publish\",\"owner\":{\"incarnation\":\"incarnation\",\"claim\":\"claim\"},\"bindingId\":\"binding\",\"registration\":\"registration\",\"schemaId\":\"schema.v1\",\"mediaType\":\"application/json\",\"bytes\":\"b25lIGJ5dGU=\",\"topics\":[\"selected\"],\"observedAt\":\"2026-08-30T00:00:00Z\"}\n";
1059+
let unknown_field = b"{\"type\":\"publish\",\"owner\":{\"incarnation\":\"incarnation\",\"claim\":\"claim\"},\"bindingId\":\"binding\",\"registration\":\"registration\",\"schemaId\":\"schema.v1\",\"mediaType\":\"application/json\",\"bytes\":\"b25lIGJ5dGU=\",\"topics\":[\"selected\"],\"extra\":true}\n";
1060+
1061+
assert_eq!(
1062+
decode_runtime_line(without_observed_at).unwrap(),
1063+
publish(b"one byte")
1064+
);
1065+
assert_eq!(
1066+
decode_runtime_line(with_observed_at).unwrap(),
1067+
publish(b"one byte")
1068+
);
1069+
assert!(matches!(
1070+
decode_runtime_line(unknown_field),
1071+
Err(ProtocolError::Json(_))
1072+
));
1073+
}
1074+
9711075
#[test]
9721076
fn observation_results_have_one_atomic_tagged_wire_shape() {
9731077
let unchanged = RuntimeMessage::ObservationResult {
@@ -1104,11 +1208,7 @@ mod tests {
11041208
decode_runtime_line(unknown_publication_field),
11051209
Err(ProtocolError::Json(_))
11061210
));
1107-
let obsolete_observed_at = b"{\"type\":\"publish\",\"owner\":{\"incarnation\":\"i\",\"claim\":\"c\"},\"bindingId\":\"b\",\"registration\":\"r\",\"schemaId\":\"s\",\"mediaType\":\"m\",\"bytes\":\"\",\"topics\":[],\"observedAt\":\"2026-08-30T00:00:00Z\"}\n";
1108-
assert!(matches!(
1109-
decode_runtime_line(obsolete_observed_at),
1110-
Err(ProtocolError::Json(_))
1111-
));
1211+
11121212
let obsolete_settlement = b"{\"type\":\"observationSettled\",\"owner\":{\"incarnation\":\"i\",\"claim\":\"c\"},\"bindingId\":\"b\",\"registration\":\"r\",\"demandWatermark\":1,\"outcome\":\"unchanged\"}\n";
11131213
assert!(matches!(
11141214
decode_runtime_line(obsolete_settlement),

src/main.rs

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3379,6 +3379,19 @@ fn resource_bindings(
33793379
selector: &str,
33803380
host: &str,
33813381
) -> Result<(String, Vec<st2::Resource>)> {
3382+
with_resource_bindings_snapshot(root, selector, host, |identity, bindings| {
3383+
Ok((identity, bindings))
3384+
})
3385+
}
3386+
3387+
/// Resolve one binding projection and let the caller finish consuming that exact catalog snapshot
3388+
/// before its shared authoring fence is released.
3389+
fn with_resource_bindings_snapshot<T>(
3390+
root: &Path,
3391+
selector: &str,
3392+
host: &str,
3393+
consume: impl FnOnce(String, Vec<st2::Resource>) -> Result<T>,
3394+
) -> Result<T> {
33823395
let _catalog_lock = st2::CatalogLock::shared(root)
33833396
.context("acquire shared catalog-authoring lock for Resource bindings")?;
33843397
let found = st2::discover_strict(root);
@@ -3403,9 +3416,9 @@ fn resource_bindings(
34033416
} else {
34043417
exact
34053418
};
3406-
match matches.as_slice() {
3419+
let (identity, bindings) = match matches.as_slice() {
34073420
[] => anyhow::bail!("no agent '{selector}' found in catalog {}", root.display()),
3408-
[spec] => Ok((spec.bus_id(host), spec.resources.clone())),
3421+
[spec] => (spec.bus_id(host), spec.resources.clone()),
34093422
many => {
34103423
let mut candidates = many
34113424
.iter()
@@ -3417,9 +3430,27 @@ fn resource_bindings(
34173430
candidates.join(", ")
34183431
)
34193432
}
3433+
};
3434+
consume(identity, bindings)
3435+
}
3436+
3437+
#[cfg(debug_assertions)]
3438+
fn resource_refresh_snapshot_checkpoint() {
3439+
let (Ok(ready), Ok(release)) = (
3440+
std::env::var("ST2_TEST_RESOURCE_REFRESH_SNAPSHOT_READY"),
3441+
std::env::var("ST2_TEST_RESOURCE_REFRESH_SNAPSHOT_RELEASE"),
3442+
) else {
3443+
return;
3444+
};
3445+
let _ = std::fs::write(ready, b"ready");
3446+
while !Path::new(&release).exists() {
3447+
std::thread::yield_now();
34203448
}
34213449
}
34223450

3451+
#[cfg(not(debug_assertions))]
3452+
fn resource_refresh_snapshot_checkpoint() {}
3453+
34233454
fn resource_cmd(cmd: ResourceCmd) -> Result<()> {
34243455
match cmd {
34253456
ResourceCmd::Ls {
@@ -3502,17 +3533,24 @@ fn resource_cmd(cmd: ResourceCmd) -> Result<()> {
35023533
(selector, first)
35033534
}
35043535
};
3505-
let (identity, bindings) = resource_bindings(&root, &selector, &host)?;
3506-
bindings
3507-
.iter()
3508-
.find(|binding| binding.name() == name)
3509-
.with_context(|| format!("no resource binding '{name}' declared by {identity}"))?;
3510-
let request = st2::resource_observe::ObserveRequest::new(
3511-
identity.clone(),
3512-
name.clone(),
3513-
st2::resource_observe::catalog_generation(&root)?,
3514-
None,
3515-
)?;
3536+
let (identity, request) =
3537+
with_resource_bindings_snapshot(&root, &selector, &host, |identity, bindings| {
3538+
bindings
3539+
.iter()
3540+
.find(|binding| binding.name() == name)
3541+
.with_context(|| {
3542+
format!("no resource binding '{name}' declared by {identity}")
3543+
})?;
3544+
resource_refresh_snapshot_checkpoint();
3545+
let generation = st2::resource_observe::catalog_generation(&root)?;
3546+
let request = st2::resource_observe::ObserveRequest::new(
3547+
identity.clone(),
3548+
name.clone(),
3549+
generation,
3550+
None,
3551+
)?;
3552+
Ok((identity, request))
3553+
})?;
35163554
let request_id = request.request_id.clone();
35173555
let client = st2::resource_observe::submit_request(&root, &host, &request)?;
35183556
let waited = client.wait_for_terminal(Duration::from_secs(wait))?;

tests/agent_resource.rs

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,135 @@ fn a_binding_with_a_trailing_line_comment_is_removable_and_updatable() {
507507
);
508508
}
509509

510+
#[test]
511+
fn refresh_binding_validation_and_generation_share_one_catalog_snapshot() {
512+
let _guard = OBSERVE_ENV_LOCK
513+
.lock()
514+
.unwrap_or_else(std::sync::PoisonError::into_inner);
515+
let temporary = tempfile::tempdir().unwrap();
516+
let root = temporary.path().join("catalog");
517+
let state = temporary.path().join("state");
518+
write(
519+
&root,
520+
"h/worker/agent.kdl",
521+
&declaration(
522+
"worker",
523+
"catalog",
524+
" resource \"work\" reason=\"Old binding.\" uri=\"https://example.test/old\"\n",
525+
),
526+
);
527+
ok(
528+
&root,
529+
&[
530+
"resource",
531+
"add",
532+
"anchor",
533+
"--agent",
534+
"worker",
535+
"--uri",
536+
"https://example.test/anchor",
537+
"--reason",
538+
"Initialize the catalog generation.",
539+
],
540+
);
541+
let old_generation = st2::resource_observe::catalog_generation(&root)
542+
.unwrap()
543+
.expect("mediated edit initialized catalog generation");
544+
545+
let previous_state = std::env::var_os("XDG_STATE_HOME");
546+
unsafe { std::env::set_var("XDG_STATE_HOME", &state) };
547+
st2::event::publish_owner_binding_for_test(&root, "h").unwrap();
548+
let scope = st2::park::SupervisorScope::current(&root, "h").unwrap();
549+
let request_dir = scope
550+
.park_dir()
551+
.parent()
552+
.unwrap()
553+
.join("observe-requests");
554+
match previous_state {
555+
Some(value) => unsafe { std::env::set_var("XDG_STATE_HOME", value) },
556+
None => unsafe { std::env::remove_var("XDG_STATE_HOME") },
557+
}
558+
559+
let snapshot_ready = temporary.path().join("snapshot-ready");
560+
let snapshot_release = temporary.path().join("snapshot-release");
561+
let replacement_attempt = temporary.path().join("replacement-attempt");
562+
let refresh = Command::new(env!("CARGO_BIN_EXE_st2"))
563+
.args(["--catalog", root.to_str().unwrap()])
564+
.args([
565+
"resource", "refresh", "worker", "work", "--wait", "0", "--host", "h",
566+
])
567+
.env("XDG_STATE_HOME", &state)
568+
.env("ST2_TEST_RESOURCE_REFRESH_SNAPSHOT_READY", &snapshot_ready)
569+
.env(
570+
"ST2_TEST_RESOURCE_REFRESH_SNAPSHOT_RELEASE",
571+
&snapshot_release,
572+
)
573+
.env_remove("ST_AGENT")
574+
.stdout(Stdio::piped())
575+
.stderr(Stdio::piped())
576+
.spawn()
577+
.unwrap();
578+
wait_for_file(&snapshot_ready);
579+
580+
let mut replacement = Command::new(env!("CARGO_BIN_EXE_st2"))
581+
.args(["--catalog", root.to_str().unwrap()])
582+
.args([
583+
"resource",
584+
"add",
585+
"work",
586+
"--agent",
587+
"worker",
588+
"--uri",
589+
"https://example.test/replacement",
590+
"--reason",
591+
"Replacement binding.",
592+
])
593+
.env("ST2_TEST_CATALOG_LOCK_ATTEMPT", &replacement_attempt)
594+
.env_remove("ST_AGENT")
595+
.stdout(Stdio::piped())
596+
.stderr(Stdio::piped())
597+
.spawn()
598+
.unwrap();
599+
wait_for_file(&replacement_attempt);
600+
assert!(
601+
replacement.try_wait().unwrap().is_none(),
602+
"replacement crossed the shared snapshot lock before generation capture"
603+
);
604+
assert!(
605+
spec(&root).contains("https://example.test/old"),
606+
"replacement landed while refresh held its snapshot"
607+
);
608+
609+
fs::write(&snapshot_release, b"release").unwrap();
610+
let refresh = refresh.wait_with_output().unwrap();
611+
assert!(
612+
!refresh.status.success(),
613+
"zero client wait should leave the request queued"
614+
);
615+
let replacement = replacement.wait_with_output().unwrap();
616+
assert!(
617+
replacement.status.success(),
618+
"stdout: {}\nstderr: {}",
619+
stdout(&replacement),
620+
stderr(&replacement)
621+
);
622+
623+
let request = wait_for_refresh_request(&request_dir);
624+
assert_eq!(
625+
request["expectedCatalogGeneration"],
626+
serde_json::json!(old_generation),
627+
"request generation must come from the validated old binding snapshot"
628+
);
629+
let replacement_generation = st2::resource_observe::catalog_generation(&root)
630+
.unwrap()
631+
.unwrap();
632+
assert!(
633+
replacement_generation > old_generation,
634+
"replacement did not advance catalog generation"
635+
);
636+
assert!(spec(&root).contains("https://example.test/replacement"));
637+
}
638+
510639
#[test]
511640
fn refresh_cli_reports_exact_receipts_and_wait_expiry_keeps_the_request() {
512641
let _guard = OBSERVE_ENV_LOCK

0 commit comments

Comments
 (0)