feat(core): add optic invocation admission skeleton#331
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds v0 optic invocation request types and an admission-only admission method on OpticArtifactRegistry that resolves artifact handles, checks operation id and capability presentation, and always returns an obstruction outcome; includes tests for each obstruction reason and small export/changelog edits. ChangesOptic Invocation Admission
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 253-257: The module-level rustdoc is outdated: it claims
invocation admission is intentionally out of scope, but the function
admit_optic_invocation now implements admission behavior; update the module
header to reflect the new admission boundary by removing or revising the
sentence that says invocation admission is out of scope and describing that
admit_optic_invocation handles admission (mention the function name
admit_optic_invocation) and any relevant intended semantics or limitations so
readers understand the current responsibility of this module.
In `@crates/warp-core/tests/optic_invocation_admission_tests.rs`:
- Around line 89-100: Add a new test that mirrors
optic_invocation_obstructs_missing_capability_for_registered_handle but sets
invocation.capability_presentation = Some(OpticCapabilityPresentation { ... })
(use fixture_invocation(handle) and then mutate or construct the invocation to
include a capability_presentation), call
registry.admit_optic_invocation(&invocation), and assert the result equals
OpticInvocationAdmissionOutcome::Obstructed(OpticInvocationObstruction::CapabilityValidationUnavailable);
reference the functions/values admit_optic_invocation,
OpticInvocationAdmissionOutcome, OpticInvocationObstruction,
OpticCapabilityPresentation, fixture_registry_and_handle, and fixture_invocation
to locate where to add the test.
- Around line 38-40: The test currently calls
registry.register_optic_artifact(fixture_artifact(),
fixture_descriptor()).expect("fixture descriptor should register") which
triggers clippy::expect_used; replace the expect call with explicit error
handling (e.g., match the Result or use unwrap_or_else with a panic!) so
failures still produce the "fixture descriptor should register" message; update
the binding to let handle = match
registry.register_optic_artifact(fixture_artifact(), fixture_descriptor()) {
Ok(h) => h, Err(e) => panic!("fixture descriptor should register: {:?}", e) }
(or equivalent) to satisfy the lint while preserving diagnostic output for
register_optic_artifact, fixture_artifact, and fixture_descriptor.
🪄 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: 75309d48-9a63-4800-88d8-298bef84696e
📒 Files selected for processing (4)
CHANGELOG.mdcrates/warp-core/src/lib.rscrates/warp-core/src/optic_artifact.rscrates/warp-core/tests/optic_invocation_admission_tests.rs
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
crates/warp-core/src/optic_artifact.rs (1)
254-259:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate module rustdoc to reflect that invocation admission now exists.
admit_optic_invocationis implemented, but Line 5–Line 8 still says invocation admission is out of scope. Keep docs aligned with current module responsibility.Proposed doc patch
-//! This module intentionally stops at registration. It does not admit -//! invocations, validate capability grants, issue admission tickets, or execute -//! runtime work. The only authority proven here is that Echo accepted and -//! stored a specific Wesley artifact identity and its admission requirements. +//! This module owns registration and an admission-only invocation gate. +//! It admits/obstructs invocation requests at the handle/operation/capability +//! boundary, but does not validate grants, issue admission tickets, or execute +//! runtime work.🤖 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 254 - 259, The module-level rustdoc incorrectly claims invocation admission is out of scope even though admit_optic_invocation is implemented; update the comments preceding the function admit_optic_invocation to state that invocation admission is supported (describe that the function can admit or obstruct invocations and that success paths now exist), remove the sentence about "no success path", and adjust the must_use/summary text to accurately describe the current behavior of admit_optic_invocation so docs match the implementation.crates/warp-core/tests/optic_invocation_admission_tests.rs (1)
39-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace
expect(...)in test fixture setup to satisfy enforced clippy policy.
This call site has already been flagged by CI forclippy::expect_used; keep the same failure context with explicit match/panic handling.As per coding guidelines, "`**/*.rs`: Run `cargo clippy --all-targets` for linting with a zero warnings policy."Proposed patch
- let handle = registry - .register_optic_artifact(fixture_artifact(), fixture_descriptor()) - .expect("fixture descriptor should register"); + let handle = match registry.register_optic_artifact(fixture_artifact(), fixture_descriptor()) { + Ok(handle) => handle, + Err(err) => panic!("fixture descriptor should register: {err:?}"), + };🤖 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/tests/optic_invocation_admission_tests.rs` around lines 39 - 41, Change the test setup to avoid using expect(...) on the call to registry.register_optic_artifact(fixture_artifact(), fixture_descriptor()) by matching its Result and panicking with the same message on Err; specifically, replace the expect call on register_optic_artifact so that the Result is matched (e.g., match registry.register_optic_artifact(...)) and on Err call panic!("fixture descriptor should register: {:?}", err) (or similar) while binding the Ok value to handle, preserving the original failure context and message.
🤖 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.
Duplicate comments:
In `@crates/warp-core/src/optic_artifact.rs`:
- Around line 254-259: The module-level rustdoc incorrectly claims invocation
admission is out of scope even though admit_optic_invocation is implemented;
update the comments preceding the function admit_optic_invocation to state that
invocation admission is supported (describe that the function can admit or
obstruct invocations and that success paths now exist), remove the sentence
about "no success path", and adjust the must_use/summary text to accurately
describe the current behavior of admit_optic_invocation so docs match the
implementation.
In `@crates/warp-core/tests/optic_invocation_admission_tests.rs`:
- Around line 39-41: Change the test setup to avoid using expect(...) on the
call to registry.register_optic_artifact(fixture_artifact(),
fixture_descriptor()) by matching its Result and panicking with the same message
on Err; specifically, replace the expect call on register_optic_artifact so that
the Result is matched (e.g., match registry.register_optic_artifact(...)) and on
Err call panic!("fixture descriptor should register: {:?}", err) (or similar)
while binding the Ok value to handle, preserving the original failure context
and message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 936f1adc-12ce-47dd-8f9f-483ac72e4097
📒 Files selected for processing (3)
CHANGELOG.mdcrates/warp-core/src/optic_artifact.rscrates/warp-core/tests/optic_invocation_admission_tests.rs
Owner
Author
|
This was referenced May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the first Echo-side optic invocation admission boundary.
This proves that a registered
OpticArtifactHandleis not authority. Echo resolves the handle internally, checks the requested operation against registered metadata, and obstructs invocations that use unknown handles, mismatched operations, or registered handles without capability presentation.This slice intentionally does not add grant validation,
AdmissionTicket,LawWitness, runtime execution, scheduler integration, WASM ABI changes, app nouns, or Continuum schema publication.What this unlocks
The stack now has three vertebrae:
Verification
cargo test -p warp-core optic_invocationcargo test -p warp-core optic_artifact_registrycargo check -p warp-corescripts/ban-nondeterminism.shgit diff --checkSummary by CodeRabbit