Add atomic Resource demand observations - #404
Conversation
agent-identity: dev3.direct.omp.536sbpvb agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@000f2b3
059287e to
95b915d
Compare
agent-identity: dev3.direct.omp.536sbpvb agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@000f2b3
|
@codex please review this PR. Posted on behalf of @schickling
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d072a03901
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[serde(flatten)] | ||
| publication: Publication, |
There was a problem hiding this comment.
Keep observedAt accepted on ABI-3 publishes
Existing ABI-3 runtimes may include the previously supported optional observedAt field on every Publish. Flattening the new Publication type removes that field while RuntimeMessage and Publication still deny unknown fields, so those frames now fail decoding and the supervisor tears down the runtime, affecting every binding for a shared runtime. Continue accepting and ignoring the deprecated field, or introduce a new protocol version before removing it.
Useful? React with 👍 / 👎.
| let request = st2::resource_observe::ObserveRequest::new( | ||
| identity.clone(), | ||
| name.clone(), | ||
| st2::resource_observe::catalog_generation(&root)?, |
There was a problem hiding this comment.
Fence binding lookup and generation with the same lock
If a catalog author replaces the same agent-and-binding name after resource_bindings releases its shared lock but before this separate generation read, the request combines the old validated binding with the replacement's generation. Once the resident supervisor refreshes, that generation matches and the request is dispatched to the replacement binding rather than becoming stale, defeating the intended catalog fence. Read the binding and generation while holding one shared catalog lock.
Useful? React with 👍 / 👎.
Why
Observable Resource Profiles need an exact, bounded way to request a fresh observation from the resident profile runtime. Snapshot reads alone cannot distinguish unchanged data, provider failure, stale generations, or a request that is still queued.
The protocol must not split one observation across publication and settlement frames. A split result permits partial acceptance and duplicates the existing publication path.
What
Add the supervisor-scoped ObserveNow request/receipt protocol and
st2 resource refreshCLI.Extend the Resource Profile protocol with one atomic, status-tagged
ObservationResult:unchangedfailed, with an optional diagnosticpublished, with the sharedPublicationenvelopeObservecarries the demand watermark. PeriodicPublishand demand-drivenpublishedresults share the same publication validation, snapshot acceptance, typed facts, semantic invalidation, and catch-up core. PeriodicPublishremains wire-compatible through a flattened publication envelope, so the descriptor ABI stays at version 3.How
Requests and receipts use bounded, private, atomic files under the supervisor generation scope. Admission is locked and rejects the 257th concurrent request. The supervisor retains an enqueued request until a terminal receipt is durable, recovers queued work after restart, fences each result by owner/binding/registration/watermark, coalesces one in-flight and one trailing batch per binding, and retries terminal receipt writes without redispatching settled runtime work.
The runtime result is accepted as one unit. A
publishedresult is validated and stored before the matching terminal receipt is committed. Both publication paths use the same host-computed digest and bounded facts/topics contract. Missing demand capability settles asabsentBindingwith a diagnostic instead of sending an unsupported frame.Rationale
The atomic tagged result has one source of truth for observation outcome and publication content. It removes the former two-frame settlement overlap while retaining periodic publication as an independent provider behavior.
Durable request and receipt records make demand state inspectable and restart-safe. Exact receipts preserve changed, unchanged, failed, stale, absent, unavailable, and backpressured outcomes without a correctness timer.
Verification
cargo test --features wasm-resolver --test resource_profile_supervisor_e2e -- --nocapture(7 passed)cargo test --features wasm-resolver --test agent_resource -- --nocapture(9 passed)cargo test --features wasm-resolver --lib(all workspace library tests passed)nix build .#checks.x86_64-linux.wasm-resolver-feature --no-link --print-build-logs(passed)rustfmt --checkfor every changed Rust module and integration testgit diff --checkA full
nix flake checkpassed every reached check and test suite except the pre-existingcodex_app_server::tests::a_dead_socket_is_removed_before_launchrace in one parallel derivation. The exactwasm-resolver-featureNix check then passed, including the complete Resource supervisor suite.Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile