Lower compiler-produced Edict operations generically - #692
Conversation
|
@coderabbitai review |
|
@codex please review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (29)
📒 Files selected for processing (52)
✨ 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 review |
|
@codex please review |
✅ Action performedReview finished.
|
|
Merge gate: CI is fully green; no bot-authored review threads exist; CodeRabbit is rate-limited; Codex acknowledged the current head and produced no findings within the bounded review window. Echo main is unprotected and no collaborator approval is required, so criteria 1–4 substitute for human approval. |
![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: 5917d6e0bc
ℹ️ 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".
| if text_field(step, "targetIntrinsic") != Some(TARGET_INTRINSIC) | ||
| || as_text(failure) != Some(PRECONDITION_MISMATCH) |
There was a problem hiding this comment.
When a schema-valid Target IR changes its effect, input, result, constraints, budget, requirements, binding, or obstructionArms while retaining this intrinsic and failure coordinate, this check still accepts it. The expected package later copies the supplied Target IR digest, so a package emitted for that altered IR compares equal and receives an accepted verifier report even though the IR may no longer implement the supplied Core/adapter relation. Independently compare all semantically relevant target-intent and step fields before certifying the package.
Useful? React with 👍 / 👎.
| "application_schema_coordinate", | ||
| canonical_text(&closure.exports.artifact.reference.coordinate), |
There was a problem hiding this comment.
Every generic package labels the selected lawpack-exports artifact as its application schema. That artifact is the portable lawpack export surface, not the arbitrary application's public contract schema, and no application-schema artifact is otherwise supplied or checked. The emitted application_schema_coordinate and identity therefore attest the wrong artifact, so admission and invocation evidence cannot bind the public input/output contract that the package claims to own.
Useful? React with 👍 / 👎.
| if core_effect | ||
| .strip_prefix(lawpack_coordinate) | ||
| .is_some_and(|suffix| suffix.starts_with('.') && suffix.len() > 1) | ||
| { | ||
| return Ok(core_effect.to_owned()); |
There was a problem hiding this comment.
When compiler-produced Core contains the fully qualified lawpack effect coordinate, this early return bypasses source inspection entirely. Because validate_source checks only UTF-8 and coordinate equality, arbitrary source bytes can be substituted under that coordinate and the emitted package will bind their digest; the verifier follows the same shortcut and accepts the false source-to-Core/package relation. Require the exact digest-locked lawpack import from parsed source or corroborate the equivalent compiler-owned Core import before returning.
Useful? React with 👍 / 👎.
| const GENERATOR_COORDINATE_V1: &str = "echo-wesley-gen.provider-artifact-generator@1"; | ||
| const EXPECTED_ARTIFACT_COUNT: usize = 10; | ||
| const EXPECTED_SCHEMA_BINDING_COUNT: usize = 24; | ||
| const EXPECTED_SCHEMA_BINDING_COUNT: usize = 30; |
There was a problem hiding this comment.
This increases the package closure to 30 schema bindings, but docs/architecture/application-contract-hosting.md still describes the package identity as containing 24 bindings and decomposes that obsolete count. That stale canonical description can cause reviewers or external package implementations to construct the wrong identity preimage; update the owning architecture document alongside the generated/package documentation.
AGENTS.md reference: AGENTS.md:L144-L150
Useful? React with 👍 / 👎.
| let selected = adapters | ||
| .iter() | ||
| .find(|candidate| { | ||
| required_map(candidate, "acceptedTargetProfile", "lawpack.echo-operation") | ||
| .is_ok_and(|reference| resource_ref_matches(reference, target_profile)) |
There was a problem hiding this comment.
When a schema-valid lawpack contains multiple targetAdapters entries for the same exact target profile, this selects the first instead of rejecting the ambiguous lawpack. The CDDL represents these entries as an array and does not enforce selector uniqueness, while this generic route does not run the semantic-source validator that normally rejects duplicate selectors. Conflicting adapters can therefore be hidden behind array order and both the lowerer and copied verifier logic will certify whichever entry appears first; require exactly one matching selector.
Useful? React with 👍 / 👎.
| The refreshed 230,285-byte checked lowerer component has SHA-256 | ||
| `9575ca8c843f3dfe4e5550ecdc40a8a8584c1aa7012b5b6f6a76e3f05f1b68f3` and has |
There was a problem hiding this comment.
This README publishes a 230,285-byte lowerer with digest 9575ca…, but the checked component in this revision is 230,297 bytes with SHA-256 f800df20…; the verifier README and component README likewise retain the obsolete 247,754-byte/d87f3d… identity instead of the checked 247,766-byte/edbef0fe… artifact. Anyone following these audit pins will reject the repository artifacts or authenticate obsolete bytes, so synchronize all component identity documentation with the promoted files.
AGENTS.md reference: AGENTS.md:L144-L150
Useful? React with 👍 / 👎.
1 participant
Lower compiler-produced Edict operations through generic provider components and an independent verifier. Pin the checked provider package and exact Edict host witness.
Closes #691.