feat(core): add runtime support v0 boundary#354
Conversation
📝 WalkthroughWalkthroughThis PR introduces a ChangesRuntimeSupport v0 Boundary Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex please confirm these self-review findings before merge.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19895005bf
ℹ️ 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".
| pub fn record_runtime_support_v0_fixture_for_requirements( | ||
| &mut self, | ||
| requirements_digest: impl Into<String>, | ||
| ) { | ||
| let requirements_digest = requirements_digest.into(); | ||
| let support_digest = runtime_support_v0_fixture_digest(); | ||
| self.runtime_support_v0_by_requirements | ||
| .insert(requirements_digest.clone(), support_digest); | ||
| self.publish_runtime_support_recorded_fact(requirements_digest, support_digest); |
There was a problem hiding this comment.
record_runtime_support_v0_fixture_for_requirements accepts any caller-provided digest and always publishes GraphFact::RuntimeSupportRecorded without checking that the digest belongs to a currently registered artifact. In this commit that makes RuntimeSupport v0 authority depend on convention: code with &mut OpticArtifactRegistry can mint support facts for nonexistent (or not-yet-registered) requirements, and those facts are later consumed by admission once an artifact with that digest appears. This contradicts the method’s “registered requirements digest” contract and weakens provenance integrity for runtime-owned evidence.
Useful? React with 👍 / 👎.
| grant, or admit an invocation. The only lawful next refusal in this slice is | ||
| `RuntimeSupportUnavailable`. |
There was a problem hiding this comment.
The BudgetResolution v0 section still states that the only lawful next refusal is RuntimeSupportUnavailable, but this same change introduces a second lawful outcome after budget resolution when runtime support is present: InvocationAdmissionUnavailable. Leaving this sentence unchanged makes the design checkpoint inconsistent with implemented admission behavior and can mislead follow-on tests/docs that treat this file as the spec.
Useful? React with 👍 / 👎.
|
Resolved the self-review findings.
Validation:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/warp-core/src/optic_artifact.rs`:
- Around line 1097-1128: The current flow validates handles in
record_runtime_support_v0_fixture_for_artifact then unconditionally calls
record_runtime_support_v0_fixture_for_requirements_digest which inserts into
runtime_support_v0_by_requirements and always calls
publish_runtime_support_recorded_fact, causing duplicate RuntimeSupportRecorded
facts when multiple artifacts share a requirements_digest; to fix, make
record_runtime_support_v0_fixture_for_requirements_digest check whether
runtime_support_v0_by_requirements already contains the requirements_digest (or
the same support_digest) and only call publish_runtime_support_recorded_fact
when inserting a new mapping, leaving
record_runtime_support_v0_fixture_for_artifact and
runtime_support_v0_fixture_digest unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ffaf3ad5-f37f-4371-9d1c-8a6cb4616017
📒 Files selected for processing (6)
CHANGELOG.mdcrates/warp-core/src/causal_facts.rscrates/warp-core/src/optic_artifact.rscrates/warp-core/tests/causal_fact_publication_tests.rscrates/warp-core/tests/optic_invocation_admission_tests.rsdocs/design/optic-admission-ladder-checkpoint.md
| /// Records Echo-owned RuntimeSupport v0 fixture evidence for a registered | ||
| /// artifact's requirements digest. | ||
| /// | ||
| /// This is runtime context, not invocation context. Callers cannot provide | ||
| /// a support request through [`OpticInvocation`]; the admission ladder only | ||
| /// consults facts recorded on this registry. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns [`OpticArtifactRegistrationError::UnknownHandle`] if Echo did not | ||
| /// issue the handle in this registry instance. | ||
| pub fn record_runtime_support_v0_fixture_for_artifact( | ||
| &mut self, | ||
| handle: &OpticArtifactHandle, | ||
| ) -> Result<(), OpticArtifactRegistrationError> { | ||
| let requirements_digest = self | ||
| .resolve_optic_artifact_handle(handle)? | ||
| .requirements_digest | ||
| .clone(); | ||
| self.record_runtime_support_v0_fixture_for_requirements_digest(requirements_digest); | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn record_runtime_support_v0_fixture_for_requirements_digest( | ||
| &mut self, | ||
| requirements_digest: String, | ||
| ) { | ||
| let support_digest = runtime_support_v0_fixture_digest(); | ||
| self.runtime_support_v0_by_requirements | ||
| .insert(requirements_digest.clone(), support_digest); | ||
| self.publish_runtime_support_recorded_fact(requirements_digest, support_digest); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Handle-scoped API addresses the P2 authority boundary concern.
The method correctly validates the handle via resolve_optic_artifact_handle before recording support, rejecting unknown handles with UnknownHandle error. This ensures RuntimeSupportRecorded facts can only reference registered artifacts.
One consideration: repeated calls for artifacts sharing the same requirements_digest will publish duplicate RuntimeSupportRecorded facts. The map entry is idempotent, but published_graph_facts accumulates duplicates. If callers expect idempotent recording, consider guarding the publish with an existence check.
Optional: Add idempotency guard
fn record_runtime_support_v0_fixture_for_requirements_digest(
&mut self,
requirements_digest: String,
) {
let support_digest = runtime_support_v0_fixture_digest();
+ if self.runtime_support_v0_by_requirements.contains_key(&requirements_digest) {
+ return;
+ }
self.runtime_support_v0_by_requirements
.insert(requirements_digest.clone(), support_digest);
self.publish_runtime_support_recorded_fact(requirements_digest, support_digest);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Records Echo-owned RuntimeSupport v0 fixture evidence for a registered | |
| /// artifact's requirements digest. | |
| /// | |
| /// This is runtime context, not invocation context. Callers cannot provide | |
| /// a support request through [`OpticInvocation`]; the admission ladder only | |
| /// consults facts recorded on this registry. | |
| /// | |
| /// # Errors | |
| /// | |
| /// Returns [`OpticArtifactRegistrationError::UnknownHandle`] if Echo did not | |
| /// issue the handle in this registry instance. | |
| pub fn record_runtime_support_v0_fixture_for_artifact( | |
| &mut self, | |
| handle: &OpticArtifactHandle, | |
| ) -> Result<(), OpticArtifactRegistrationError> { | |
| let requirements_digest = self | |
| .resolve_optic_artifact_handle(handle)? | |
| .requirements_digest | |
| .clone(); | |
| self.record_runtime_support_v0_fixture_for_requirements_digest(requirements_digest); | |
| Ok(()) | |
| } | |
| fn record_runtime_support_v0_fixture_for_requirements_digest( | |
| &mut self, | |
| requirements_digest: String, | |
| ) { | |
| let support_digest = runtime_support_v0_fixture_digest(); | |
| self.runtime_support_v0_by_requirements | |
| .insert(requirements_digest.clone(), support_digest); | |
| self.publish_runtime_support_recorded_fact(requirements_digest, support_digest); | |
| } | |
| /// Records Echo-owned RuntimeSupport v0 fixture evidence for a registered | |
| /// artifact's requirements digest. | |
| /// | |
| /// This is runtime context, not invocation context. Callers cannot provide | |
| /// a support request through [`OpticInvocation`]; the admission ladder only | |
| /// consults facts recorded on this registry. | |
| /// | |
| /// # Errors | |
| /// | |
| /// Returns [`OpticArtifactRegistrationError::UnknownHandle`] if Echo did not | |
| /// issue the handle in this registry instance. | |
| pub fn record_runtime_support_v0_fixture_for_artifact( | |
| &mut self, | |
| handle: &OpticArtifactHandle, | |
| ) -> Result<(), OpticArtifactRegistrationError> { | |
| let requirements_digest = self | |
| .resolve_optic_artifact_handle(handle)? | |
| .requirements_digest | |
| .clone(); | |
| self.record_runtime_support_v0_fixture_for_requirements_digest(requirements_digest); | |
| Ok(()) | |
| } | |
| fn record_runtime_support_v0_fixture_for_requirements_digest( | |
| &mut self, | |
| requirements_digest: String, | |
| ) { | |
| let support_digest = runtime_support_v0_fixture_digest(); | |
| if self.runtime_support_v0_by_requirements.contains_key(&requirements_digest) { | |
| return; | |
| } | |
| self.runtime_support_v0_by_requirements | |
| .insert(requirements_digest.clone(), support_digest); | |
| self.publish_runtime_support_recorded_fact(requirements_digest, support_digest); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/warp-core/src/optic_artifact.rs` around lines 1097 - 1128, The current
flow validates handles in record_runtime_support_v0_fixture_for_artifact then
unconditionally calls record_runtime_support_v0_fixture_for_requirements_digest
which inserts into runtime_support_v0_by_requirements and always calls
publish_runtime_support_recorded_fact, causing duplicate RuntimeSupportRecorded
facts when multiple artifacts share a requirements_digest; to fix, make
record_runtime_support_v0_fixture_for_requirements_digest check whether
runtime_support_v0_by_requirements already contains the requirements_digest (or
the same support_digest) and only call publish_runtime_support_recorded_fact
when inserting a new mapping, leaving
record_runtime_support_v0_fixture_for_artifact and
runtime_support_v0_fixture_digest unchanged.
1 participant
Summary
InvocationAdmissionUnavailableas the terminal obstruction after runtime support resolvesTests
cargo test -p warp-core --test causal_fact_publication_testscargo test -p warp-core --test optic_invocation_admission_testscargo test -p warp-corecargo checkcargo fmt --checkgit diff --checkpnpm exec markdownlint-cli2 CHANGELOG.md docs/design/optic-admission-ladder-checkpoint.mdSummary by CodeRabbit
Release Notes
New Features
Documentation